Skip to content

Commit dcc84db

Browse files
sanjoysanjoy
committed
Bugfix: SCEVExpander incorrectly marks increment operations as no-wrap
 (The change was landed in r230280 and caused the regression PR22674. This version contains a fix and a test-case for PR22674). When emitting the increment operation, SCEVExpander marks the operation as nuw or nsw based on the flags on the preincrement SCEV. This is incorrect because, for instance, it is possible that {-6,+,1} is <nuw> while {-6,+,1}+1 = {-5,+,1} is not. This change teaches SCEV to mark the increment as nuw/nsw only if it can explicitly prove that the increment operation won't overflow. Apart from the attached test case, another (more realistic) manifestation of the bug can be seen in Transforms/IndVarSimplify/pr20680.ll. Differential Revision: http://reviews.llvm.org/D7778 llvm-svn: 230533
1 parent f42e1ec commit dcc84db

File tree

10 files changed

+173
-10
lines changed

10 files changed

+173
-10
lines changed

‎llvm/lib/Analysis/ScalarEvolutionExpander.cpp

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1063,6 +1063,34 @@ static bool canBeCheaplyTransformed(ScalarEvolution &SE,
10631063
return false;
10641064
}
10651065

1066+
static bool IsIncrementNSW(ScalarEvolution &SE, const SCEVAddRecExpr *AR) {
1067+
if (!isa<IntegerType>(AR->getType()))
1068+
return false;
1069+
1070+
unsigned BitWidth = cast<IntegerType>(AR->getType())->getBitWidth();
1071+
Type *WideTy = IntegerType::get(AR->getType()->getContext(), BitWidth * 2);
1072+
const SCEV *Step = AR->getStepRecurrence(SE);
1073+
const SCEV *OpAfterExtend = SE.getAddExpr(SE.getSignExtendExpr(Step, WideTy),
1074+
SE.getSignExtendExpr(AR, WideTy));
1075+
const SCEV *ExtendAfterOp =
1076+
SE.getSignExtendExpr(SE.getAddExpr(AR, Step), WideTy);
1077+
return ExtendAfterOp == OpAfterExtend;
1078+
}
1079+
1080+
static bool IsIncrementNUW(ScalarEvolution &SE, const SCEVAddRecExpr *AR) {
1081+
if (!isa<IntegerType>(AR->getType()))
1082+
return false;
1083+
1084+
unsigned BitWidth = cast<IntegerType>(AR->getType())->getBitWidth();
1085+
Type *WideTy = IntegerType::get(AR->getType()->getContext(), BitWidth * 2);
1086+
const SCEV *Step = AR->getStepRecurrence(SE);
1087+
const SCEV *OpAfterExtend = SE.getAddExpr(SE.getZeroExtendExpr(Step, WideTy),
1088+
SE.getZeroExtendExpr(AR, WideTy));
1089+
const SCEV *ExtendAfterOp =
1090+
SE.getZeroExtendExpr(SE.getAddExpr(AR, Step), WideTy);
1091+
return ExtendAfterOp == OpAfterExtend;
1092+
}
1093+
10661094
/// getAddRecExprPHILiterally - Helper for expandAddRecExprLiterally. Expand
10671095
/// the base addrec, which is the addrec without any non-loop-dominating
10681096
/// values, and return the PHI.
@@ -1154,6 +1182,9 @@ SCEVExpander::getAddRecExprPHILiterally(const SCEVAddRecExpr *Normalized,
11541182
}
11551183
}
11561184

1185+
bool IncrementIsNUW = IsIncrementNUW(SE, Normalized);
1186+
bool IncrementIsNSW = IsIncrementNSW(SE, Normalized);
1187+
11571188
// Save the original insertion point so we can restore it when we're done.
11581189
BuilderType::InsertPointGuard Guard(Builder);
11591190

@@ -1213,10 +1244,11 @@ SCEVExpander::getAddRecExprPHILiterally(const SCEVAddRecExpr *Normalized,
12131244
IVIncInsertPos : Pred->getTerminator();
12141245
Builder.SetInsertPoint(InsertPos);
12151246
Value *IncV = expandIVInc(PN, StepV, L, ExpandTy, IntTy, useSubtract);
1247+
12161248
if (isa<OverflowingBinaryOperator>(IncV)) {
1217-
if (Normalized->getNoWrapFlags(SCEV::FlagNUW))
1249+
if (IncrementIsNUW)
12181250
cast<BinaryOperator>(IncV)->setHasNoUnsignedWrap();
1219-
if (Normalized->getNoWrapFlags(SCEV::FlagNSW))
1251+
if (IncrementIsNSW)
12201252
cast<BinaryOperator>(IncV)->setHasNoSignedWrap();
12211253
}
12221254
PN->addIncoming(IncV, Pred);

‎llvm/test/Analysis/ScalarEvolution/pr22674.ll

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
; RUN: opt -loop-reduce -S %s
2+
3+
4+
target datalayout = "e-m:e-p:32:32-i64:64-f80:128-n8:16:32:64-S128"
5+
target triple = "x86_64-pc-linux-gnux32"
6+
7+
%"class.llvm::AttributeSetNode.230.2029.3828.6141.6912.7683.8454.9482.9996.10253.18506" = type { %"class.llvm::FoldingSetImpl::Node.1.1801.3600.5913.6684.7455.8226.9254.9768.10025.18505", i32 }
8+
%"class.llvm::FoldingSetImpl::Node.1.1801.3600.5913.6684.7455.8226.9254.9768.10025.18505" = type { i8* }
9+
%"struct.std::pair.241.2040.3839.6152.6923.7694.8465.9493.10007.10264.18507" = type { i32, %"class.llvm::AttributeSetNode.230.2029.3828.6141.6912.7683.8454.9482.9996.10253.18506"* }
10+
%"class.llvm::Attribute.222.2021.3820.6133.6904.7675.8446.9474.9988.10245.18509" = type { %"class.llvm::AttributeImpl.2.1802.3601.5914.6685.7456.8227.9255.9769.10026.18508"* }
11+
%"class.llvm::AttributeImpl.2.1802.3601.5914.6685.7456.8227.9255.9769.10026.18508" = type <{ i32 (...)**, %"class.llvm::FoldingSetImpl::Node.1.1801.3600.5913.6684.7455.8226.9254.9768.10025.18505", i8, [3 x i8] }>
12+
13+
; Function Attrs: nounwind uwtable
14+
define void @_ZNK4llvm11AttrBuilder13hasAttributesENS_12AttributeSetEy() #0 align 2 {
15+
entry:
16+
br i1 undef, label %cond.false, label %_ZNK4llvm12AttributeSet11getNumSlotsEv.exit
17+
18+
_ZNK4llvm12AttributeSet11getNumSlotsEv.exit: ; preds = %entry
19+
br i1 undef, label %cond.false, label %for.body.lr.ph.for.body.lr.ph.split_crit_edge
20+
21+
for.body.lr.ph.for.body.lr.ph.split_crit_edge: ; preds = %_ZNK4llvm12AttributeSet11getNumSlotsEv.exit
22+
br label %land.lhs.true.i
23+
24+
land.lhs.true.i: ; preds = %for.inc, %for.body.lr.ph.for.body.lr.ph.split_crit_edge
25+
%I.099 = phi i32 [ 0, %for.body.lr.ph.for.body.lr.ph.split_crit_edge ], [ %inc, %for.inc ]
26+
%cmp.i = icmp ugt i32 undef, %I.099
27+
br i1 %cmp.i, label %_ZNK4llvm12AttributeSet12getSlotIndexEj.exit, label %cond.false.i.split
28+
29+
cond.false.i.split: ; preds = %land.lhs.true.i
30+
unreachable
31+
32+
_ZNK4llvm12AttributeSet12getSlotIndexEj.exit: ; preds = %land.lhs.true.i
33+
br i1 undef, label %for.end, label %for.inc
34+
35+
for.inc: ; preds = %_ZNK4llvm12AttributeSet12getSlotIndexEj.exit
36+
%inc = add i32 %I.099, 1
37+
br i1 undef, label %cond.false, label %land.lhs.true.i
38+
39+
for.end: ; preds = %_ZNK4llvm12AttributeSet12getSlotIndexEj.exit
40+
%I.099.lcssa129 = phi i32 [ %I.099, %_ZNK4llvm12AttributeSet12getSlotIndexEj.exit ]
41+
br i1 undef, label %cond.false, label %_ZNK4llvm12AttributeSet3endEj.exit
42+
43+
cond.false: ; preds = %for.end, %for.inc, %_ZNK4llvm12AttributeSet11getNumSlotsEv.exit, %entry
44+
unreachable
45+
46+
_ZNK4llvm12AttributeSet3endEj.exit: ; preds = %for.end
47+
%second.i.i.i = getelementptr inbounds %"struct.std::pair.241.2040.3839.6152.6923.7694.8465.9493.10007.10264.18507"* undef, i32 %I.099.lcssa129, i32 1
48+
%0 = load %"class.llvm::AttributeSetNode.230.2029.3828.6141.6912.7683.8454.9482.9996.10253.18506"** %second.i.i.i, align 4, !tbaa !2
49+
%NumAttrs.i.i.i = getelementptr inbounds %"class.llvm::AttributeSetNode.230.2029.3828.6141.6912.7683.8454.9482.9996.10253.18506"* %0, i32 0, i32 1
50+
%1 = load i32* %NumAttrs.i.i.i, align 4, !tbaa !8
51+
%add.ptr.i.i.i55 = getelementptr inbounds %"class.llvm::Attribute.222.2021.3820.6133.6904.7675.8446.9474.9988.10245.18509"* undef, i32 %1
52+
br i1 undef, label %return, label %for.body11
53+
54+
for.cond9: ; preds = %_ZNK4llvm9Attribute13getKindAsEnumEv.exit
55+
%cmp10 = icmp eq %"class.llvm::Attribute.222.2021.3820.6133.6904.7675.8446.9474.9988.10245.18509"* %incdec.ptr, %add.ptr.i.i.i55
56+
br i1 %cmp10, label %return, label %for.body11
57+
58+
for.body11: ; preds = %for.cond9, %_ZNK4llvm12AttributeSet3endEj.exit
59+
%I5.096 = phi %"class.llvm::Attribute.222.2021.3820.6133.6904.7675.8446.9474.9988.10245.18509"* [ %incdec.ptr, %for.cond9 ], [ undef, %_ZNK4llvm12AttributeSet3endEj.exit ]
60+
%2 = bitcast %"class.llvm::Attribute.222.2021.3820.6133.6904.7675.8446.9474.9988.10245.18509"* %I5.096 to i32*
61+
%3 = load i32* %2, align 4, !tbaa !10
62+
%tobool.i59 = icmp eq i32 %3, 0
63+
br i1 %tobool.i59, label %cond.false21, label %_ZNK4llvm9Attribute15isEnumAttributeEv.exit
64+
65+
_ZNK4llvm9Attribute15isEnumAttributeEv.exit: ; preds = %for.body11
66+
switch i8 undef, label %cond.false21 [
67+
i8 0, label %_ZNK4llvm9Attribute13getKindAsEnumEv.exit
68+
i8 1, label %_ZNK4llvm9Attribute13getKindAsEnumEv.exit
69+
i8 2, label %_ZNK4llvm9Attribute15getKindAsStringEv.exit
70+
]
71+
72+
_ZNK4llvm9Attribute13getKindAsEnumEv.exit: ; preds = %_ZNK4llvm9Attribute15isEnumAttributeEv.exit, %_ZNK4llvm9Attribute15isEnumAttributeEv.exit
73+
%incdec.ptr = getelementptr inbounds %"class.llvm::Attribute.222.2021.3820.6133.6904.7675.8446.9474.9988.10245.18509"* %I5.096, i32 1
74+
br i1 undef, label %for.cond9, label %return
75+
76+
cond.false21: ; preds = %_ZNK4llvm9Attribute15isEnumAttributeEv.exit, %for.body11
77+
unreachable
78+
79+
_ZNK4llvm9Attribute15getKindAsStringEv.exit: ; preds = %_ZNK4llvm9Attribute15isEnumAttributeEv.exit
80+
unreachable
81+
82+
return: ; preds = %_ZNK4llvm9Attribute13getKindAsEnumEv.exit, %for.cond9, %_ZNK4llvm12AttributeSet3endEj.exit
83+
ret void
84+
}
85+
86+
attributes #0 = { nounwind uwtable "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
87+
88+
!llvm.module.flags = !{!0}
89+
!llvm.ident = !{!1}
90+
91+
!0 = !{i32 1, !"PIC Level", i32 2}
92+
!1 = !{!"clang version 3.7.0 (ssh://[email protected]/export/server/git/llvm/clang 4c31740d4f81614b6d278c7825cfdae5a1c78799) (llvm/llvm.git b693958bd09144aed90312709a7e2ccf7124eb53)"}
93+
!2 = !{!3, !7, i64 4}
94+
!3 = !{!"_ZTSSt4pairIjPN4llvm16AttributeSetNodeEE", !4, i64 0, !7, i64 4}
95+
!4 = !{!"int", !5, i64 0}
96+
!5 = !{!"omnipotent char", !6, i64 0}
97+
!6 = !{!"Simple C/C++ TBAA"}
98+
!7 = !{!"any pointer", !5, i64 0}
99+
!8 = !{!9, !4, i64 4}
100+
!9 = !{!"_ZTSN4llvm16AttributeSetNodeE", !4, i64 4}
101+
!10 = !{!7, !7, i64 0}

