[PATCH] analysis of global statics and removal associated vdefs and vuses

[Mark Mitchell]

Kenneth Zadeck wrote:

> Mark Mitchell wrote:
>
>> Diego Novillo wrote:
>>
>>> On Fri, 2004-09-03 at 11:00, Kenneth Zadeck wrote:
>>>
>>>  
>>>
>>>> The enclosed patch essentually does everything suggested in Diego's 
>>>> original comments. (I only left the single comment in the mail that 
>>>> I discuss here)
>>>>
>>>>   
>>>
>>>
>>> Looks good to me now.  The timings are indeed better and in the 
>>> noise. We should get pretty good results once we propagate this 
>>> information
>>> more into alias analysis.
>>>
>>> I can't really approve the patch, though.  I'd like a maintainer to go
>>> over the Makefile.in and cgraph changes.
>>>
>> Every single function needs to say what every single argument is for 
>> and what the return value is.  The argument names should be in 
>> ALL_CAPS.  Also, single-statement dependent statements, like:
>>
>>  if (x)
>>    {
>>       f();
>>    }
>>
>> should be reformatted as:
>>
>>  if (x)
>>     f();
>>
>> and the braces never go on the same line as anything else, so:
>>
>>   } while (v != x)
>>
>> in searchc is wrong.  Kenny, you'll have to go through and fix all 
>> that up.
>>
>> Also, this:
>>
>> +     if (strcmp (TREE_STRING_POINTER (TREE_VALUE (link)), "memory") 
>> == 0)
>> is wrong; TREE_IDENTIFIERs are unique.  So, what you want is a global 
>> tree:
>>
>>  memory_identifier = get_identifier ("memory")
>>
>> and then, here, you just want to do: "TREE_VALUE (link) == 
>> memory_identifier".
>>
>> As for splay trees, I expect part of the problem is that we indirect 
>> through a function pointer to do the comparisons on the nodes.  C is 
>> not helpful here; no templates.  We could do macro stuff to make it 
>> faster, but using a hash table might be better.
>>
>> I'm going to approve the patch, if you make those changes, since 
>> Diego's looked it over and is happy, and since I don't see anything 
>> algorithm that looks too scary.  You also need to run the regression 
>> testsuite after you fix up the issues above, and you should always 
>> post the platform on which you tested with your submission.
>>
> Mark,
>
> Thanks for your review.
>
> I will try to get a new patch out today with all of the above changes 
> in it.  I am leaving for a week vacation tomorrow.
> This last issue in your review is somewhat problematic.  I will of 
> course do whatever is correct.  However I copied this code from 
> tree-ssa-operands.c at diego's request.  So if this is a bug, it is a 
> bug that is already latent in the compiler.
> A grep for \"memory\" yields a second hit in varasm.c that to my 
> (mostly untrained) eye looks like a similar mistake.
> I did not submit this to gcc bugs since things are not supposed to be 
> posted to two news groups but it you are correct, there is at least 
> one other bug (and possibly two) already in the compiler. 

Calling it a "bug" is probably an overly strong statement; the "strcmp" 
will work.  It is just inefficient.  So, we should fix the existing 
inefficiencies, and certainly not introduce new ones.

Thank,

-- 
Mark Mitchell
CodeSourcery, LLC
(916) 791-8304
mark@codesourcery.com