[PATCH] D23237: [MSSA] Fix PR28880 by fixing stack popping behavior.

[Daniel Berlin via llvm-commits]

dberlin created this revision.
dberlin added a reviewer: george.burgess.iv.
dberlin added subscribers: sebpop, llvm-commits, MatzeB.

In the use optimizer, we need to keep track of the stack size after popping off dominating accesses, or else we may decide a lower bound is still valid when it is not due to
intervening pushes.
Fixes PR28880 (and probably a bunch of other things).

https://reviews.llvm.org/D23237

Files:
  lib/Transforms/Utils/MemorySSA.cpp
  test/Transforms/Util/MemorySSA/pr28880.ll

Index: test/Transforms/Util/MemorySSA/pr28880.ll
===================================================================
--- /dev/null
+++ test/Transforms/Util/MemorySSA/pr28880.ll
@@ -0,0 +1,54 @@
+; RUN: opt -basicaa -print-memoryssa -verify-memoryssa -analyze < %s 2>&1 | FileCheck %s
+; RUN: opt -aa-pipeline=basic-aa -passes='print<memoryssa>,verify<memoryssa>' -disable-output < %s 2>&1 | FileCheck %s
+
+; This testcase is reduced from SingleSource/Benchmarks/Misc/fbench.c
+; It is testing to make sure that the MemorySSA use optimizer
+; comes up with right answers when dealing with multiple MemoryLocations
+; over different blocks. See PR28880 for more details.
+ at global = external hidden unnamed_addr global double, align 8
+ at global.1 = external hidden unnamed_addr global double, align 8
+
+; Function Attrs: nounwind ssp uwtable
+define hidden fastcc void @hoge() unnamed_addr #0 {
+bb:
+  br i1 undef, label %bb1, label %bb2
+
+bb1:                                              ; preds = %bb
+; These accesses should not conflict.
+; CHECK:  1 = MemoryDef(liveOnEntry)
+; 1 = MemoryDef(liveOnEntry)
+; CHECK-NEXT:   store double undef, double* @global, align 8
+  store double undef, double* @global, align 8
+; CHECK:  MemoryUse(liveOnEntry)
+; MemoryUse(liveOnEntry)
+; CHECK-NEXT:   %tmp = load double, double* @global.1, align 8
+  %tmp = load double, double* @global.1, align 8
+  unreachable
+
+bb2:                                              ; preds = %bb
+  br label %bb3
+
+bb3:                                              ; preds = %bb2
+  br i1 undef, label %bb4, label %bb6
+
+bb4:                                              ; preds = %bb3
+; These accesses should conflict.
+; CHECK:  2 = MemoryDef(liveOnEntry)
+; 2 = MemoryDef(liveOnEntry)
+; CHECK-NEXT:   store double 0.000000e+00, double* @global.1, align 8
+  store double 0.000000e+00, double* @global.1, align 8
+; CHECK:  MemoryUse(2)
+; MemoryUse(2)
+; CHECK-NEXT:   %tmp5 = load double, double* @global.1, align 8
+  %tmp5 = load double, double* @global.1, align 8
+  unreachable
+
+bb6:                                              ; preds = %bb3
+  unreachable
+}
+
+attributes #0 = { nounwind ssp uwtable "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="core2" "target-features"="+cx16,+fxsr,+mmx,+sse,+sse2,+sse3,+ssse3" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
+!llvm.ident = !{!0}
+
+!0 = !{!"Apple LLVM version 8.0.0 (clang-800.0.35)"}
Index: lib/Transforms/Utils/MemorySSA.cpp
===================================================================
--- lib/Transforms/Utils/MemorySSA.cpp
+++ lib/Transforms/Utils/MemorySSA.cpp
@@ -1333,7 +1333,10 @@
       VersionStack.pop_back();
     ++PopEpoch;
   }
-
+  // We have to track the size after we first popped, because we may
+  // see later pushes before we see uses, and we need to be able to
+  // tell if any LowerBounds pointed into old parts of the stack.
+  unsigned SizeAfterPop = VersionStack.size();
   for (MemoryAccess &MA : *Accesses) {
     auto *MU = dyn_cast<MemoryUse>(&MA);
     if (!MU) {
@@ -1356,7 +1359,7 @@
       LocInfo.PopEpoch = PopEpoch;
       LocInfo.StackEpoch = StackEpoch;
       // If the lower bound was in the info we popped, we have to reset it.
-      if (LocInfo.LowerBound >= VersionStack.size()) {
+      if (LocInfo.LowerBound >= SizeAfterPop) {
         // Reset the lower bound of things to check.
         // TODO: Some day we should be able to reset to last kill, rather than
         // 0.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D23237.67075.patch
Type: text/x-patch
Size: 3668 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160806/b00f5663/attachment.bin>