[llvm] da81ec6 - [SimplifyCFG] Volatile memory operations do not trap

[Roman Lebedev via llvm-commits]

Author: Roman Lebedev
Date: 2021-07-02T21:47:44+03:00
New Revision: da81ec61585b717d7f0c07aa3e2b5460eeeb6ce3

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

LOG: [SimplifyCFG] Volatile memory operations do not trap

Somewhat related to D105338.
While it is up for discussion whether or not volatile store traps,
so far there has been no complaints that volatile load/cmpxchg/atomicrmw also may trap.
And even if simplifycfg currently concervatively believes that to be the case,
instcombine does not: https://godbolt.org/z/5vhv4K5b8

Reviewed By: nikic

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

Added: 
    

Modified: 
    llvm/lib/Transforms/Utils/SimplifyCFG.cpp
    llvm/test/Transforms/SimplifyCFG/trapping-load-unreachable.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 6288a62326554..8294a79a1c0a1 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -4673,17 +4673,13 @@ bool SimplifyCFGOpt::simplifyUnreachable(UnreachableInst *UI) {
 
     if (BBI->mayHaveSideEffects()) {
       if (auto *SI = dyn_cast<StoreInst>(BBI)) {
+        // 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 (SI->isVolatile())
           break;
-      } else if (auto *LI = dyn_cast<LoadInst>(BBI)) {
-        if (LI->isVolatile())
-          break;
-      } else if (auto *RMWI = dyn_cast<AtomicRMWInst>(BBI)) {
-        if (RMWI->isVolatile())
-          break;
-      } else if (auto *CXI = dyn_cast<AtomicCmpXchgInst>(BBI)) {
-        if (CXI->isVolatile())
-          break;
       } else if (isa<CatchPadInst>(BBI)) {
         // A catchpad may invoke exception object constructors and such, which
         // in some languages can be arbitrary code, so be conservative by
@@ -4692,8 +4688,9 @@ bool SimplifyCFGOpt::simplifyUnreachable(UnreachableInst *UI) {
         if (classifyEHPersonality(BB->getParent()->getPersonalityFn()) !=
             EHPersonality::CoreCLR)
           break;
-      } else if (!isa<FenceInst>(BBI) && !isa<VAArgInst>(BBI) &&
-                 !isa<LandingPadInst>(BBI)) {
+      } else if (!isa<LoadInst>(BBI) && !isa<AtomicRMWInst>(BBI) &&
+                 !isa<AtomicCmpXchgInst>(BBI) && !isa<FenceInst>(BBI) &&
+                 !isa<VAArgInst>(BBI) && !isa<LandingPadInst>(BBI)) {
         break;
       }
       // Note that deleting LandingPad's here is in fact okay, although it

diff  --git a/llvm/test/Transforms/SimplifyCFG/trapping-load-unreachable.ll b/llvm/test/Transforms/SimplifyCFG/trapping-load-unreachable.ll
index b277cb6cf4f9a..e437f40cbe753 100644
--- a/llvm/test/Transforms/SimplifyCFG/trapping-load-unreachable.ll
+++ b/llvm/test/Transforms/SimplifyCFG/trapping-load-unreachable.ll
@@ -10,11 +10,8 @@ define void @test1(i32 %x) nounwind {
 ; CHECK-LABEL: @test1(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[TMP0:%.*]] = icmp eq i32 [[X:%.*]], 0
-; CHECK-NEXT:    br i1 [[TMP0]], label [[BB:%.*]], label [[RETURN:%.*]]
-; CHECK:       bb:
-; CHECK-NEXT:    [[TMP1:%.*]] = load volatile i32, i32* null, align 4
-; CHECK-NEXT:    unreachable
-; CHECK:       return:
+; CHECK-NEXT:    [[TMP1:%.*]] = xor i1 [[TMP0]], true
+; CHECK-NEXT:    call void @llvm.assume(i1 [[TMP1]])
 ; CHECK-NEXT:    ret void
 ;
 entry:
@@ -34,11 +31,8 @@ define void @test1_no_null_opt(i32 %x) nounwind #0 {
 ; CHECK-LABEL: @test1_no_null_opt(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[TMP0:%.*]] = icmp eq i32 [[X:%.*]], 0
-; CHECK-NEXT:    br i1 [[TMP0]], label [[BB:%.*]], label [[RETURN:%.*]]
-; CHECK:       bb:
-; CHECK-NEXT:    [[TMP1:%.*]] = load volatile i32, i32* null, align 4
-; CHECK-NEXT:    unreachable
-; CHECK:       return:
+; CHECK-NEXT:    [[TMP1:%.*]] = xor i1 [[TMP0]], true
+; CHECK-NEXT:    call void @llvm.assume(i1 [[TMP1]])
 ; CHECK-NEXT:    ret void
 ;
 entry:
@@ -127,11 +121,8 @@ F:
 define void @test5(i1 %C, i32* %P) {
 ; CHECK-LABEL: @test5(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    br i1 [[C:%.*]], label [[T:%.*]], label [[F:%.*]]
-; CHECK:       T:
-; CHECK-NEXT:    [[TMP0:%.*]] = cmpxchg volatile i32* [[P:%.*]], i32 0, i32 1 seq_cst seq_cst, 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:
@@ -147,11 +138,8 @@ F:
 define void @test6(i1 %C, i32* %P) {
 ; CHECK-LABEL: @test6(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    br i1 [[C:%.*]], label [[T:%.*]], label [[F:%.*]]
-; CHECK:       T:
-; CHECK-NEXT:    [[TMP0:%.*]] = atomicrmw volatile xchg i32* [[P:%.*]], i32 0 seq_cst, 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: