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