[llvm] e338d08 - [SimplifyCFG] Fix SimplifyBranchOnICmpChain to be undef/poison safe.
hyeongyu kim via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 12 23:35:24 PDT 2021
Author: hyeongyu kim
Date: 2021-07-13T15:35:18+09:00
New Revision: e338d08ae609bf07b1f43ad15a6488e7c302800b
URL: https://github.com/llvm/llvm-project/commit/e338d08ae609bf07b1f43ad15a6488e7c302800b
DIFF: https://github.com/llvm/llvm-project/commit/e338d08ae609bf07b1f43ad15a6488e7c302800b.diff
LOG: [SimplifyCFG] Fix SimplifyBranchOnICmpChain to be undef/poison safe.
This patch fixes the problem of SimplifyBranchOnICmpChain that occurs
when extra values are Undef or poison.
Suppose the %mode is 51 and the %Cond is poison, and let's look at the
case below.
```
%A = icmp ne i32 %mode, 0
%B = icmp ne i32 %mode, 51
%C = select i1 %A, i1 %B, i1 false
%D = select i1 %C, i1 %Cond, i1 false
br i1 %D, label %T, label %F
=>
br i1 %Cond, label %switch.early.test, label %F
switch.early.test:
switch i32 %mode, label %T [
i32 51, label %F
i32 0, label %F
]
```
incorrectness: https://alive2.llvm.org/ce/z/BWScX
Code before transformation will not raise UB because %C and %D is false,
and it will not use %Cond. But after transformation, %Cond is being used
immediately, and it will raise UB.
This problem can be solved by adding freeze instruction.
correctness: https://alive2.llvm.org/ce/z/x9x4oY
Reviewed By: nikic
Differential Revision: https://reviews.llvm.org/D104569
Added:
Modified:
llvm/lib/Transforms/Utils/SimplifyCFG.cpp
llvm/test/Transforms/SimplifyCFG/switch_create-custom-dl.ll
llvm/test/Transforms/SimplifyCFG/switch_create.ll
llvm/test/Transforms/SimplifyCFG/switch_msan.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index f08ab18b15b2..805bc32f5f8d 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -4241,11 +4241,6 @@ bool SimplifyCFGOpt::SimplifyBranchOnICmpChain(BranchInst *BI,
BasicBlock *BB = BI->getParent();
- // MSAN does not like undefs as branch condition which can be introduced
- // with "explicit branch".
- if (ExtraCase && BB->getParent()->hasFnAttribute(Attribute::SanitizeMemory))
- return false;
-
LLVM_DEBUG(dbgs() << "Converting 'icmp' chain with " << Values.size()
<< " cases into SWITCH. BB is:\n"
<< *BB);
@@ -4263,6 +4258,16 @@ bool SimplifyCFGOpt::SimplifyBranchOnICmpChain(BranchInst *BI,
Instruction *OldTI = BB->getTerminator();
Builder.SetInsertPoint(OldTI);
+ // There can be an unintended UB if extra values are Poison. Before the
+ // transformation, extra values may not be evaluated according to the
+ // condition, and it will not raise UB. But after transformation, we are
+ // evaluating extra values before checking the condition, and it will raise
+ // UB. It can be solved by adding freeze instruction to extra values.
+ AssumptionCache *AC = Options.AC;
+
+ if (!isGuaranteedNotToBeUndefOrPoison(ExtraCase, AC, BI, nullptr))
+ ExtraCase = Builder.CreateFreeze(ExtraCase);
+
if (TrueWhenEqual)
Builder.CreateCondBr(ExtraCase, EdgeBB, NewBB);
else
diff --git a/llvm/test/Transforms/SimplifyCFG/switch_create-custom-dl.ll b/llvm/test/Transforms/SimplifyCFG/switch_create-custom-dl.ll
index d9d4c48cfbd7..499064e40a90 100644
--- a/llvm/test/Transforms/SimplifyCFG/switch_create-custom-dl.ll
+++ b/llvm/test/Transforms/SimplifyCFG/switch_create-custom-dl.ll
@@ -256,7 +256,8 @@ define void @test7(i8 zeroext %c, i32 %x) nounwind ssp noredzone {
; CHECK-LABEL: @test7(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[CMP:%.*]] = icmp ult i32 [[X:%.*]], 32
-; CHECK-NEXT: br i1 [[CMP]], label [[IF_THEN:%.*]], label [[SWITCH_EARLY_TEST:%.*]]
+; CHECK-NEXT: [[TMP0:%.*]] = freeze i1 [[CMP]]
+; CHECK-NEXT: br i1 [[TMP0]], label [[IF_THEN:%.*]], label [[SWITCH_EARLY_TEST:%.*]]
; CHECK: switch.early.test:
; CHECK-NEXT: switch i8 [[C:%.*]], label [[COMMON_RET:%.*]] [
; CHECK-NEXT: i8 99, label [[IF_THEN]]
@@ -291,7 +292,8 @@ define i32 @test8(i8 zeroext %c, i32 %x, i1 %C) nounwind ssp noredzone {
; CHECK-NEXT: br i1 [[C:%.*]], label [[N:%.*]], label [[IF_THEN:%.*]]
; CHECK: N:
; CHECK-NEXT: [[CMP:%.*]] = icmp ult i32 [[X:%.*]], 32
-; CHECK-NEXT: br i1 [[CMP]], label [[IF_THEN]], label [[SWITCH_EARLY_TEST:%.*]]
+; CHECK-NEXT: [[TMP0:%.*]] = freeze i1 [[CMP]]
+; CHECK-NEXT: br i1 [[TMP0]], label [[IF_THEN]], label [[SWITCH_EARLY_TEST:%.*]]
; CHECK: switch.early.test:
; CHECK-NEXT: switch i8 [[C:%.*]], label [[COMMON_RET:%.*]] [
; CHECK-NEXT: i8 99, label [[IF_THEN]]
@@ -330,7 +332,8 @@ define i32 @test9(i8 zeroext %c) nounwind ssp noredzone {
; CHECK-LABEL: @test9(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[CMP:%.*]] = icmp ult i8 [[C:%.*]], 33
-; CHECK-NEXT: br i1 [[CMP]], label [[LOR_END:%.*]], label [[SWITCH_EARLY_TEST:%.*]]
+; CHECK-NEXT: [[TMP0:%.*]] = freeze i1 [[CMP]]
+; CHECK-NEXT: br i1 [[TMP0]], label [[LOR_END:%.*]], label [[SWITCH_EARLY_TEST:%.*]]
; CHECK: switch.early.test:
; CHECK-NEXT: switch i8 [[C]], label [[LOR_RHS:%.*]] [
; CHECK-NEXT: i8 92, label [[LOR_END]]
@@ -346,8 +349,8 @@ define i32 @test9(i8 zeroext %c) nounwind ssp noredzone {
; CHECK: lor.rhs:
; CHECK-NEXT: br label [[LOR_END]]
; CHECK: lor.end:
-; CHECK-NEXT: [[TMP0:%.*]] = phi i1 [ true, [[SWITCH_EARLY_TEST]] ], [ false, [[LOR_RHS]] ], [ true, [[ENTRY:%.*]] ], [ true, [[SWITCH_EARLY_TEST]] ], [ true, [[SWITCH_EARLY_TEST]] ], [ true, [[SWITCH_EARLY_TEST]] ], [ true, [[SWITCH_EARLY_TEST]] ], [ true, [[SWITCH_EARLY_TEST]] ], [ true, [[SWITCH_EARLY_TEST]] ], [ true, [[SWITCH_EARLY_TEST]] ], [ true, [[SWITCH_EARLY_TEST]] ]
-; CHECK-NEXT: [[CONV46:%.*]] = zext i1 [[TMP0]] to i32
+; CHECK-NEXT: [[TMP1:%.*]] = phi i1 [ true, [[SWITCH_EARLY_TEST]] ], [ false, [[LOR_RHS]] ], [ true, [[ENTRY:%.*]] ], [ true, [[SWITCH_EARLY_TEST]] ], [ true, [[SWITCH_EARLY_TEST]] ], [ true, [[SWITCH_EARLY_TEST]] ], [ true, [[SWITCH_EARLY_TEST]] ], [ true, [[SWITCH_EARLY_TEST]] ], [ true, [[SWITCH_EARLY_TEST]] ], [ true, [[SWITCH_EARLY_TEST]] ], [ true, [[SWITCH_EARLY_TEST]] ]
+; CHECK-NEXT: [[CONV46:%.*]] = zext i1 [[TMP1]] to i32
; CHECK-NEXT: ret i32 [[CONV46]]
;
entry:
@@ -400,7 +403,8 @@ lor.end: ; preds = %lor.rhs, %lor.lhs.f
define i32 @test10(i32 %mode, i1 %Cond) {
; CHECK-LABEL: @test10(
-; CHECK-NEXT: br i1 [[COND:%.*]], label [[SWITCH_EARLY_TEST:%.*]], label [[F:%.*]]
+; CHECK-NEXT: [[TMP1:%.*]] = freeze i1 [[COND:%.*]]
+; CHECK-NEXT: br i1 [[TMP1]], label [[SWITCH_EARLY_TEST:%.*]], label [[F:%.*]]
; CHECK: switch.early.test:
; CHECK-NEXT: switch i32 [[MODE:%.*]], label [[T:%.*]] [
; CHECK-NEXT: i32 51, label [[F]]
diff --git a/llvm/test/Transforms/SimplifyCFG/switch_create.ll b/llvm/test/Transforms/SimplifyCFG/switch_create.ll
index aa469344d825..1ba0971fc4f0 100644
--- a/llvm/test/Transforms/SimplifyCFG/switch_create.ll
+++ b/llvm/test/Transforms/SimplifyCFG/switch_create.ll
@@ -310,7 +310,8 @@ define void @test7(i8 zeroext %c, i32 %x) nounwind ssp noredzone {
; CHECK-LABEL: @test7(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[CMP:%.*]] = icmp ult i32 [[X:%.*]], 32
-; CHECK-NEXT: br i1 [[CMP]], label [[IF_THEN:%.*]], label [[SWITCH_EARLY_TEST:%.*]]
+; CHECK-NEXT: [[TMP0:%.*]] = freeze i1 [[CMP]]
+; CHECK-NEXT: br i1 [[TMP0]], label [[IF_THEN:%.*]], label [[SWITCH_EARLY_TEST:%.*]]
; CHECK: switch.early.test:
; CHECK-NEXT: switch i8 [[C:%.*]], label [[COMMON_RET:%.*]] [
; CHECK-NEXT: i8 99, label [[IF_THEN]]
@@ -345,7 +346,8 @@ define i32 @test8(i8 zeroext %c, i32 %x, i1 %C) nounwind ssp noredzone {
; CHECK-NEXT: br i1 [[C:%.*]], label [[N:%.*]], label [[IF_THEN:%.*]]
; CHECK: N:
; CHECK-NEXT: [[CMP:%.*]] = icmp ult i32 [[X:%.*]], 32
-; CHECK-NEXT: br i1 [[CMP]], label [[IF_THEN]], label [[SWITCH_EARLY_TEST:%.*]]
+; CHECK-NEXT: [[TMP0:%.*]] = freeze i1 [[CMP]]
+; CHECK-NEXT: br i1 [[TMP0]], label [[IF_THEN]], label [[SWITCH_EARLY_TEST:%.*]]
; CHECK: switch.early.test:
; CHECK-NEXT: switch i8 [[C:%.*]], label [[COMMON_RET:%.*]] [
; CHECK-NEXT: i8 99, label [[IF_THEN]]
@@ -384,7 +386,8 @@ define i32 @test9(i8 zeroext %c) nounwind ssp noredzone {
; CHECK-LABEL: @test9(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[CMP:%.*]] = icmp ult i8 [[C:%.*]], 33
-; CHECK-NEXT: br i1 [[CMP]], label [[LOR_END:%.*]], label [[SWITCH_EARLY_TEST:%.*]]
+; CHECK-NEXT: [[TMP0:%.*]] = freeze i1 [[CMP]]
+; CHECK-NEXT: br i1 [[TMP0]], label [[LOR_END:%.*]], label [[SWITCH_EARLY_TEST:%.*]]
; CHECK: switch.early.test:
; CHECK-NEXT: switch i8 [[C]], label [[LOR_RHS:%.*]] [
; CHECK-NEXT: i8 92, label [[LOR_END]]
@@ -400,8 +403,8 @@ define i32 @test9(i8 zeroext %c) nounwind ssp noredzone {
; CHECK: lor.rhs:
; CHECK-NEXT: br label [[LOR_END]]
; CHECK: lor.end:
-; CHECK-NEXT: [[TMP0:%.*]] = phi i1 [ true, [[SWITCH_EARLY_TEST]] ], [ false, [[LOR_RHS]] ], [ true, [[ENTRY:%.*]] ], [ true, [[SWITCH_EARLY_TEST]] ], [ true, [[SWITCH_EARLY_TEST]] ], [ true, [[SWITCH_EARLY_TEST]] ], [ true, [[SWITCH_EARLY_TEST]] ], [ true, [[SWITCH_EARLY_TEST]] ], [ true, [[SWITCH_EARLY_TEST]] ], [ true, [[SWITCH_EARLY_TEST]] ], [ true, [[SWITCH_EARLY_TEST]] ]
-; CHECK-NEXT: [[CONV46:%.*]] = zext i1 [[TMP0]] to i32
+; CHECK-NEXT: [[TMP1:%.*]] = phi i1 [ true, [[SWITCH_EARLY_TEST]] ], [ false, [[LOR_RHS]] ], [ true, [[ENTRY:%.*]] ], [ true, [[SWITCH_EARLY_TEST]] ], [ true, [[SWITCH_EARLY_TEST]] ], [ true, [[SWITCH_EARLY_TEST]] ], [ true, [[SWITCH_EARLY_TEST]] ], [ true, [[SWITCH_EARLY_TEST]] ], [ true, [[SWITCH_EARLY_TEST]] ], [ true, [[SWITCH_EARLY_TEST]] ], [ true, [[SWITCH_EARLY_TEST]] ]
+; CHECK-NEXT: [[CONV46:%.*]] = zext i1 [[TMP1]] to i32
; CHECK-NEXT: ret i32 [[CONV46]]
;
entry:
@@ -454,7 +457,8 @@ lor.end: ; preds = %lor.rhs, %lor.lhs.f
define i32 @test10(i32 %mode, i1 %Cond) {
; CHECK-LABEL: @test10(
-; CHECK-NEXT: br i1 [[COND:%.*]], label [[SWITCH_EARLY_TEST:%.*]], label [[F:%.*]]
+; CHECK-NEXT: [[TMP1:%.*]] = freeze i1 [[COND:%.*]]
+; CHECK-NEXT: br i1 [[TMP1]], label [[SWITCH_EARLY_TEST:%.*]], label [[F:%.*]]
; CHECK: switch.early.test:
; CHECK-NEXT: switch i32 [[MODE:%.*]], label [[T:%.*]] [
; CHECK-NEXT: i32 51, label [[F]]
@@ -486,7 +490,8 @@ F:
define i32 @test10_select(i32 %mode, i1 %Cond) {
; CHECK-LABEL: @test10_select(
-; CHECK-NEXT: br i1 [[COND:%.*]], label [[SWITCH_EARLY_TEST:%.*]], label [[F:%.*]]
+; CHECK-NEXT: [[TMP1:%.*]] = freeze i1 [[COND:%.*]]
+; CHECK-NEXT: br i1 [[TMP1]], label [[SWITCH_EARLY_TEST:%.*]], label [[F:%.*]]
; CHECK: switch.early.test:
; CHECK-NEXT: switch i32 [[MODE:%.*]], label [[T:%.*]] [
; CHECK-NEXT: i32 51, label [[F]]
@@ -519,7 +524,8 @@ F:
; TODO: %Cond doesn't need freeze
define i32 @test10_select_and(i32 %mode, i1 %Cond) {
; CHECK-LABEL: @test10_select_and(
-; CHECK-NEXT: br i1 [[COND:%.*]], label [[SWITCH_EARLY_TEST:%.*]], label [[F:%.*]]
+; CHECK-NEXT: [[TMP1:%.*]] = freeze i1 [[COND:%.*]]
+; CHECK-NEXT: br i1 [[TMP1]], label [[SWITCH_EARLY_TEST:%.*]], label [[F:%.*]]
; CHECK: switch.early.test:
; CHECK-NEXT: switch i32 [[MODE:%.*]], label [[T:%.*]] [
; CHECK-NEXT: i32 51, label [[F]]
diff --git a/llvm/test/Transforms/SimplifyCFG/switch_msan.ll b/llvm/test/Transforms/SimplifyCFG/switch_msan.ll
index a59515ed463a..178d1d4ae766 100644
--- a/llvm/test/Transforms/SimplifyCFG/switch_msan.ll
+++ b/llvm/test/Transforms/SimplifyCFG/switch_msan.ll
@@ -19,7 +19,8 @@ define void @test_no_msan() {
; CHECK-NEXT: [[C_NOT_10_OR_13:%.*]] = xor i1 [[C_10_OR_13]], true
; CHECK-NEXT: br i1 [[C_NOT_10_OR_13]], label [[WHILE_BODY_I]], label [[WHILE_BODY_I_BREAK:%.*]]
; CHECK: while.body.i.break:
-; CHECK-NEXT: br i1 [[MAYBE_UNDEF]], label [[WHILE_BODY]], label [[SWITCH_EARLY_TEST:%.*]]
+; CHECK-NEXT: [[TMP0:%.*]] = freeze i1 [[MAYBE_UNDEF]]
+; CHECK-NEXT: br i1 [[TMP0]], label [[WHILE_BODY]], label [[SWITCH_EARLY_TEST:%.*]]
; CHECK: switch.early.test:
; CHECK-NEXT: switch i8 [[C]], label [[RETURN:%.*]] [
; CHECK-NEXT: i8 13, label [[WHILE_BODY]]
@@ -71,7 +72,13 @@ define void @test_msan() sanitize_memory {
; CHECK-NEXT: [[C_NOT_10_OR_13:%.*]] = xor i1 [[C_10_OR_13]], true
; CHECK-NEXT: br i1 [[C_NOT_10_OR_13]], label [[WHILE_BODY_I]], label [[WHILE_BODY_I_BREAK:%.*]]
; CHECK: while.body.i.break:
-; CHECK-NEXT: br i1 [[NEXT_MAYBE_UNDEF]], label [[WHILE_BODY]], label [[RETURN:%.*]]
+; CHECK-NEXT: [[TMP0:%.*]] = freeze i1 [[MAYBE_UNDEF]]
+; CHECK-NEXT: br i1 [[TMP0]], label [[WHILE_BODY]], label [[SWITCH_EARLY_TEST:%.*]]
+; CHECK: switch.early.test:
+; CHECK-NEXT: switch i8 [[C]], label [[RETURN:%.*]] [
+; CHECK-NEXT: i8 13, label [[WHILE_BODY]]
+; CHECK-NEXT: i8 10, label [[WHILE_BODY]]
+; CHECK-NEXT: ]
; CHECK: return:
; CHECK-NEXT: ret void
;
More information about the llvm-commits
mailing list