‎llvm/test/Analysis/ScalarEvolution/scev-expander-incorrect-nowrap.ll

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
; RUN: opt -indvars -S < %s | FileCheck %s
2+
3+
declare void @use(i32)
4+
declare void @use.i8(i8)
5+
6+
define void @f() {
7+
; CHECK-LABEL: @f
8+
entry:
9+
br label %loop
10+
11+
loop:
12+
; The only use for idx.mirror is to induce an nuw for %idx. It does
13+
; not induce an nuw for %idx.inc
14+
%idx.mirror = phi i8 [ -6, %entry ], [ %idx.mirror.inc, %loop ]
15+
%idx = phi i8 [ -5, %entry ], [ %idx.inc, %loop ]
16+
17+
%idx.sext = sext i8 %idx to i32
18+
call void @use(i32 %idx.sext)
19+
20+
%idx.mirror.inc = add nuw i8 %idx.mirror, 1
21+
call void @use.i8(i8 %idx.mirror.inc)
22+
23+
%idx.inc = add i8 %idx, 1
24+
; CHECK-NOT: %indvars.iv.next = add nuw nsw i32 %indvars.iv, 1
25+
%cmp = icmp ugt i8 %idx.inc, 0
26+
br i1 %cmp, label %loop, label %exit
27+
28+
exit:
29+
ret void
30+
}

