[llvm] 416a119 - [GlobalOpt] don't hoist constant expressions that can trap
Sanjay Patel via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 27 05:25:44 PDT 2021
Author: Sanjay Patel
Date: 2021-08-27T08:10:20-04:00
New Revision: 416a119f9e5ce45df4c26215d19ed5be29b052cd
URL: https://github.com/llvm/llvm-project/commit/416a119f9e5ce45df4c26215d19ed5be29b052cd
DIFF: https://github.com/llvm/llvm-project/commit/416a119f9e5ce45df4c26215d19ed5be29b052cd.diff
LOG: [GlobalOpt] don't hoist constant expressions that can trap
We try to forward a stored-once-constant-value from one global access
to another, but that's not safe if the constant value is an expression
that can trap.
The tests are reduced from the miscompile examples in:
https://llvm.org/PR47578
Differential Revision: https://reviews.llvm.org/D108771
Added:
Modified:
llvm/lib/Transforms/IPO/GlobalOpt.cpp
llvm/test/Transforms/GlobalOpt/constant-can-trap.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/IPO/GlobalOpt.cpp b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
index 04282b5f8f10c..68b6fbadf703b 100644
--- a/llvm/lib/Transforms/IPO/GlobalOpt.cpp
+++ b/llvm/lib/Transforms/IPO/GlobalOpt.cpp
@@ -1606,28 +1606,32 @@ processInternalGlobal(GlobalVariable *GV, const GlobalStatus &GS,
return true;
}
if (GS.StoredType == GlobalStatus::StoredOnce && GS.StoredOnceValue) {
+ // Avoid speculating constant expressions that might trap (div/rem).
+ auto *SOVConstant = dyn_cast<Constant>(GS.StoredOnceValue);
+ if (SOVConstant && SOVConstant->canTrap())
+ return Changed;
+
// If the initial value for the global was an undef value, and if only
// one other value was stored into it, we can just change the
// initializer to be the stored value, then delete all stores to the
// global. This allows us to mark it constant.
- if (Constant *SOVConstant = dyn_cast<Constant>(GS.StoredOnceValue))
- if (SOVConstant->getType() == GV->getValueType() &&
- isa<UndefValue>(GV->getInitializer())) {
- // Change the initial value here.
- GV->setInitializer(SOVConstant);
-
- // Clean up any obviously simplifiable users now.
- CleanupConstantGlobalUsers(GV, GV->getInitializer(), DL, GetTLI);
-
- if (GV->use_empty()) {
- LLVM_DEBUG(dbgs() << " *** Substituting initializer allowed us to "
- << "simplify all users and delete global!\n");
- GV->eraseFromParent();
- ++NumDeleted;
- }
- ++NumSubstitute;
- return true;
+ if (SOVConstant && SOVConstant->getType() == GV->getValueType() &&
+ isa<UndefValue>(GV->getInitializer())) {
+ // Change the initial value here.
+ GV->setInitializer(SOVConstant);
+
+ // Clean up any obviously simplifiable users now.
+ CleanupConstantGlobalUsers(GV, GV->getInitializer(), DL, GetTLI);
+
+ if (GV->use_empty()) {
+ LLVM_DEBUG(dbgs() << " *** Substituting initializer allowed us to "
+ << "simplify all users and delete global!\n");
+ GV->eraseFromParent();
+ ++NumDeleted;
}
+ ++NumSubstitute;
+ return true;
+ }
// Try to optimize globals based on the knowledge that only one value
// (besides its initializer) is ever stored to the global.
@@ -1637,12 +1641,10 @@ processInternalGlobal(GlobalVariable *GV, const GlobalStatus &GS,
// Otherwise, if the global was not a boolean, we can shrink it to be a
// boolean.
- if (Constant *SOVConstant = dyn_cast<Constant>(GS.StoredOnceValue)) {
- if (GS.Ordering == AtomicOrdering::NotAtomic) {
- if (TryToShrinkGlobalToBoolean(GV, SOVConstant)) {
- ++NumShrunkToBool;
- return true;
- }
+ if (SOVConstant && GS.Ordering == AtomicOrdering::NotAtomic) {
+ if (TryToShrinkGlobalToBoolean(GV, SOVConstant)) {
+ ++NumShrunkToBool;
+ return true;
}
}
}
diff --git a/llvm/test/Transforms/GlobalOpt/constant-can-trap.ll b/llvm/test/Transforms/GlobalOpt/constant-can-trap.ll
index 5caef59fa8091..8a86987e317a3 100644
--- a/llvm/test/Transforms/GlobalOpt/constant-can-trap.ll
+++ b/llvm/test/Transforms/GlobalOpt/constant-can-trap.ll
@@ -4,20 +4,21 @@
@i = internal unnamed_addr global i32 1, align 4
@r = internal global i64 0, align 8
+; negative test for store-once-global - the urem constant expression must not be speculated
+
declare dso_local void @use(i32)
define i32 @cantrap_constant() {
; CHECK-LABEL: @cantrap_constant(
; CHECK-NEXT: entry:
-; CHECK-NEXT: [[DOTB:%.*]] = load i1, i1* @i, align 1
-; CHECK-NEXT: [[TMP0:%.*]] = select i1 [[DOTB]], i32 trunc (i64 urem (i64 7, i64 zext (i1 icmp eq (i64* inttoptr (i64 1 to i64*), i64* @r) to i64)) to i32), i32 1
+; CHECK-NEXT: [[TMP0:%.*]] = load i32, i32* @i, align 4
; CHECK-NEXT: [[TOBOOL:%.*]] = icmp eq i32 [[TMP0]], 0
; CHECK-NEXT: [[NOT_TOBOOL:%.*]] = xor i1 [[TOBOOL]], true
; CHECK-NEXT: [[SPEC_SELECT:%.*]] = zext i1 [[NOT_TOBOOL]] to i32
; CHECK-NEXT: tail call void @use(i32 [[SPEC_SELECT]])
; CHECK-NEXT: br i1 [[TOBOOL]], label [[IF_THEN:%.*]], label [[EXIT:%.*]]
; CHECK: if.then:
-; CHECK-NEXT: store i1 true, i1* @i, align 1
+; CHECK-NEXT: store i32 trunc (i64 urem (i64 7, i64 zext (i1 icmp eq (i64* inttoptr (i64 1 to i64*), i64* @r) to i64)) to i32), i32* @i, align 4
; CHECK-NEXT: br label [[EXIT]]
; CHECK: exit:
; CHECK-NEXT: ret i32 0
@@ -38,12 +39,14 @@ exit:
ret i32 0
}
+; negative test for store-once-global - the srem constant expression must not be speculated
+
@b1 = internal global i64* null, align 8
@d1 = internal unnamed_addr global i32 0, align 2
define void @maytrap() {
; CHECK-LABEL: @maytrap(
-; CHECK-NEXT: store i1 true, i1* @d1, align 1
+; CHECK-NEXT: store i32 srem (i32 7, i32 zext (i1 icmp eq (i64** inttoptr (i64 16 to i64**), i64** @b1) to i32)), i32* @d1, align 2
; CHECK-NEXT: ret void
;
store i32 srem (i32 7, i32 zext (i1 icmp eq (i64** inttoptr (i64 16 to i64**), i64** @b1) to i32)), i32* @d1, align 2
@@ -52,14 +55,15 @@ define void @maytrap() {
define i32 @main1() {
; CHECK-LABEL: @main1(
-; CHECK-NEXT: [[T0_B:%.*]] = load i1, i1* @d1, align 1
-; CHECK-NEXT: [[T0:%.*]] = select i1 [[T0_B]], i32 srem (i32 7, i32 zext (i1 icmp eq (i64** inttoptr (i64 16 to i64**), i64** @b1) to i32)), i32 0
+; CHECK-NEXT: [[T0:%.*]] = load i32, i32* @d1, align 2
; CHECK-NEXT: ret i32 [[T0]]
;
%t0 = load i32, i32* @d1, align 2
ret i32 %t0
}
+; This is fine to optimize as a store-once-global constant because the expression can't trap
+
@b2 = internal global i64* null, align 8
@d2 = internal unnamed_addr global i32 0, align 2
More information about the llvm-commits
mailing list