[llvm-commits] [llvm] r164641 - in /llvm/trunk: lib/Transforms/Scalar/SROA.cpp test/Transforms/SROA/basictest.ll

[Nick Lewycky]

On 25 September 2012 18:10, Eli Friedman <eli.friedman at gmail.com> wrote:

> On Tue, Sep 25, 2012 at 3:46 PM, Nick Lewycky <nicholas at mxc.ca> wrote:
> > Author: nicholas
> > Date: Tue Sep 25 17:46:21 2012
> > New Revision: 164641
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=164641&view=rev
> > Log:
> > Don't drop the alignment on a memcpy intrinsic when producing a store.
> This is
> > only a missed optimization opportunity if the store is over-aligned, but
> a
> > miscompile if the store's new type has a higher natural alignment than
> the
> > memcpy did. Fixes PR13920!
> >
> > Modified:
> >     llvm/trunk/lib/Transforms/Scalar/SROA.cpp
> >     llvm/trunk/test/Transforms/SROA/basictest.ll
> >
> > Modified: llvm/trunk/lib/Transforms/Scalar/SROA.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/SROA.cpp?rev=164641&r1=164640&r2=164641&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/Transforms/Scalar/SROA.cpp (original)
> > +++ llvm/trunk/lib/Transforms/Scalar/SROA.cpp Tue Sep 25 17:46:21 2012
> > @@ -2272,8 +2272,9 @@
> >                                      getName(".insert"));
> >      }
> >
> > -    Value *Store = IRB.CreateStore(Src, DstPtr, II.isVolatile());
> > -    (void)Store;
> > +    StoreInst *Store = cast<StoreInst>(IRB.CreateStore(Src, DstPtr,
> > +
> II.isVolatile()));
> > +    Store->setAlignment(II.getAlignment());
>
> This isn't quite right: "0" alignment on a store has a different
> meaning from "0" alignment on a memcpy.
>

Well spotted, I was not aware of the difference! However, I looked at
LangRef and I disagree with your analysis:

store: "A value of 0 or an omitted "align" argument means that the
operation has the preferential alignment for the target."

memcpy: "If the call to this intrinsic has an alignment value that is not 0
or 1, then the caller guarantees that both the source and destination
pointers are aligned to that boundary."

So transforming zero to zero is fine as they both mean preferred alignment.
However, a memcpy with align 1 also means preferred alignment whereas with
store it means unaligned. Turning a memcpy with alignment 1 into a store
with alignment 1 is a missed optz'n, but not a miscompile.

Nick



>
> -Eli
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120925/e2b95918/attachment.html>