| commit 52aeacfbf5ce5f949efe0eae029e56db171ea1f7 |
| Author: Roman Lebedev <lebedev.ri@gmail.com> |
| Date: Fri Jul 9 13:36:21 2021 +0300 |
| |
| Revert "Temporarily do not drop volatile stores before unreachable" |
| |
| This reverts commit 4e413e16216d0c94ada2171f3c59e0a85f4fa4b6, |
| which landed almost 10 months ago under premise that the original behavior |
| didn't match reality and was breaking users, even though it was correct as per |
| the LangRef. But the LangRef change still hasn't appeared, which might suggest |
| that the affected parties aren't really worried about this problem. |
| |
| Please refer to discussion in: |
| * https://reviews.llvm.org/D87399 (`Revert "[InstCombine] erase instructions leading up to unreachable"`) |
| * https://reviews.llvm.org/D53184 (`[LangRef] Clarify semantics of volatile operations.`) |
| * https://reviews.llvm.org/D87149 (`[InstCombine] erase instructions leading up to unreachable`) |
| |
| clang has `-Wnull-dereference` which will diagnose the obvious cases |
| of null dereference, it was adjusted in f4877c78c0fc98be47b926439bbfe33d5e1d1b6d, |
| but it will only catch the cases where the pointer is a null literal, |
| it will not catch the cases where an arbitrary store is expected to trap. |
| |
| Differential Revision: https://reviews.llvm.org/D105338 |
| |
| diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp |
| index e00bcf8826d0..7e4b7d0b636c 100644 |
| --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp |
| +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp |
| @@ -2888,14 +2888,6 @@ Instruction *InstCombinerImpl::visitUnreachableInst(UnreachableInst &I) { |
| // Otherwise, this instruction can be freely erased, |
| // even if it is not side-effect free. |
| |
| - // Temporarily disable removal of volatile stores preceding unreachable, |
| - // pending a potential LangRef change permitting volatile stores to trap. |
| - // TODO: Either remove this code, or properly integrate the check into |
| - // isGuaranteedToTransferExecutionToSuccessor(). |
| - if (auto *SI = dyn_cast<StoreInst>(Prev)) |
| - if (SI->isVolatile()) |
| - return nullptr; // Can not drop this instruction. We're done here. |
| - |
| // A value may still have uses before we process it here (for example, in |
| // another unreachable block), so convert those to poison. |
| replaceInstUsesWith(*Prev, PoisonValue::get(Prev->getType())); |
| diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp |
| index cfbc24c0001f..2ddf75229f41 100644 |
| --- a/llvm/lib/Transforms/Utils/Local.cpp |
| +++ b/llvm/lib/Transforms/Utils/Local.cpp |
| @@ -2297,9 +2297,6 @@ static bool markAliveBlocks(Function &F, |
| // that they should be changed to unreachable by passes that can't |
| // modify the CFG. |
| |
| - // Don't touch volatile stores. |
| - if (SI->isVolatile()) continue; |
| - |
| Value *Ptr = SI->getOperand(1); |
| |
| if (isa<UndefValue>(Ptr) || |
| diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp |
| index f08ab18b15b2..fa4b8c9a28ce 100644 |
| --- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp |
| +++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp |
| @@ -4672,14 +4672,6 @@ bool SimplifyCFGOpt::simplifyUnreachable(UnreachableInst *UI) { |
| // Otherwise, this instruction can be freely erased, |
| // even if it is not side-effect free. |
| |
| - // Temporarily disable removal of volatile stores preceding unreachable, |
| - // pending a potential LangRef change permitting volatile stores to trap. |
| - // TODO: Either remove this code, or properly integrate the check into |
| - // isGuaranteedToTransferExecutionToSuccessor(). |
| - if (auto *SI = dyn_cast<StoreInst>(&*BBI)) |
| - if (SI->isVolatile()) |
| - break; // Can not drop this instruction. We're done here. |
| - |
| // Note that deleting EH's here is in fact okay, although it involves a bit |
| // of subtle reasoning. If this inst is an EH, all the predecessors of this |
| // block will be the unwind edges of Invoke/CatchSwitch/CleanupReturn, |
| diff --git a/llvm/test/CodeGen/X86/indirect-branch-tracking-eh2.ll b/llvm/test/CodeGen/X86/indirect-branch-tracking-eh2.ll |
| index b6a6ff35ea9c..32e984df8eb3 100644 |
| --- a/llvm/test/CodeGen/X86/indirect-branch-tracking-eh2.ll |
| +++ b/llvm/test/CodeGen/X86/indirect-branch-tracking-eh2.ll |
| @@ -3,51 +3,38 @@ |
| |
| ; NUM-COUNT-3: endbr64 |
| |
| -;SJLJ: main: # @main |
| -;SJLJ-NEXT: .Lfunc_begin0: |
| -;SJLJ-NEXT: # %bb.0: # %entry |
| -;SJLJ-NEXT: endbr64 |
| -;SJLJ-NEXT: pushq %rbp |
| -;SJLJ: callq _Unwind_SjLj_Register |
| -;SJLJ-NEXT: .Ltmp0: |
| -;SJLJ-NEXT: callq _Z3foov |
| -;SJLJ-NEXT: .Ltmp1: |
| -;SJLJ-NEXT: # %bb.1: # %invoke.cont |
| -;SJLJ-NEXT: movl |
| -;SJLJ-NEXT: .LBB0_7: # %return |
| -;SJLJ: callq _Unwind_SjLj_Unregister |
| -;SJLJ: retq |
| -;SJLJ-NEXT: .LBB0_9: |
| -;SJLJ-NEXT: endbr64 |
| -;SJLJ-NEXT: movl |
| -;SJLJ-NEXT: cmpl |
| -;SJLJ-NEXT: jb .LBB0_10 |
| -;SJLJ-NEXT: # %bb.11: |
| -;SJLJ-NEXT: ud2 |
| -;SJLJ-NEXT: .LBB0_10: |
| -;SJLJ-NEXT: leaq .LJTI0_0(%rip), %rcx |
| -;SJLJ-NEXT: jmpq *(%rcx,%rax,8) |
| -;SJLJ-NEXT: .LBB0_2: # %lpad |
| -;SJLJ-NEXT: .Ltmp2: |
| -;SJLJ-NEXT: endbr64 |
| -;SJLJ: jne .LBB0_4 |
| -;SJLJ-NEXT: # %bb.3: # %catch3 |
| -;SJLJ: callq __cxa_begin_catch |
| -;SJLJ: jmp .LBB0_6 |
| -;SJLJ-NEXT: .LBB0_4: # %catch.fallthrough |
| -;SJLJ-NEXT: cmpl |
| -;SJLJ-NEXT: jne .LBB0_8 |
| -;SJLJ-NEXT: # %bb.5: # %catch |
| -;SJLJ: callq __cxa_begin_catch |
| -;SJLJ: cmpb |
| -;SJLJ-NEXT: .LBB0_6: # %return |
| -;SJLJ: callq __cxa_end_catch |
| -;SJLJ-NEXT: jmp .LBB0_7 |
| -;SJLJ-NEXT: .LBB0_8: # %eh.resume |
| -;SJLJ-NEXT: movl |
| -;SJLJ-NEXT: .Lfunc_end0: |
| -;SJLJ: .LJTI0_0: |
| -;SJLJ-NEXT: .quad .LBB0_2 |
| +; SJLJ-LABEL: main: |
| +; SJLJ: # %bb.0: # %entry |
| +; SJLJ-NEXT: endbr64 |
| +; SJLJ: callq _Unwind_SjLj_Register@PLT |
| +; SJLJ-NEXT: .Ltmp0: |
| +; SJLJ-NEXT: callq _Z3foov |
| +; SJLJ-NEXT: .Ltmp1: |
| +; SJLJ-NEXT: # %bb.1: # %invoke.cont |
| +; SJLJ: .LBB0_6: # %return |
| +; SJLJ: callq _Unwind_SjLj_Unregister@PLT |
| +; SJLJ: retq |
| +; SJLJ-NEXT: .LBB0_7: |
| +; SJLJ-NEXT: endbr64 |
| +; SJLJ: jb .LBB0_8 |
| +; SJLJ-NEXT: # %bb.9: |
| +; SJLJ-NEXT: ud2 |
| +; SJLJ-NEXT: .LBB0_8: |
| +; SJLJ: jmpq *(%rcx,%rax,8) |
| +; SJLJ-NEXT: .LBB0_2: # %lpad |
| +; SJLJ-NEXT: .Ltmp2: |
| +; SJLJ-NEXT: endbr64 |
| +; SJLJ: jne .LBB0_4 |
| +; SJLJ-NEXT: # %bb.3: # %catch3 |
| +; SJLJ: callq __cxa_begin_catch |
| +; SJLJ: jmp .LBB0_5 |
| +; SJLJ-NEXT: .LBB0_4: # %catch |
| +; SJLJ: callq __cxa_begin_catch |
| +; SJLJ: cmpb $3, %al |
| +; SJLJ-NEXT: .LBB0_5: # %return |
| +; SJLJ-NEXT: setne %cl |
| +; SJLJ: callq __cxa_end_catch |
| +; SJLJ-NEXT: jmp .LBB0_6 |
| |
| @_ZTIi = external dso_local constant i8* |
| @_ZTIc = external dso_local constant i8* |
| diff --git a/llvm/test/Transforms/InstCombine/volatile_store.ll b/llvm/test/Transforms/InstCombine/volatile_store.ll |
| index 105ec83056d6..ae9e512afd6c 100644 |
| --- a/llvm/test/Transforms/InstCombine/volatile_store.ll |
| +++ b/llvm/test/Transforms/InstCombine/volatile_store.ll |
| @@ -25,7 +25,6 @@ define void @volatile_store_before_unreachable(i1 %c, i8* %p) { |
| ; CHECK-LABEL: @volatile_store_before_unreachable( |
| ; CHECK-NEXT: br i1 [[C:%.*]], label [[TRUE:%.*]], label [[FALSE:%.*]] |
| ; CHECK: true: |
| -; CHECK-NEXT: store volatile i8 0, i8* [[P:%.*]], align 1 |
| ; CHECK-NEXT: unreachable |
| ; CHECK: false: |
| ; CHECK-NEXT: ret void |
| diff --git a/llvm/test/Transforms/SimplifyCFG/trapping-load-unreachable.ll b/llvm/test/Transforms/SimplifyCFG/trapping-load-unreachable.ll |
| index e437f40cbe75..06b0242f7850 100644 |
| --- a/llvm/test/Transforms/SimplifyCFG/trapping-load-unreachable.ll |
| +++ b/llvm/test/Transforms/SimplifyCFG/trapping-load-unreachable.ll |
| @@ -76,8 +76,8 @@ entry: |
| define void @test3() nounwind { |
| ; CHECK-LABEL: @test3( |
| ; CHECK-NEXT: entry: |
| -; CHECK-NEXT: store volatile i32 4, i32* null, align 4 |
| -; CHECK-NEXT: ret void |
| +; CHECK-NEXT: call void @llvm.trap() |
| +; CHECK-NEXT: unreachable |
| ; |
| entry: |
| store volatile i32 4, i32* null |
| @@ -101,11 +101,8 @@ entry: |
| define void @test4(i1 %C, i32* %P) { |
| ; CHECK-LABEL: @test4( |
| ; CHECK-NEXT: entry: |
| -; CHECK-NEXT: br i1 [[C:%.*]], label [[T:%.*]], label [[F:%.*]] |
| -; CHECK: T: |
| -; CHECK-NEXT: store volatile i32 0, i32* [[P:%.*]], align 4 |
| -; CHECK-NEXT: unreachable |
| -; CHECK: F: |
| +; CHECK-NEXT: [[TMP0:%.*]] = xor i1 [[C:%.*]], true |
| +; CHECK-NEXT: call void @llvm.assume(i1 [[TMP0]]) |
| ; CHECK-NEXT: ret void |
| ; |
| entry: |
