[llvm] 4e413e1 - [InstCombine] Temporarily do not drop volatile stores before unreachable

[Nikita Popov via llvm-commits]

Author: Nikita Popov
Date: 2020-09-10T16:16:44+02:00
New Revision: 4e413e16216d0c94ada2171f3c59e0a85f4fa4b6

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

LOG: [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.

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 0ca256860c59..63ba7eb85c66 100644
--- 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 c2f63d6659f0..105ec83056d6 100644
--- 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
+}