[InstCombine] Temporarily do not drop volatile stores before unreachable

See discussion in D87149. Dropping volatile stores here is legal
per LLVM semantics, but causes issues for real code and may result
in a change to LLVM volatile semantics. Temporarily treat volatile
stores as "not guaranteed to transfer execution" in just this place,
until this issue has been resolved.

diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 0ca2568..63ba7eb 100644 (file)
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -2805,6 +2805,14 @@ Instruction *InstCombinerImpl::visitUnreachableInst(UnreachableInst &I) {
   Instruction *Prev = I.getPrevNonDebugInstruction();
   if (Prev && !Prev->isEHPad() &&
       isGuaranteedToTransferExecutionToSuccessor(Prev)) {
+    // 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;
+
     eraseInstFromFunction(*Prev);
     return &I;
   }

diff --git a/llvm/test/Transforms/InstCombine/volatile_store.ll b/llvm/test/Transforms/InstCombine/volatile_store.ll
index c2f63d6..105ec83 100644 (file)
--- a/llvm/test/Transforms/InstCombine/volatile_store.ll
+++ b/llvm/test/Transforms/InstCombine/volatile_store.ll
@@ -1,4 +1,4 @@
-; NOTE: Assertions have been autogenerated by update_test_checks.py
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -instcombine -S | FileCheck %s
 
 @x = weak global i32 0
@@ -8,7 +8,7 @@ define void @self_assign_1() {
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[TMP:%.*]] = load volatile i32, i32* @x, align 4
 ; CHECK-NEXT:    store volatile i32 [[TMP]], i32* @x, align 4
-; CHECK-NEXT:    br label %return
+; CHECK-NEXT:    br label [[RETURN:%.*]]
 ; CHECK:       return:
 ; CHECK-NEXT:    ret void
 ;
@@ -20,3 +20,22 @@ entry:
 return:
   ret void
 }
+
+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
+;
+  br i1 %c, label %true, label %false
+
+true:
+  store volatile i8 0, i8* %p
+  unreachable
+
+false:
+  ret void
+}