[llvm] 01c48d7 - [Attributor] Fold terminators before changing instructions to unreachable

[Stefanos Baziotis via llvm-commits]

Author: Stefanos Baziotis
Date: 2020-03-07T12:38:44+02:00
New Revision: 01c48d7d11e657fba0d031fa8b478ae9fc730a98

URL: https://github.com/llvm/llvm-project/commit/01c48d7d11e657fba0d031fa8b478ae9fc730a98
DIFF: https://github.com/llvm/llvm-project/commit/01c48d7d11e657fba0d031fa8b478ae9fc730a98.diff

LOG: [Attributor] Fold terminators before changing instructions to unreachable

It is possible that an instruction to be changed to unreachable is
in the same block with a terminator that can be constant-folded.
In this case, as of now, the instruction will be changed to
unreachable before the terminator is folded. But, then the
whole BB becomes invalidated and so when we go ahead to fold
the terminator, we trap.

Change the order of these two.

Differential Revision: https://reviews.llvm.org/D75780

Added: 
    

Modified: 
    llvm/lib/Transforms/IPO/Attributor.cpp
    llvm/test/Transforms/Attributor/undefined_behavior.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp
index f08d7f8ba00f..3aef6e9fe5b4 100644
--- a/llvm/lib/Transforms/IPO/Attributor.cpp
+++ b/llvm/lib/Transforms/IPO/Attributor.cpp
@@ -7798,15 +7798,15 @@ ChangeStatus Attributor::run() {
           ToBeChangedToUnreachableInsts.insert(&NormalDestBB->front());
         }
       }
+    for (Instruction *I : TerminatorsToFold) {
+      CGModifiedFunctions.insert(I->getFunction());
+      ConstantFoldTerminator(I->getParent());
+    }
     for (auto &V : ToBeChangedToUnreachableInsts)
       if (Instruction *I = dyn_cast_or_null<Instruction>(V)) {
         CGModifiedFunctions.insert(I->getFunction());
         changeToUnreachable(I, /* UseLLVMTrap */ false);
       }
-    for (Instruction *I : TerminatorsToFold) {
-      CGModifiedFunctions.insert(I->getFunction());
-      ConstantFoldTerminator(I->getParent());
-    }
 
     for (auto &V : ToBeDeletedInsts) {
       if (Instruction *I = dyn_cast_or_null<Instruction>(V)) {

diff  --git a/llvm/test/Transforms/Attributor/undefined_behavior.ll b/llvm/test/Transforms/Attributor/undefined_behavior.ll
index cf721611973f..492dcd1b6293 100644
--- a/llvm/test/Transforms/Attributor/undefined_behavior.ll
+++ b/llvm/test/Transforms/Attributor/undefined_behavior.ll
@@ -298,3 +298,27 @@ t:
 e:
   ret i32 2
 }
+
+; Note that the `load` has UB (so it will be changed to unreachable)
+; and the branch is a terminator that can be constant-folded.
+; We want to test that doing both won't cause a segfault.
+define internal i32 @callee(i1 %C, i32* %A) {
+; ATTRIBUTOR-NOT: @callee(
+;
+entry:
+  %A.0 = load i32, i32* null
+  br i1 %C, label %T, label %F
+
+T:
+  ret i32 %A.0
+
+F:
+  ret i32 1
+}
+
+define i32 @foo() {
+; ATTRIBUTOR-LABEL: @foo()
+; ATTRIBUTOR-NEXT:    ret i32 1
+  %X = call i32 @callee(i1 false, i32* null)
+  ret i32 %X
+}