Skip to content

Commit 01d0a0d

Browse files
hyeongyukimmemfrob
hyeongyukim
authored and
memfrob
committed
[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
1 parent b49dadf commit 01d0a0d

File tree

4 files changed

+43
-21
lines changed

4 files changed

+43
-21
lines changed

‎llvm/lib/Transforms/Utils/SimplifyCFG.cpp

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4241,11 +4241,6 @@ bool SimplifyCFGOpt::SimplifyBranchOnICmpChain(BranchInst *BI,
42414241

42424242
BasicBlock *BB = BI->getParent();
42434243

4244-
// MSAN does not like undefs as branch condition which can be introduced
4245-
// with "explicit branch".
4246-
if (ExtraCase && BB->getParent()->hasFnAttribute(Attribute::SanitizeMemory))
4247-
return false;
4248-
42494244
LLVM_DEBUG(dbgs() << "Converting 'icmp' chain with " << Values.size()
42504245
<< " cases into SWITCH. BB is:\n"
42514246
<< *BB);
@@ -4263,6 +4258,16 @@ bool SimplifyCFGOpt::SimplifyBranchOnICmpChain(BranchInst *BI,
42634258
Instruction *OldTI = BB->getTerminator();
42644259
Builder.SetInsertPoint(OldTI);
42654260

4261+
// There can be an unintended UB if extra values are Poison. Before the
4262+
// transformation, extra values may not be evaluated according to the
4263+
// condition, and it will not raise UB. But after transformation, we are
4264+
// evaluating extra values before checking the condition, and it will raise
4265+
// UB. It can be solved by adding freeze instruction to extra values.
4266+
AssumptionCache *AC = Options.AC;
4267+
4268+
if (!isGuaranteedNotToBeUndefOrPoison(ExtraCase, AC, BI, nullptr))
4269+
ExtraCase = Builder.CreateFreeze(ExtraCase);
4270+
42664271
if (TrueWhenEqual)
42674272
Builder.CreateCondBr(ExtraCase, EdgeBB, NewBB);
42684273
else

‎llvm/test/Transforms/SimplifyCFG/switch_create-custom-dl.ll

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,8 @@ define void @test7(i8 zeroext %c, i32 %x) nounwind ssp noredzone {
256256
; CHECK-LABEL: @test7(
257257
; CHECK-NEXT: entry:
258258
; CHECK-NEXT: [[CMP:%.*]] = icmp ult i32 [[X:%.*]], 32
259-
; CHECK-NEXT: br i1 [[CMP]], label [[IF_THEN:%.*]], label [[SWITCH_EARLY_TEST:%.*]]
259+
; CHECK-NEXT: [[TMP0:%.*]] = freeze i1 [[CMP]]
260+
; CHECK-NEXT: br i1 [[TMP0]], label [[IF_THEN:%.*]], label [[SWITCH_EARLY_TEST:%.*]]
260261
; CHECK: switch.early.test:
261262
; CHECK-NEXT: switch i8 [[C:%.*]], label [[COMMON_RET:%.*]] [
262263
; CHECK-NEXT: i8 99, label [[IF_THEN]]
@@ -291,7 +292,8 @@ define i32 @test8(i8 zeroext %c, i32 %x, i1 %C) nounwind ssp noredzone {
291292
; CHECK-NEXT: br i1 [[C:%.*]], label [[N:%.*]], label [[IF_THEN:%.*]]
292293
; CHECK: N:
293294
; CHECK-NEXT: [[CMP:%.*]] = icmp ult i32 [[X:%.*]], 32
294-
; CHECK-NEXT: br i1 [[CMP]], label [[IF_THEN]], label [[SWITCH_EARLY_TEST:%.*]]
295+
; CHECK-NEXT: [[TMP0:%.*]] = freeze i1 [[CMP]]
296+
; CHECK-NEXT: br i1 [[TMP0]], label [[IF_THEN]], label [[SWITCH_EARLY_TEST:%.*]]
295297
; CHECK: switch.early.test:
296298
; CHECK-NEXT: switch i8 [[C:%.*]], label [[COMMON_RET:%.*]] [
297299
; CHECK-NEXT: i8 99, label [[IF_THEN]]
@@ -330,7 +332,8 @@ define i32 @test9(i8 zeroext %c) nounwind ssp noredzone {
330332
; CHECK-LABEL: @test9(
331333
; CHECK-NEXT: entry:
332334
; CHECK-NEXT: [[CMP:%.*]] = icmp ult i8 [[C:%.*]], 33
333-
; CHECK-NEXT: br i1 [[CMP]], label [[LOR_END:%.*]], label [[SWITCH_EARLY_TEST:%.*]]
335+
; CHECK-NEXT: [[TMP0:%.*]] = freeze i1 [[CMP]]
336+
; CHECK-NEXT: br i1 [[TMP0]], label [[LOR_END:%.*]], label [[SWITCH_EARLY_TEST:%.*]]
334337
; CHECK: switch.early.test:
335338
; CHECK-NEXT: switch i8 [[C]], label [[LOR_RHS:%.*]] [
336339
; CHECK-NEXT: i8 92, label [[LOR_END]]
@@ -346,8 +349,8 @@ define i32 @test9(i8 zeroext %c) nounwind ssp noredzone {
346349
; CHECK: lor.rhs:
347350
; CHECK-NEXT: br label [[LOR_END]]
348351
; CHECK: lor.end:
349-
; 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]] ]
350-
; CHECK-NEXT: [[CONV46:%.*]] = zext i1 [[TMP0]] to i32
352+
; 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]] ]
353+
; CHECK-NEXT: [[CONV46:%.*]] = zext i1 [[TMP1]] to i32
351354
; CHECK-NEXT: ret i32 [[CONV46]]
352355
;
353356
entry:
@@ -400,7 +403,8 @@ lor.end: ; preds = %lor.rhs, %lor.lhs.f
400403

401404
define i32 @test10(i32 %mode, i1 %Cond) {
402405
; CHECK-LABEL: @test10(
403-
; CHECK-NEXT: br i1 [[COND:%.*]], label [[SWITCH_EARLY_TEST:%.*]], label [[F:%.*]]
406+
; CHECK-NEXT: [[TMP1:%.*]] = freeze i1 [[COND:%.*]]
407+
; CHECK-NEXT: br i1 [[TMP1]], label [[SWITCH_EARLY_TEST:%.*]], label [[F:%.*]]
404408
; CHECK: switch.early.test:
405409
; CHECK-NEXT: switch i32 [[MODE:%.*]], label [[T:%.*]] [
406410
; CHECK-NEXT: i32 51, label [[F]]

‎llvm/test/Transforms/SimplifyCFG/switch_create.ll

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,8 @@ define void @test7(i8 zeroext %c, i32 %x) nounwind ssp noredzone {
310310
; CHECK-LABEL: @test7(
311311
; CHECK-NEXT: entry:
312312
; CHECK-NEXT: [[CMP:%.*]] = icmp ult i32 [[X:%.*]], 32
313-
; CHECK-NEXT: br i1 [[CMP]], label [[IF_THEN:%.*]], label [[SWITCH_EARLY_TEST:%.*]]
313+
; CHECK-NEXT: [[TMP0:%.*]] = freeze i1 [[CMP]]
314+
; CHECK-NEXT: br i1 [[TMP0]], label [[IF_THEN:%.*]], label [[SWITCH_EARLY_TEST:%.*]]
314315
; CHECK: switch.early.test:
315316
; CHECK-NEXT: switch i8 [[C:%.*]], label [[COMMON_RET:%.*]] [
316317
; CHECK-NEXT: i8 99, label [[IF_THEN]]
@@ -345,7 +346,8 @@ define i32 @test8(i8 zeroext %c, i32 %x, i1 %C) nounwind ssp noredzone {
345346
; CHECK-NEXT: br i1 [[C:%.*]], label [[N:%.*]], label [[IF_THEN:%.*]]
346347
; CHECK: N:
347348
; CHECK-NEXT: [[CMP:%.*]] = icmp ult i32 [[X:%.*]], 32
348-
; CHECK-NEXT: br i1 [[CMP]], label [[IF_THEN]], label [[SWITCH_EARLY_TEST:%.*]]
349+
; CHECK-NEXT: [[TMP0:%.*]] = freeze i1 [[CMP]]
350+
; CHECK-NEXT: br i1 [[TMP0]], label [[IF_THEN]], label [[SWITCH_EARLY_TEST:%.*]]
349351
; CHECK: switch.early.test:
350352
; CHECK-NEXT: switch i8 [[C:%.*]], label [[COMMON_RET:%.*]] [
351353
; CHECK-NEXT: i8 99, label [[IF_THEN]]
@@ -384,7 +386,8 @@ define i32 @test9(i8 zeroext %c) nounwind ssp noredzone {
384386
; CHECK-LABEL: @test9(
385387
; CHECK-NEXT: entry:
386388
; CHECK-NEXT: [[CMP:%.*]] = icmp ult i8 [[C:%.*]], 33
387-
; CHECK-NEXT: br i1 [[CMP]], label [[LOR_END:%.*]], label [[SWITCH_EARLY_TEST:%.*]]
389+
; CHECK-NEXT: [[TMP0:%.*]] = freeze i1 [[CMP]]
390+
; CHECK-NEXT: br i1 [[TMP0]], label [[LOR_END:%.*]], label [[SWITCH_EARLY_TEST:%.*]]
388391
; CHECK: switch.early.test:
389392
; CHECK-NEXT: switch i8 [[C]], label [[LOR_RHS:%.*]] [
390393
; CHECK-NEXT: i8 92, label [[LOR_END]]
@@ -400,8 +403,8 @@ define i32 @test9(i8 zeroext %c) nounwind ssp noredzone {
400403
; CHECK: lor.rhs:
401404
; CHECK-NEXT: br label [[LOR_END]]
402405
; CHECK: lor.end:
403-
; 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]] ]
404-
; CHECK-NEXT: [[CONV46:%.*]] = zext i1 [[TMP0]] to i32
406+
; 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]] ]
407+
; CHECK-NEXT: [[CONV46:%.*]] = zext i1 [[TMP1]] to i32
405408
; CHECK-NEXT: ret i32 [[CONV46]]
406409
;
407410
entry:
@@ -454,7 +457,8 @@ lor.end: ; preds = %lor.rhs, %lor.lhs.f
454457

455458
define i32 @test10(i32 %mode, i1 %Cond) {
456459
; CHECK-LABEL: @test10(
457-
; CHECK-NEXT: br i1 [[COND:%.*]], label [[SWITCH_EARLY_TEST:%.*]], label [[F:%.*]]
460+
; CHECK-NEXT: [[TMP1:%.*]] = freeze i1 [[COND:%.*]]
461+
; CHECK-NEXT: br i1 [[TMP1]], label [[SWITCH_EARLY_TEST:%.*]], label [[F:%.*]]
458462
; CHECK: switch.early.test:
459463
; CHECK-NEXT: switch i32 [[MODE:%.*]], label [[T:%.*]] [
460464
; CHECK-NEXT: i32 51, label [[F]]
@@ -486,7 +490,8 @@ F:
486490

487491
define i32 @test10_select(i32 %mode, i1 %Cond) {
488492
; CHECK-LABEL: @test10_select(
489-
; CHECK-NEXT: br i1 [[COND:%.*]], label [[SWITCH_EARLY_TEST:%.*]], label [[F:%.*]]
493+
; CHECK-NEXT: [[TMP1:%.*]] = freeze i1 [[COND:%.*]]
494+
; CHECK-NEXT: br i1 [[TMP1]], label [[SWITCH_EARLY_TEST:%.*]], label [[F:%.*]]
490495
; CHECK: switch.early.test:
491496
; CHECK-NEXT: switch i32 [[MODE:%.*]], label [[T:%.*]] [
492497
; CHECK-NEXT: i32 51, label [[F]]
@@ -519,7 +524,8 @@ F:
519524
; TODO: %Cond doesn't need freeze
520525
define i32 @test10_select_and(i32 %mode, i1 %Cond) {
521526
; CHECK-LABEL: @test10_select_and(
522-
; CHECK-NEXT: br i1 [[COND:%.*]], label [[SWITCH_EARLY_TEST:%.*]], label [[F:%.*]]
527+
; CHECK-NEXT: [[TMP1:%.*]] = freeze i1 [[COND:%.*]]
528+
; CHECK-NEXT: br i1 [[TMP1]], label [[SWITCH_EARLY_TEST:%.*]], label [[F:%.*]]
523529
; CHECK: switch.early.test:
524530
; CHECK-NEXT: switch i32 [[MODE:%.*]], label [[T:%.*]] [
525531
; CHECK-NEXT: i32 51, label [[F]]

‎llvm/test/Transforms/SimplifyCFG/switch_msan.ll

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ define void @test_no_msan() {
1919
; CHECK-NEXT: [[C_NOT_10_OR_13:%.*]] = xor i1 [[C_10_OR_13]], true
2020
; CHECK-NEXT: br i1 [[C_NOT_10_OR_13]], label [[WHILE_BODY_I]], label [[WHILE_BODY_I_BREAK:%.*]]
2121
; CHECK: while.body.i.break:
22-
; CHECK-NEXT: br i1 [[MAYBE_UNDEF]], label [[WHILE_BODY]], label [[SWITCH_EARLY_TEST:%.*]]
22+
; CHECK-NEXT: [[TMP0:%.*]] = freeze i1 [[MAYBE_UNDEF]]
23+
; CHECK-NEXT: br i1 [[TMP0]], label [[WHILE_BODY]], label [[SWITCH_EARLY_TEST:%.*]]
2324
; CHECK: switch.early.test:
2425
; CHECK-NEXT: switch i8 [[C]], label [[RETURN:%.*]] [
2526
; CHECK-NEXT: i8 13, label [[WHILE_BODY]]
@@ -71,7 +72,13 @@ define void @test_msan() sanitize_memory {
7172
; CHECK-NEXT: [[C_NOT_10_OR_13:%.*]] = xor i1 [[C_10_OR_13]], true
7273
; CHECK-NEXT: br i1 [[C_NOT_10_OR_13]], label [[WHILE_BODY_I]], label [[WHILE_BODY_I_BREAK:%.*]]
7374
; CHECK: while.body.i.break:
74-
; CHECK-NEXT: br i1 [[NEXT_MAYBE_UNDEF]], label [[WHILE_BODY]], label [[RETURN:%.*]]
75+
; CHECK-NEXT: [[TMP0:%.*]] = freeze i1 [[MAYBE_UNDEF]]
76+
; CHECK-NEXT: br i1 [[TMP0]], label [[WHILE_BODY]], label [[SWITCH_EARLY_TEST:%.*]]
77+
; CHECK: switch.early.test:
78+
; CHECK-NEXT: switch i8 [[C]], label [[RETURN:%.*]] [
79+
; CHECK-NEXT: i8 13, label [[WHILE_BODY]]
80+
; CHECK-NEXT: i8 10, label [[WHILE_BODY]]
81+
; CHECK-NEXT: ]
7582
; CHECK: return:
7683
; CHECK-NEXT: ret void
7784
;

0 commit comments

Comments
 (0)