Skip to content
This repository was archived by the owner on Feb 5, 2019. It is now read-only.

Commit 0dee675

Browse files
committed
SROA: Only split loads on byte boundaries
r199771 accidently broke the logic that makes sure that SROA only splits load on byte boundaries. If such a split happens, some bits get lost when reassembling loads of wider types, causing data corruption. Move the width check up to reject such splits early, avoiding the corruption. Fixes PR19250. Patch by: Björn Steinbrink <[email protected]> git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@211082 91177308-0d34-0410-b5e6-96231b3b80d8
1 parent 84fea77 commit 0dee675

File tree

3 files changed

+69
-5
lines changed

3 files changed

+69
-5
lines changed

lib/Transforms/Scalar/SROA.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1032,11 +1032,6 @@ static Type *findCommonType(AllocaSlices::const_iterator B,
10321032
UserTy = SI->getValueOperand()->getType();
10331033
}
10341034

1035-
if (!UserTy || (Ty && Ty != UserTy))
1036-
TyIsCommon = false; // Give up on anything but an iN type.
1037-
else
1038-
Ty = UserTy;
1039-
10401035
if (IntegerType *UserITy = dyn_cast_or_null<IntegerType>(UserTy)) {
10411036
// If the type is larger than the partition, skip it. We only encounter
10421037
// this for split integer operations where we want to use the type of the
@@ -1051,6 +1046,13 @@ static Type *findCommonType(AllocaSlices::const_iterator B,
10511046
if (!ITy || ITy->getBitWidth() < UserITy->getBitWidth())
10521047
ITy = UserITy;
10531048
}
1049+
1050+
// To avoid depending on the order of slices, Ty and TyIsCommon must not
1051+
// depend on types skipped above.
1052+
if (!UserTy || (Ty && Ty != UserTy))
1053+
TyIsCommon = false; // Give up on anything but an iN type.
1054+
else
1055+
Ty = UserTy;
10541056
}
10551057

10561058
return TyIsCommon ? Ty : ITy;
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
; RUN: opt < %s -sroa -S | FileCheck %s
2+
target datalayout = "e-p:64:64:64-p1:16:16:16-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-n8:16:32:64"
3+
4+
declare void @llvm.memcpy.p0i8.p0i8.i32(i8* nocapture, i8* nocapture, i32, i32, i1) nounwind
5+
6+
; Check that the chosen type for a split is independent from the order of
7+
; slices even in case of types that are skipped because their width is not a
8+
; byte width multiple
9+
define void @skipped_inttype_first({ i16*, i32 }*) {
10+
; CHECK-LABEL: @skipped_inttype_first
11+
; CHECK: alloca i8*
12+
%arg = alloca { i16*, i32 }, align 8
13+
%2 = bitcast { i16*, i32 }* %0 to i8*
14+
%3 = bitcast { i16*, i32 }* %arg to i8*
15+
call void @llvm.memcpy.p0i8.p0i8.i32(i8* %3, i8* %2, i32 16, i32 8, i1 false)
16+
%b = getelementptr inbounds { i16*, i32 }* %arg, i64 0, i32 0
17+
%pb0 = bitcast i16** %b to i63*
18+
%b0 = load i63* %pb0
19+
%pb1 = bitcast i16** %b to i8**
20+
%b1 = load i8** %pb1
21+
ret void
22+
}
23+
24+
define void @skipped_inttype_last({ i16*, i32 }*) {
25+
; CHECK-LABEL: @skipped_inttype_last
26+
; CHECK: alloca i8*
27+
%arg = alloca { i16*, i32 }, align 8
28+
%2 = bitcast { i16*, i32 }* %0 to i8*
29+
%3 = bitcast { i16*, i32 }* %arg to i8*
30+
call void @llvm.memcpy.p0i8.p0i8.i32(i8* %3, i8* %2, i32 16, i32 8, i1 false)
31+
%b = getelementptr inbounds { i16*, i32 }* %arg, i64 0, i32 0
32+
%pb1 = bitcast i16** %b to i8**
33+
%b1 = load i8** %pb1
34+
%pb0 = bitcast i16** %b to i63*
35+
%b0 = load i63* %pb0
36+
ret void
37+
}

test/Transforms/SROA/slice-width.ll

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
; RUN: opt < %s -sroa -S | FileCheck %s
2+
target datalayout = "e-p:64:64:64-p1:16:16:16-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-n8:16:32:64"
3+
4+
declare void @llvm.memcpy.p0i8.p0i8.i32(i8* nocapture, i8* nocapture, i32, i32, i1) nounwind
5+
6+
define void @no_split_on_non_byte_width(i32) {
7+
; This tests that allocas are not split into slices that are not byte width multiple
8+
%arg = alloca i32 , align 8
9+
store i32 %0, i32* %arg
10+
br label %load_i32
11+
12+
load_i32:
13+
; CHECK-LABEL: load_i32:
14+
; CHECK-NOT: bitcast {{.*}} to i1
15+
; CHECK-NOT: zext i1
16+
%r0 = load i32* %arg
17+
br label %load_i1
18+
19+
load_i1:
20+
; CHECK-LABEL: load_i1:
21+
; CHECK: bitcast {{.*}} to i1
22+
%p1 = bitcast i32* %arg to i1*
23+
%t1 = load i1* %p1
24+
ret void
25+
}

0 commit comments

Comments
 (0)