[llvm] r176408 - recommit r172363 & r171325 (reverted in r172756)

Arnold Schwaighofer aschwaighofer at apple.com
Sun Mar 3 15:29:36 PST 2013


On Mar 3, 2013, at 4:52 PM, Shuxin Yang <shuxin.llvm at gmail.com> wrote:

>> 
>> 
>> Shuxin: I'm not breaking any assumption here.
> If you don't think you break the assumption,  tell us your immediate reaction to
> the class name "ObjectSizeOffsetVisitor".
> 
> It seem so odd to me that  xxx_ObjectWhatever() return a meaningful value however we have no idea which
> object it is talking about.
> 
> >If you notice, we already do a similar analysis for select instructions.
> Show me the code.
> On the other hand,  there are some code showing similar usage
> dose not necessarily mean that piece of code is correct, or it is good practice to write the code this way.
> 
> This is contrived usage of these functions:
>  p1 = &object1.field1
>  p2 = &object2.field2
>  p3 = phi(p1, p2);
> 
>  get-object-offset() would return offset(object.filed1).  the optimizer randomly take object1 as
> the "underlying object", and further analysis reveals that the location of &object1.field1 is either used as
> char or int32, a contrived type-based alias alias would mistakenly make a conclusion that *p3
> will not alias a load of floating point type, which could be wrong because object2.field2 could be of fp type.
> 
>  In my option, it better to hack for same specific cases than to convert a common function into a
> dangerous, error-prone one.

Shuxin, I think your concern could be addressed by introducing a family of functions that make their semantics very clear:

// Returns the size if there is a set of identified underlying objects
// and they all share the same size. UnknownSize otherwise.
getSizeOfUnderlyingObjects()
// Returns the pointer adjusted size if there is a set of identified underlying objects
// and the pointer adjusted values all share the same size. UnknownSize otherwise.
getSizeOfPointerAdjustedUnderlyingObjects()

// Returns the size of an underlying object if the underlying object can
// be identified as a unique identified underlying object.
getSizeOfUniqueUnderlyingObject()
// Returns the size of an pointer adjusted underlying object if the underlying
// object can be identified as a unique identified underlying object.
getSizeOfPointerAdjustedUniqueUnderlyingObject()

And the proper usage within our code base.

There remains the concern about the semantics of "llvm.objectsize.i32/i64":
  Which of above functions to you map it to. See my previous email.

(Possibly utility classes like ObjectOffsetVisitor will also have to be renamed to reflect what they do)

Best,
Arnold




More information about the llvm-commits mailing list