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

[Yi Jiang]

Hi Nuno,

Can you take a look at bug 15540? It seems that this commit result in 
10% GVN pass time regressions. Please let me know if you need more 
information. Thank you.

-Yi

On 3/19/13, 10:01 AM, Shuxin Yang wrote:
>
>
>
> -------- Original Message --------
> Subject: 	Re: [llvm] r176408 - recommit r172363 & r171325 (reverted in 
> r172756)
> Date: 	Sun, 03 Mar 2013 15:45:00 -0800
> From: 	Shuxin Yang <shuxin.llvm at gmail.com>
> To: 	Arnold Schwaighofer <aschwaighofer at apple.com>
> CC: 	Nuno Lopes <nunoplopes at sapo.pt>, llvm-commits at cs.uiuc.edu
>
>
>
> Hi, Arnold:
>
>    You are right. As I put in the report to the pr14988, my concerns
> have two parts:
>     1) in alias analysis context, while Nuno's change dose not seem
> correctness problem for what we have today.
>         It likely has problem in the future, as it deviate the original
> meaning of class ObjectSizeOffsetVisitor.
>         I gave a contrived problem in the previous mail.
>
>         What you proposed here is that : if Nuno's change have to be
> committed, we can remedy this concern by
>         coming up another set of functions, each having *CLEAR* meaning,
> and semantics.
>
>         That is absolutely right, but I don't think it's worth that
> trouble.
>
>     2) in the context llvm.objectsize related optimization.  I'm not
> expert of the llvm lang-ref.
>          However, IMHO, even if it has some loophole in the lang spec,
> we should not take this advantage to
>          code hard to read or maintain.
>
> .
>
> > 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
> >
>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130319/30ef43d9/attachment.html>