[llvm] r179111 - Revert r176408 and r176408 to address PR15540.

[Nuno Lopes]

> On 4/13/13 11:43 PM, Nuno Lopes wrote:
>>> On 4/13/13 11:19 PM, Nuno Lopes wrote:
>>>> Hi,
>>>>
>>>> In summary, I would like to reapply r176407, yes.
>>>> The reason is that it is the correct fix for PR15540. This new 
>>>> function, getUnderlyingObjectSize(), gives exactly the semantics that 
>>>> BasicAA wants (which is to get the size of the object pointed by a 
>>>> given pointer).
>>> To be pedantic, BasicAA need min-size-of-all-potential-object pointed by 
>>> the pointer in question.
>>
>> Sure, the precision of the analysis could be improved. The same goes with 
>> constant folding expressions involving __builtin_object_size().
> If you care the __builtin_object_size(), you certainly can improve it 
> without impairing alias's cost.
> Our team(s) are *VERY* picky at compile time,  please don't increase 
> alias's cost without any good reason.
>
>> At this point, the only analysis we have is this one, that compute exact 
>> object sizes instead of ranges.
>>
>>
>>>> The function getObjectSize(), on the other hand, gives the size of an 
>>>> object minus the offset of the given pointer. Using getObjectSize() is 
>>>> therefore wrong (I wrote that code, but later I realized my mistake).
>>> Before the r176407, the compiler avoid the problem by *NOT* evaluating 
>>> the size if the pointer does not points to a object.
>>
>> Analyzing phi nodes is orthogonal to getUnderlyingObjectSize(). Please 
>> don't mix the two.
> I recall the getUnderlyingObjectSize() call get.*Offset(), which walk 
> along the U-D chain. No?

Yes. But getObjectSize() does exactly the same thing.  The problem is that 
it became noticeable after introducing the analysis of phi nodes, and before 
it was just bailing out sooner.


>> What you can say is that the analysis was improved to be more aggressive 
>> by analyzing phi nodes, which triggered the *latent* bug in the LTO 
>> build.
>>
>>
>>>> I believe r176407 is the correct fix, and it should be reapplied.
>>>>
>>> Then come up an example to prove r176407-1 dose not work.
>>
>> I believe your patch does work, since you restrict the analysis to 
>> pointers pointing to a single object.  But that's overly restrictive, and 
>> inhibits further improvements.
>
> I'm wondering how many times we come across a case where a pointer points 
> to difficult "objects" having the same size.
> If you can show us data to justify this cost, that is certainly great.
>
> The missed the opportunity can picked up by other analyses, whose result 
> are cached, and can be queried with low cost.
>
> However, calling get.*...Size() is different story, each time alias query 
> is called, it has invoke this function. We should keep it cheap.

Yes, agreed.  But I want to extend the analysis to provide ranges.  For AA 
it's probably irrelevant, but it is a huge help for __builtin_object_size() 
folding.
The lesson (for me) is that first I need to convert this whole thing into a 
proper analysis so that the results can be cached across passes.  And it may 
make sense to have different precisions for different clients.

Nuno