Skip to content

Commit

Permalink
Fix the root cause of PR15348 by correctly handling alignment 0 on
Browse files Browse the repository at this point in the history
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.

This fixes weird crashes I've seen in library users of LLVM on 32-bit
hosts, etc.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@176022 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
chandlerc committed Feb 25, 2013
1 parent de89ecd commit af23f8e
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 0 deletions.
3 changes: 3 additions & 0 deletions lib/CodeGen/SelectionDAG/SelectionDAG.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3867,6 +3867,7 @@ SDValue SelectionDAG::getMemcpy(SDValue Chain, DebugLoc dl, SDValue Dst,
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.
Expand Down Expand Up @@ -3934,6 +3935,7 @@ SDValue SelectionDAG::getMemmove(SDValue Chain, DebugLoc dl, SDValue Dst,
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.
Expand Down Expand Up @@ -3988,6 +3990,7 @@ SDValue SelectionDAG::getMemset(SDValue Chain, DebugLoc dl, SDValue Dst,
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.
Expand Down
6 changes: 6 additions & 0 deletions lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4467,6 +4467,8 @@ SelectionDAGBuilder::visitIntrinsicCall(const CallInst &I, unsigned Intrinsic) {
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)),
Expand All @@ -4483,6 +4485,8 @@ SelectionDAGBuilder::visitIntrinsicCall(const CallInst &I, unsigned Intrinsic) {
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))));
Expand All @@ -4500,6 +4504,8 @@ SelectionDAGBuilder::visitIntrinsicCall(const CallInst &I, unsigned Intrinsic) {
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)),
Expand Down
13 changes: 13 additions & 0 deletions test/CodeGen/X86/memcpy.ll
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
16 changes: 16 additions & 0 deletions test/CodeGen/X86/memset.ll
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}

0 comments on commit af23f8e

Please sign in to comment.