[llvm] r278401 - Fix PR 28933

[Daniel Berlin via llvm-commits]

Author: dannyb
Date: Thu Aug 11 15:32:43 2016
New Revision: 278401

URL: http://llvm.org/viewvc/llvm-project?rev=278401&view=rev
Log:
Fix PR 28933

Summary:
This fixes PR 28933 by making sure GVNHoist does not try to recreate memory
accesses when it has not actually moved them.

Reviewers: sebpop

Subscribers: llvm-commits, george.burgess.iv

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

Added:
    llvm/trunk/test/Transforms/GVN/hoist-pr28933.ll
Modified:
    llvm/trunk/lib/Transforms/Scalar/GVNHoist.cpp

Modified: llvm/trunk/lib/Transforms/Scalar/GVNHoist.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/GVNHoist.cpp?rev=278401&r1=278400&r2=278401&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/GVNHoist.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/GVNHoist.cpp Thu Aug 11 15:32:43 2016
@@ -740,10 +740,14 @@ private:
           if (!Repl || firstInBB(I, Repl))
             Repl = I;
 
+      // Keep track of whether we moved the instruction so we know whether we
+      // should move the MemoryAccess.
+      bool MoveAccess = true;
       if (Repl) {
         // Repl is already in HoistPt: it remains in place.
         assert(allOperandsAvailable(Repl, HoistPt) &&
                "instruction depends on operands that are not available");
+        MoveAccess = false;
       } else {
         // When we do not find Repl in HoistPt, select the first in the list
         // and move it to HoistPt.
@@ -771,14 +775,16 @@ private:
         DFSNumber[Repl] = DFSNumber[Last]++;
       }
 
-      MemoryAccess *NewMemAcc = nullptr;
-      if (MemoryAccess *MA = MSSA->getMemoryAccess(Repl)) {
-        if (MemoryUseOrDef *OldMemAcc = dyn_cast<MemoryUseOrDef>(MA)) {
+      MemoryAccess *NewMemAcc = MSSA->getMemoryAccess(Repl);
+
+      if (MoveAccess) {
+        if (MemoryUseOrDef *OldMemAcc =
+                dyn_cast_or_null<MemoryUseOrDef>(NewMemAcc)) {
           // The definition of this ld/st will not change: ld/st hoisting is
           // legal when the ld/st is not moved past its current definition.
           MemoryAccess *Def = OldMemAcc->getDefiningAccess();
-          NewMemAcc = MSSA->createMemoryAccessInBB(Repl, Def, HoistPt,
-                                                   MemorySSA::End);
+          NewMemAcc =
+              MSSA->createMemoryAccessInBB(Repl, Def, HoistPt, MemorySSA::End);
           OldMemAcc->replaceAllUsesWith(NewMemAcc);
           MSSA->removeMemoryAccess(OldMemAcc);
         }

Added: llvm/trunk/test/Transforms/GVN/hoist-pr28933.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVN/hoist-pr28933.ll?rev=278401&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/GVN/hoist-pr28933.ll (added)
+++ llvm/trunk/test/Transforms/GVN/hoist-pr28933.ll Thu Aug 11 15:32:43 2016
@@ -0,0 +1,21 @@
+; RUN: opt -S -gvn-hoist -verify-memoryssa < %s | FileCheck %s
+
+; Check that we end up with one load and one store, in the right order
+; CHECK-LABEL:  define void @test_it(
+; CHECK: store
+; CHECK-NEXT: load
+; CHECK-NOT: store
+; CHECK-NOT: load
+        
+%rec894.0.1.2.3.12 = type { i16 }
+
+ at a = external global %rec894.0.1.2.3.12
+
+define void @test_it() {
+bb2:
+  store i16 undef, i16* getelementptr inbounds (%rec894.0.1.2.3.12, %rec894.0.1.2.3.12* @a, i16 0, i32 0), align 1
+  %_tmp61 = load i16, i16* getelementptr inbounds (%rec894.0.1.2.3.12, %rec894.0.1.2.3.12* @a, i16 0, i32 0), align 1
+  store i16 undef, i16* getelementptr inbounds (%rec894.0.1.2.3.12, %rec894.0.1.2.3.12* @a, i16 0, i32 0), align 1
+  %_tmp92 = load i16, i16* getelementptr inbounds (%rec894.0.1.2.3.12, %rec894.0.1.2.3.12* @a, i16 0, i32 0), align 1
+  ret void
+}