‎llvm/test/Analysis/ScalarEvolution/zext-signed-addrec.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ if.end: ; preds = %if.end, %for.cond1.
4343
%shl = and i32 %conv7, 510
4444
store i32 %shl, i32* @c, align 4
4545

46-
; CHECK: %lsr.iv.next = add i32 %lsr.iv, -258
46+
; CHECK: %lsr.iv.next = add nsw i32 %lsr.iv, -258
4747
%dec = add i8 %2, -1
4848

4949
%cmp2 = icmp sgt i8 %dec, -1

‎llvm/test/CodeGen/AArch64/arm64-scaled_iv.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ for.body: ; preds = %for.body, %entry
2020
%arrayidx = getelementptr inbounds double* %b, i64 %tmp
2121
%tmp1 = load double* %arrayidx, align 8
2222
; The induction variable should carry the scaling factor: 1 * 8 = 8.
23-
; CHECK: [[IVNEXT]] = add nuw i64 [[IV]], 8
23+
; CHECK: [[IVNEXT]] = add nuw nsw i64 [[IV]], 8
2424
%indvars.iv.next = add i64 %indvars.iv, 1
2525
%arrayidx2 = getelementptr inbounds double* %c, i64 %indvars.iv.next
2626
%tmp2 = load double* %arrayidx2, align 8

