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

Shuxin Yang shuxin.llvm at gmail.com
Sat Apr 13 23:54:26 PDT 2013


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?

> 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.



More information about the llvm-commits mailing list