[llvm] cf75e83 - [InstCombine] replace zombie unreachable values with 'undef' before erasing

[Sanjay Patel via llvm-commits]

Author: Sanjay Patel
Date: 2020-09-20T12:25:08-04:00
New Revision: cf75e83275d16f9172fa5ab448a981afdadd34d0

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

LOG: [InstCombine] replace zombie unreachable values with 'undef' before erasing

The test (currently crashing) is reduced from the example provided
in the post-commit discussion in D87149.

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

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
    llvm/test/Transforms/InstCombine/phi.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 63ba7eb85c66..44e29b650016 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -2813,6 +2813,9 @@ Instruction *InstCombinerImpl::visitUnreachableInst(UnreachableInst &I) {
       if (SI->isVolatile())
         return nullptr;
 
+    // A value may still have uses before we process it here (for example, in
+    // another unreachable block), so convert those to undef.
+    replaceInstUsesWith(*Prev, UndefValue::get(Prev->getType()));
     eraseInstFromFunction(*Prev);
     return &I;
   }

diff  --git a/llvm/test/Transforms/InstCombine/phi.ll b/llvm/test/Transforms/InstCombine/phi.ll
index 9ac0bd2c7e3d..e61cec7da47b 100644
--- a/llvm/test/Transforms/InstCombine/phi.ll
+++ b/llvm/test/Transforms/InstCombine/phi.ll
@@ -1181,3 +1181,39 @@ if.end:                                           ; preds = %entry, %if.then
   %cmp1 = icmp ne i32 %a.0, 0
   ret i1  %cmp1
 }
+
+; This would crash trying to delete an instruction (conv)
+; that still had uses because the user (the phi) was not
+; updated to remove a use from an unreachable block (g.exit).
+
+define void @main(i1 %cond, i16 %x) {
+; CHECK-LABEL: @main(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[FOR_COND:%.*]]
+; CHECK:       for.cond:
+; CHECK-NEXT:    br i1 [[COND:%.*]], label [[FOR_END:%.*]], label [[FOR_BODY:%.*]]
+; CHECK:       for.body:
+; CHECK-NEXT:    unreachable
+; CHECK:       g.exit:
+; CHECK-NEXT:    br label [[FOR_COND]]
+; CHECK:       for.end:
+; CHECK-NEXT:    ret void
+;
+entry:
+  br label %for.cond
+
+for.cond:
+  %p = phi double [ %conv, %g.exit ], [ undef, %entry ]
+  br i1 %cond, label %for.end, label %for.body
+
+for.body:
+  %conv = sitofp i16 %x to double
+  unreachable
+
+g.exit:
+  br label %for.cond
+
+for.end:
+  store double %p, double* undef
+  ret void
+}