‎llvm/test/CodeGen/X86/avoid_complex_am.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ for.body: ; preds = %for.body, %entry
2222
%arrayidx = getelementptr inbounds double* %b, i64 %tmp
2323
%tmp1 = load double* %arrayidx, align 8
2424
; The induction variable should carry the scaling factor: 1.
25-
; CHECK: [[IVNEXT]] = add nuw i64 [[IV]], 1
25+
; CHECK: [[IVNEXT]] = add nuw nsw i64 [[IV]], 1
2626
%indvars.iv.next = add i64 %indvars.iv, 1
2727
%arrayidx2 = getelementptr inbounds double* %c, i64 %indvars.iv.next
2828
%tmp2 = load double* %arrayidx2, align 8

‎llvm/test/Transforms/IndVarSimplify/overflowcheck.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ target triple = "x86_64-apple-macosx"
99
; CHECK: @llvm.sadd.with.overflow
1010
; CHECK-LABEL: loop2:
1111
; CHECK-NOT: extractvalue
12-
; CHECK: add nuw nsw
12+
; CHECK: add nuw
1313
; CHECK: @llvm.sadd.with.overflow
1414
; CHECK-LABEL: loop3:
1515
; CHECK-NOT: extractvalue

‎llvm/test/Transforms/IndVarSimplify/pr20680.ll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -204,8 +204,8 @@ for.cond2.for.inc13_crit_edge: ; preds = %for.cond2.for.inc13
204204
br label %for.inc13
205205

206206
; CHECK: [[for_inc13]]:
207-
; CHECK-NEXT: %[[indvars_iv_next]] = add nuw nsw i32 %[[indvars_iv]], 1
208-
; CHECK-NEXT: %[[exitcond4:.*]] = icmp ne i32 %[[indvars_iv]], -1
207+
; CHECK-NEXT: %[[indvars_iv_next]] = add nsw i32 %[[indvars_iv]], 1
208+
; CHECK-NEXT: %[[exitcond4:.*]] = icmp ne i32 %[[indvars_iv_next]], 0
209209
; CHECK-NEXT: br i1 %[[exitcond4]], label %[[for_cond2_preheader]], label %[[for_end15:.*]]
210210
for.inc13: ; preds = %for.cond2.for.inc13_crit_edge, %for.cond2.preheader
211211
%inc14 = add i8 %storemerge15, 1

‎llvm/test/Transforms/LoopStrengthReduce/count-to-zero.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ bb3: ; preds = %bb1
1919
%tmp4 = add i32 %c_addr.1, -1 ; <i32> [#uses=1]
2020
%c_addr.1.be = select i1 %tmp2, i32 %tmp3, i32 %tmp4 ; <i32> [#uses=1]
2121
%indvar.next = add i32 %indvar, 1 ; <i32> [#uses=1]
22-
; CHECK: add i32 %lsr.iv, -1
22+
; CHECK: add nsw i32 %lsr.iv, -1
2323
br label %bb6
2424

2525
bb6: ; preds = %bb3, %entry

‎llvm/test/Transforms/LoopStrengthReduce/uglygep.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ bb:
5959
; CHECK: loop0:
6060
; Induction variable is initialized to -2.
6161
; CHECK-NEXT: [[PHIIV:%[^ ]+]] = phi i32 [ [[IVNEXT:%[^ ]+]], %loop0 ], [ -2, %bb ]
62-
; CHECK-NEXT: [[IVNEXT]] = add i32 [[PHIIV]], 1
62+
; CHECK-NEXT: [[IVNEXT]] = add nuw nsw i32 [[PHIIV]], 1
6363
; CHECK-NEXT: br i1 false, label %loop0, label %bb0
6464
loop0: ; preds = %loop0, %bb
6565
%i0 = phi i32 [ %i0.next, %loop0 ], [ 0, %bb ] ; <i32> [#uses=2]

0 commit comments

Comments
 (0)