Skip to content

Commit 696d4ae

Browse files
Mark SeabornRichard Diamond
Mark Seaborn
authored and
Richard Diamond
committed
PNaCl: Disable parts of InstCombine that introduce *.with.overflow intrinsics
Change the PNaCl ABI checker to disallow these intrinsics. Note that I had originally intended to commit this before my earlier change (https://codereview.chromium.org/15688011) that enables the ExpandArithWithOverflow pass. Enabling ExpandArithWithOverflow without changing InstCombine causes ExpandArithWithOverflow to fail on some of the *.with.overflow intrinsic calls that InstCombine introduces. This change therefore fixes some breakage. BUG=https://code.google.com/p/nativeclient/issues/detail?id=3434 TEST=PNaCl toolchain trybots + GCC torture tests + LLVM test suite Review URL: https://codereview.chromium.org/16042011 (cherry picked from commit 8d01804) Conflicts: test/Transforms/InstCombine/overflow.ll
1 parent e8317eb commit 696d4ae

File tree

3 files changed

+60
-37
lines changed

3 files changed

+60
-37
lines changed

‎lib/Analysis/NaCl/PNaClABIVerifyModule.cpp

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,17 @@ bool PNaClABIVerifyModule::isWhitelistedIntrinsic(const Function *F,
199199
case Intrinsic::vacopy:
200200
case Intrinsic::vaend:
201201
case Intrinsic::vastart:
202+
// Disallow the *_with_overflow intrinsics because they return
203+
// struct types. All of them can be introduced by passing -ftrapv
204+
// to Clang, which we do not support for now. umul_with_overflow
205+
// and uadd_with_overflow are introduced by Clang for C++'s new[],
206+
// but ExpandArithWithOverflow expands out this use.
207+
case Intrinsic::sadd_with_overflow:
208+
case Intrinsic::ssub_with_overflow:
209+
case Intrinsic::uadd_with_overflow:
210+
case Intrinsic::usub_with_overflow:
211+
case Intrinsic::smul_with_overflow:
212+
case Intrinsic::umul_with_overflow:
202213
return false;
203214

204215
// (3) Dev intrinsics.
@@ -226,13 +237,6 @@ bool PNaClABIVerifyModule::isWhitelistedIntrinsic(const Function *F,
226237
case Intrinsic::sqrt: // Rounding is defined, but setting errno up to libm.
227238
case Intrinsic::stackrestore: // Used to support C99 VLAs.
228239
case Intrinsic::stacksave: // Used to support C99 VLAs.
229-
// the *_with_overflow return struct types, so we'll need to fix these.
230-
case Intrinsic::sadd_with_overflow: // Introduced by -ftrapv
231-
case Intrinsic::ssub_with_overflow:
232-
case Intrinsic::uadd_with_overflow:
233-
case Intrinsic::usub_with_overflow:
234-
case Intrinsic::smul_with_overflow:
235-
case Intrinsic::umul_with_overflow: // Introduced by c++ new[x * y].
236240
return PNaClABIAllowDevIntrinsics;
237241
}
238242
}

‎lib/Transforms/InstCombine/InstCombineCompares.cpp

Lines changed: 32 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2193,13 +2193,17 @@ Instruction *InstCombiner::visitICmpInst(ICmpInst &I) {
21932193
//
21942194
// sum = a + b
21952195
// if (sum+128 >u 255) ... -> llvm.sadd.with.overflow.i8
2196-
{
2197-
ConstantInt *CI2; // I = icmp ugt (add (add A, B), CI2), CI
2198-
if (I.getPredicate() == ICmpInst::ICMP_UGT &&
2199-
match(Op0, m_Add(m_Add(m_Value(A), m_Value(B)), m_ConstantInt(CI2))))
2200-
if (Instruction *Res = ProcessUGT_ADDCST_ADD(I, A, B, CI2, CI, *this))
2201-
return Res;
2202-
}
2196+
// @LOCALMOD-BEGIN
2197+
// This is disabled for PNaCl, because we don't support the
2198+
// with.overflow intrinsics in PNaCl's stable ABI.
2199+
if (0) {
2200+
ConstantInt *CI2; // I = icmp ugt (add (add A, B), CI2), CI
2201+
if (I.getPredicate() == ICmpInst::ICMP_UGT &&
2202+
match(Op0, m_Add(m_Add(m_Value(A), m_Value(B)), m_ConstantInt(CI2))))
2203+
if (Instruction *Res = ProcessUGT_ADDCST_ADD(I, A, B, CI2, CI, *this))
2204+
return Res;
2205+
}
2206+
// @LOCALMOD-END
22032207

22042208
// (icmp ne/eq (sub A B) 0) -> (icmp ne/eq A, B)
22052209
if (I.isEquality() && CI->isZero() &&
@@ -2863,21 +2867,27 @@ Instruction *InstCombiner::visitICmpInst(ICmpInst &I) {
28632867
return new ICmpInst(I.getPredicate(), ConstantExpr::getNot(RHSC), A);
28642868
}
28652869

2866-
// (a+b) <u a --> llvm.uadd.with.overflow.
2867-
// (a+b) <u b --> llvm.uadd.with.overflow.
2868-
if (I.getPredicate() == ICmpInst::ICMP_ULT &&
2869-
match(Op0, m_Add(m_Value(A), m_Value(B))) &&
2870-
(Op1 == A || Op1 == B))
2871-
if (Instruction *R = ProcessUAddIdiom(I, Op0, *this))
2872-
return R;
2873-
2874-
// a >u (a+b) --> llvm.uadd.with.overflow.
2875-
// b >u (a+b) --> llvm.uadd.with.overflow.
2876-
if (I.getPredicate() == ICmpInst::ICMP_UGT &&
2877-
match(Op1, m_Add(m_Value(A), m_Value(B))) &&
2878-
(Op0 == A || Op0 == B))
2879-
if (Instruction *R = ProcessUAddIdiom(I, Op1, *this))
2880-
return R;
2870+
// @LOCALMOD-BEGIN
2871+
// This is disabled for PNaCl, because we don't support the
2872+
// with.overflow intrinsics in PNaCl's stable ABI.
2873+
if (0) {
2874+
// (a+b) <u a --> llvm.uadd.with.overflow.
2875+
// (a+b) <u b --> llvm.uadd.with.overflow.
2876+
if (I.getPredicate() == ICmpInst::ICMP_ULT &&
2877+
match(Op0, m_Add(m_Value(A), m_Value(B))) &&
2878+
(Op1 == A || Op1 == B))
2879+
if (Instruction *R = ProcessUAddIdiom(I, Op0, *this))
2880+
return R;
2881+
2882+
// a >u (a+b) --> llvm.uadd.with.overflow.
2883+
// b >u (a+b) --> llvm.uadd.with.overflow.
2884+
if (I.getPredicate() == ICmpInst::ICMP_UGT &&
2885+
match(Op1, m_Add(m_Value(A), m_Value(B))) &&
2886+
(Op0 == A || Op0 == B))
2887+
if (Instruction *R = ProcessUAddIdiom(I, Op1, *this))
2888+
return R;
2889+
}
2890+
// @LOCALMOD-END
28812891
}
28822892

28832893
if (I.isEquality()) {

‎test/Transforms/InstCombine/overflow.ll

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,24 @@
11
; RUN: opt -S -instcombine < %s | FileCheck %s
22
; <rdar://problem/8558713>
33

4+
; @LOCALMOD-BEGIN
5+
; PNaCl does not support the with.overflow intrinsics in its stable
6+
; ABI, so these optimizations are disabled.
7+
8+
; RUN: opt -S -instcombine < %s | FileCheck %s -check-prefix=PNACL
9+
; PNACL-NOT: with.overflow
10+
411
declare void @throwAnExceptionOrWhatever()
512

613
; CHECK-LABEL: @test1(
714
define i32 @test1(i32 %a, i32 %b) nounwind ssp {
815
entry:
9-
; CHECK-NOT: sext
16+
; C;HECK-NOT: sext
1017
%conv = sext i32 %a to i64
1118
%conv2 = sext i32 %b to i64
1219
%add = add nsw i64 %conv2, %conv
1320
%add.off = add i64 %add, 2147483648
14-
; CHECK: llvm.sadd.with.overflow.i32
21+
; C;HECK: llvm.sadd.with.overflow.i32
1522
%0 = icmp ugt i64 %add.off, 4294967295
1623
br i1 %0, label %if.then, label %if.end
1724

@@ -20,9 +27,9 @@ if.then:
2027
br label %if.end
2128

2229
if.end:
23-
; CHECK-NOT: trunc
30+
; C;HECK-NOT: trunc
2431
%conv9 = trunc i64 %add to i32
25-
; CHECK: ret i32
32+
; C;HECK: ret i32
2633
ret i32 %conv9
2734
}
2835

@@ -86,7 +93,7 @@ entry:
8693
%add4 = add nsw i32 %add, 128
8794
%cmp = icmp ugt i32 %add4, 255
8895
br i1 %cmp, label %if.then, label %if.end
89-
; CHECK: llvm.sadd.with.overflow.i8
96+
; C;HECK: llvm.sadd.with.overflow.i8
9097
if.then: ; preds = %entry
9198
tail call void @throwAnExceptionOrWhatever() nounwind
9299
unreachable
@@ -98,7 +105,7 @@ if.end: ; preds = %entry
98105
}
99106

100107
; CHECK-LABEL: @test5(
101-
; CHECK: llvm.uadd.with.overflow
108+
; C;HECK: llvm.uadd.with.overflow
102109
; CHECK: ret i64
103110
define i64 @test5(i64 %a, i64 %b) nounwind ssp {
104111
entry:
@@ -109,7 +116,7 @@ entry:
109116
}
110117

111118
; CHECK-LABEL: @test6(
112-
; CHECK: llvm.uadd.with.overflow
119+
; C;HECK: llvm.uadd.with.overflow
113120
; CHECK: ret i64
114121
define i64 @test6(i64 %a, i64 %b) nounwind ssp {
115122
entry:
@@ -120,7 +127,7 @@ entry:
120127
}
121128

122129
; CHECK-LABEL: @test7(
123-
; CHECK: llvm.uadd.with.overflow
130+
; C;HECK: llvm.uadd.with.overflow
124131
; CHECK: ret i64
125132
define i64 @test7(i64 %a, i64 %b) nounwind ssp {
126133
entry:
@@ -153,3 +160,5 @@ if.end:
153160
%conv9 = trunc i64 %add to i32
154161
ret i32 %conv9
155162
}
163+
164+
; @LOCALMOD-END

0 commit comments

Comments
 (0)