[llvm] r176022 - Fix the root cause of PR15348 by correctly handling alignment 0 on

Lang Hames lhames at gmail.com
Tue Feb 26 17:08:00 PST 2013


I should definitely be preserving the alignment of the first member of the
memcpy. If my patch is spitting out alignment-0 memcpys that sounds like a
bug - I'll look in to it.

- Lang.


On Tue, Feb 26, 2013 at 4:56 PM, Evan Cheng <evan.cheng at apple.com> wrote:

>
> On Feb 25, 2013, at 6:20 AM, Chandler Carruth <chandlerc at gmail.com> wrote:
>
> > Author: chandlerc
> > Date: Mon Feb 25 08:20:21 2013
> > New Revision: 176022
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=176022&view=rev
> > Log:
> > Fix the root cause of PR15348 by correctly handling alignment 0 on
> > memory intrinsics in the SDAG builder.
> >
> > When alignment is zero, the lang ref says that *no* alignment
> > assumptions can be made. This is the exact opposite of the internal API
> > contracts of the DAG where alignment 0 indicates that the alignment can
> > be made to be anything desired.
> >
> > There is another, more explicit alignment that is better suited for the
> > role of "no alignment at all": an alignment of 1. Map the intrinsic
> > alignment to this early so that we don't end up generating aligned DAGs.
> >
> > It is really terrifying that we've never seen this before, but we
> > suddenly started generating a large number of alignment 0 memcpys due to
> > the new code to do memcpy-based copying of POD class members. That patch
> > contains a bug that rounds bitfield alignments down when they are the
> > first field. This can in turn produce zero alignments.
>
> Is this a bug in clang? If memcpy of alignment 0 ended up generating poor
> code it defeats the purpose the clang optimization. Lang?
>
> Evan
>
> >
> > This fixes weird crashes I've seen in library users of LLVM on 32-bit
> > hosts, etc.
> >
> > Modified:
> >    llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
> >    llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
> >    llvm/trunk/test/CodeGen/X86/memcpy.ll
> >    llvm/trunk/test/CodeGen/X86/memset.ll
> >
> > Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp?rev=176022&r1=176021&r2=176022&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (original)
> > +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp Mon Feb 25
> 08:20:21 2013
> > @@ -3867,6 +3867,7 @@ SDValue SelectionDAG::getMemcpy(SDValue
> >                                 unsigned Align, bool isVol, bool
> AlwaysInline,
> >                                 MachinePointerInfo DstPtrInfo,
> >                                 MachinePointerInfo SrcPtrInfo) {
> > +  assert(Align && "The SDAG layer expects explicit alignment and
> reservers 0");
> >
> >   // Check to see if we should lower the memcpy to loads and stores
> first.
> >   // For cases within the target-specified limits, this is the best
> choice.
> > @@ -3934,6 +3935,7 @@ SDValue SelectionDAG::getMemmove(SDValue
> >                                  unsigned Align, bool isVol,
> >                                  MachinePointerInfo DstPtrInfo,
> >                                  MachinePointerInfo SrcPtrInfo) {
> > +  assert(Align && "The SDAG layer expects explicit alignment and
> reservers 0");
> >
> >   // Check to see if we should lower the memmove to loads and stores
> first.
> >   // For cases within the target-specified limits, this is the best
> choice.
> > @@ -3988,6 +3990,7 @@ SDValue SelectionDAG::getMemset(SDValue
> >                                 SDValue Src, SDValue Size,
> >                                 unsigned Align, bool isVol,
> >                                 MachinePointerInfo DstPtrInfo) {
> > +  assert(Align && "The SDAG layer expects explicit alignment and
> reservers 0");
> >
> >   // Check to see if we should lower the memset to stores first.
> >   // For cases within the target-specified limits, this is the best
> choice.
> >
> > Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp?rev=176022&r1=176021&r2=176022&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
> (original)
> > +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp Mon Feb
> 25 08:20:21 2013
> > @@ -4467,6 +4467,8 @@ SelectionDAGBuilder::visitIntrinsicCall(
> >     SDValue Op2 = getValue(I.getArgOperand(1));
> >     SDValue Op3 = getValue(I.getArgOperand(2));
> >     unsigned Align =
> cast<ConstantInt>(I.getArgOperand(3))->getZExtValue();
> > +    if (!Align)
> > +      Align = 1; // @llvm.memcpy defines 0 and 1 to both mean no
> alignment.
> >     bool isVol = cast<ConstantInt>(I.getArgOperand(4))->getZExtValue();
> >     DAG.setRoot(DAG.getMemcpy(getRoot(), dl, Op1, Op2, Op3, Align,
> isVol, false,
> >                               MachinePointerInfo(I.getArgOperand(0)),
> > @@ -4483,6 +4485,8 @@ SelectionDAGBuilder::visitIntrinsicCall(
> >     SDValue Op2 = getValue(I.getArgOperand(1));
> >     SDValue Op3 = getValue(I.getArgOperand(2));
> >     unsigned Align =
> cast<ConstantInt>(I.getArgOperand(3))->getZExtValue();
> > +    if (!Align)
> > +      Align = 1; // @llvm.memset defines 0 and 1 to both mean no
> alignment.
> >     bool isVol = cast<ConstantInt>(I.getArgOperand(4))->getZExtValue();
> >     DAG.setRoot(DAG.getMemset(getRoot(), dl, Op1, Op2, Op3, Align, isVol,
> >                               MachinePointerInfo(I.getArgOperand(0))));
> > @@ -4500,6 +4504,8 @@ SelectionDAGBuilder::visitIntrinsicCall(
> >     SDValue Op2 = getValue(I.getArgOperand(1));
> >     SDValue Op3 = getValue(I.getArgOperand(2));
> >     unsigned Align =
> cast<ConstantInt>(I.getArgOperand(3))->getZExtValue();
> > +    if (!Align)
> > +      Align = 1; // @llvm.memmove defines 0 and 1 to both mean no
> alignment.
> >     bool isVol = cast<ConstantInt>(I.getArgOperand(4))->getZExtValue();
> >     DAG.setRoot(DAG.getMemmove(getRoot(), dl, Op1, Op2, Op3, Align,
> isVol,
> >                                MachinePointerInfo(I.getArgOperand(0)),
> >
> > Modified: llvm/trunk/test/CodeGen/X86/memcpy.ll
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/memcpy.ll?rev=176022&r1=176021&r2=176022&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/test/CodeGen/X86/memcpy.ll (original)
> > +++ llvm/trunk/test/CodeGen/X86/memcpy.ll Mon Feb 25 08:20:21 2013
> > @@ -105,3 +105,16 @@ entry:
> >   ret void
> > }
> >
> > +define void @PR15348(i8* %a, i8* %b) {
> > +; Ensure that alignment of '0' in an @llvm.memcpy intrinsic results in
> > +; unaligned loads and stores.
> > +; LINUX: PR15348
> > +; LINUX: movb
> > +; LINUX: movb
> > +; LINUX: movq
> > +; LINUX: movq
> > +; LINUX: movq
> > +; LINUX: movq
> > +  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %a, i8* %b, i64 17, i32 0,
> i1 false)
> > +  ret void
> > +}
> >
> > Modified: llvm/trunk/test/CodeGen/X86/memset.ll
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/memset.ll?rev=176022&r1=176021&r2=176022&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/test/CodeGen/X86/memset.ll (original)
> > +++ llvm/trunk/test/CodeGen/X86/memset.ll Mon Feb 25 08:20:21 2013
> > @@ -20,15 +20,18 @@ entry:
> > ; X86: movl $0,
> > ; X86: movl $0,
> > ; X86-NOT: movl $0,
> > +; X86: ret
> >
> > ; XMM: xorps %xmm{{[0-9]+}}, [[Z:%xmm[0-9]+]]
> > ; XMM: movaps [[Z]],
> > ; XMM: movaps [[Z]],
> > ; XMM-NOT: movaps
> > +; XMM: ret
> >
> > ; YMM: vxorps %ymm{{[0-9]+}}, %ymm{{[0-9]+}}, [[Z:%ymm[0-9]+]]
> > ; YMM: vmovaps [[Z]],
> > ; YMM-NOT: movaps
> > +; YMM: ret
> >
> >       call void @foo( %struct.x* %up_mvd116 ) nounwind
> >       ret void
> > @@ -37,3 +40,16 @@ entry:
> > declare void @foo(%struct.x*)
> >
> > declare void @llvm.memset.p0i8.i64(i8* nocapture, i8, i64, i32, i1)
> nounwind
> > +
> > +define void @PR15348(i8* %a) {
> > +; Ensure that alignment of '0' in an @llvm.memset intrinsic results in
> > +; unaligned loads and stores.
> > +; XMM: PR15348
> > +; XMM: movb $0,
> > +; XMM: movl $0,
> > +; XMM: movl $0,
> > +; XMM: movl $0,
> > +; XMM: movl $0,
> > +  call void @llvm.memset.p0i8.i64(i8* %a, i8 0, i64 17, i32 0, i1 false)
> > +  ret void
> > +}
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
> _______________________________________________
> 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/20130226/fec4c6b5/attachment.html>


More information about the llvm-commits mailing list