[PATCH] D87399: Revert "[InstCombine] erase instructions leading up to unreachable"

[stephan.yichao.zhao via Phabricator via llvm-commits]

stephan.yichao.zhao created this revision.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.
stephan.yichao.zhao requested review of this revision.

This reverts commit b22910daab95be1ebc6ab8a74190e38130b0e6ef <https://reviews.llvm.org/rGb22910daab95be1ebc6ab8a74190e38130b0e6ef>.

See the discussion from https://reviews.llvm.org/D87149

1. it is fine to remove instructions that can reach an unreachable

instruction before runnning unreachable is undefined, and undefined
behavior is retroactive.

2. however, a volatile access can abort a program. In that case, an

unreachable instruction after it is not reachable. So undefined
behavior does not happen, and the volatile access cannot be removed.

It is possible to extend
llvm::isGuaranteedToTransferExecutionToSuccessor to make this work.

Revert the change temporarily.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87399

Files:
  llvm/lib/Transforms/InstCombine/InstCombineInternal.h
  llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
  llvm/test/Transforms/InstCombine/assume.ll
  llvm/test/Transforms/InstCombine/pr33689_same_bitwidth.ll


Index: llvm/test/Transforms/InstCombine/pr33689_same_bitwidth.ll
===================================================================
--- llvm/test/Transforms/InstCombine/pr33689_same_bitwidth.ll
+++ llvm/test/Transforms/InstCombine/pr33689_same_bitwidth.ll
@@ -17,6 +17,8 @@
 ; CHECK-NEXT:    [[T12_SUB:%.*]] = getelementptr inbounds [2 x i32], [2 x i32]* [[T12]], i16 0, i16 0
 ; CHECK-NEXT:    br i1 [[COND:%.*]], label [[BB1:%.*]], label [[BB2:%.*]]
 ; CHECK:       bb1:
+; CHECK-NEXT:    [[T8:%.*]] = ptrtoint [2 x i32]* [[T12]] to i16
+; CHECK-NEXT:    store i16 [[T8]], i16* @a, align 2
 ; CHECK-NEXT:    unreachable
 ; CHECK:       bb2:
 ; CHECK-NEXT:    [[T9:%.*]] = load i16*, i16** @b, align 2
Index: llvm/test/Transforms/InstCombine/assume.ll
===================================================================
--- llvm/test/Transforms/InstCombine/assume.ll
+++ llvm/test/Transforms/InstCombine/assume.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt < %s -instcombine -S -instcombine-infinite-loop-threshold=2 | FileCheck %s
+; RUN: opt < %s -instcombine -S | FileCheck %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
@@ -543,6 +543,7 @@
 
 define void @PR36270(i32 %b) {
 ; CHECK-LABEL: @PR36270(
+; CHECK-NEXT:    tail call void @llvm.assume(i1 false)
 ; CHECK-NEXT:    unreachable
 ;
   %B7 = xor i32 -1, 2147483647
@@ -572,6 +573,8 @@
 ; CHECK-NEXT:    tail call void @llvm.assume(i1 [[CMP3]])
 ; CHECK-NEXT:    br label [[EXIT]]
 ; CHECK:       exit:
+; CHECK-NEXT:    [[CMP4:%.*]] = icmp eq i32 [[X]], 2
+; CHECK-NEXT:    tail call void @llvm.assume(i1 [[CMP4]])
 ; CHECK-NEXT:    unreachable
 ;
 entry:
@@ -609,6 +612,11 @@
 ; CHECK-NEXT:    tail call void @llvm.assume(i1 [[CMP3]])
 ; CHECK-NEXT:    br label [[EXIT]]
 ; CHECK:       exit:
+; CHECK-NEXT:    [[CMP4:%.*]] = icmp eq i32 [[X]], 2
+; CHECK-NEXT:    tail call void @llvm.assume(i1 [[CMP4]])
+; CHECK-NEXT:    [[CMP5:%.*]] = icmp ugt i32 [[Y]], 42
+; CHECK-NEXT:    tail call void @llvm.assume(i1 [[CMP5]])
+; CHECK-NEXT:    store i32 [[X]], i32* [[P:%.*]], align 4
 ; CHECK-NEXT:    unreachable
 ;
 entry:
Index: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
===================================================================
--- llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -2798,19 +2798,6 @@
   return nullptr;
 }
 
-Instruction *InstCombinerImpl::visitUnreachableInst(UnreachableInst &I) {
-  // Try to remove the previous instruction if it must lead to unreachable.
-  // This includes instructions like stores and "llvm.assume" that may not get
-  // removed by simple dead code elimination.
-  Instruction *Prev = I.getPrevNonDebugInstruction();
-  if (Prev && !Prev->isEHPad() &&
-      isGuaranteedToTransferExecutionToSuccessor(Prev)) {
-    eraseInstFromFunction(*Prev);
-    return &I;
-  }
-  return nullptr;
-}
-
 Instruction *InstCombinerImpl::visitUnconditionalBranchInst(BranchInst &BI) {
   assert(BI.isUnconditional() && "Only for unconditional branches.");
 
Index: llvm/lib/Transforms/InstCombine/InstCombineInternal.h
===================================================================
--- llvm/lib/Transforms/InstCombine/InstCombineInternal.h
+++ llvm/lib/Transforms/InstCombine/InstCombineInternal.h
@@ -159,7 +159,6 @@
   Instruction *visitFenceInst(FenceInst &FI);
   Instruction *visitSwitchInst(SwitchInst &SI);
   Instruction *visitReturnInst(ReturnInst &RI);
-  Instruction *visitUnreachableInst(UnreachableInst &I);
   Instruction *
   foldAggregateConstructionIntoAggregateReuse(InsertValueInst &OrigIVI);
   Instruction *visitInsertValueInst(InsertValueInst &IV);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D87399.290771.patch
Type: text/x-patch
Size: 3786 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200909/231732d0/attachment.bin>