[llvm] r183584 - Fix an assertion in MemCpyOpt pass.

[Shuxin Yang]

On 6/7/13 4:39 PM, Eli Friedman wrote:
> On Fri, Jun 7, 2013 at 3:45 PM, Shuxin Yang <shuxin.llvm at gmail.com 
> <mailto:shuxin.llvm at gmail.com>> wrote:
>
>     Author: shuxin_yang
>     Date: Fri Jun  7 17:45:21 2013
>     New Revision: 183584
>
>     URL: http://llvm.org/viewvc/llvm-project?rev=183584&view=rev
>     Log:
>       Fix an assertion in MemCpyOpt pass.
>
>       The MemCpyOpt pass is capable of optimizing:
>           callee(&S); copy N bytes from S to D.
>         into:
>           callee(&D);
>     subject to some legality constraints.
>
>       Assertion is triggered when the compiler tries to evalute
>     "sizeof(typeof(D))",
>     while D is an opaque-typed, 'sret' formal argument of function
>     being compiled.
>     i.e. the signature of the func being compiled is something like this:
>       T caller(...,%opaque* noalias nocapture sret %D, ...)
>
>       The fix is that when come across such situation, instead of
>     calling some
>     utility functions to get the size of D's type (which will crash),
>     we simply
>     assume D has at least N bytes as implified by the copy-instruction.
>
>     rdar://14073661
>
>     Modified:
>         llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp
>         llvm/trunk/test/Transforms/MemCpyOpt/memcpy.ll
>
>     Modified: llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp
>     URL:
>     http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp?rev=183584&r1=183583&r2=183584&view=diff
>     ==============================================================================
>     --- llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp (original)
>     +++ llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp Fri Jun
>      7 17:45:21 2013
>     @@ -626,8 +626,10 @@ bool MemCpyOpt::performCallSlotOptzn(Ins
>            return false;
>
>          Type *StructTy =
>     cast<PointerType>(A->getType())->getElementType();
>     -    uint64_t destSize = TD->getTypeAllocSize(StructTy);
>     -
>     +    // If StructTy is an opaque type, it should have at least
>     <cpyLen> bytes,
>     +    // as implified by the copy-instruction.
>     +    uint64_t destSize = StructTy->isSized() ?
>     +                        TD->getTypeAllocSize(StructTy) : cpyLen;
>          if (destSize < srcSize)
>            return false;
>
>
> Why is it safe to assume StructTy is at least cpyLen bytes?
If not, why is it possible copying cpyLen byte from (part of) src to Dest?

> And if it is safe, why is this check necessary at all?
>
I guess you confuse cpyLen with SrcLen.
I asked the same question when I read the code. After the 2nd thought, I 
think it is necessary.
You never know how the callee access the Src, it could access the last 
byte (the SrcSize-th byte) of the object
corresponding to the Src, and cpyLen < SrcSize.




-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130607/606ab12e/attachment.html>