[llvm] r291421 - NewGVN: Fix PR 31573, a failure to verify memory congruency due to

[Daniel Berlin via llvm-commits]

Author: dannyb
Date: Sun Jan  8 23:34:29 2017
New Revision: 291421

URL: http://llvm.org/viewvc/llvm-project?rev=291421&view=rev
Log:
NewGVN: Fix PR 31573, a failure to verify memory congruency due to
not excluding ourselves when checking if any equivalent stores
exist.

Added:
    llvm/trunk/test/Transforms/NewGVN/pr31573.ll
Modified:
    llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp

Modified: llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp?rev=291421&r1=291420&r2=291421&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp Sun Jan  8 23:34:29 2017
@@ -714,6 +714,15 @@ const StoreExpression *NewGVN::createSto
   return E;
 }
 
+// Utility function to check whether the congruence class has a member other
+// than the given instruction.
+bool hasMemberOtherThanUs(const CongruenceClass *CC, Instruction *I) {
+  // Either it has more than one member, in which case it must contain something
+  // other than us (because it's indexed by value), or if it only has one member
+  // right now, that member should not be us.
+  return CC->Members.size() > 1 || CC->Members.count(I) == 0;
+}
+
 const Expression *NewGVN::performSymbolicStoreEvaluation(Instruction *I,
                                                          const BasicBlock *B) {
   // Unlike loads, we never try to eliminate stores, so we do not check if they
@@ -729,8 +738,12 @@ const Expression *NewGVN::performSymboli
         cast<MemoryDef>(StoreAccess)->getDefiningAccess());
     const Expression *OldStore = createStoreExpression(SI, StoreRHS, B);
     CongruenceClass *CC = ExpressionToClass.lookup(OldStore);
+    // Basically, check if the congruence class the store is in is defined by a
+    // store that isn't us, and has the same value.  MemorySSA takes care of
+    // ensuring the store has the same memory state as us already.
     if (CC && CC->DefiningExpr && isa<StoreExpression>(CC->DefiningExpr) &&
-        CC->RepLeader == lookupOperandLeader(SI->getValueOperand(), SI, B))
+        CC->RepLeader == lookupOperandLeader(SI->getValueOperand(), SI, B) &&
+        hasMemberOtherThanUs(CC, I))
       return createStoreExpression(SI, StoreRHS, B);
   }
 

Added: llvm/trunk/test/Transforms/NewGVN/pr31573.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/NewGVN/pr31573.ll?rev=291421&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/NewGVN/pr31573.ll (added)
+++ llvm/trunk/test/Transforms/NewGVN/pr31573.ll Sun Jan  8 23:34:29 2017
@@ -0,0 +1,42 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -basicaa -newgvn -S | FileCheck %s
+
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+
+define void @patatino(i8* %blah) {
+; CHECK-LABEL: @patatino(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[WHILE_COND:%.*]]
+; CHECK:       while.cond:
+; CHECK-NEXT:    [[MEH:%.*]] = phi i8* [ [[BLAH:%.*]], [[ENTRY:%.*]] ], [ null, [[WHILE_BODY:%.*]] ]
+; CHECK-NEXT:    switch i32 undef, label [[WHILE_BODY]] [
+; CHECK-NEXT:    i32 666, label [[WHILE_END:%.*]]
+; CHECK-NEXT:    ]
+; CHECK:       while.body:
+; CHECK-NEXT:    br label [[WHILE_COND]]
+; CHECK:       while.end:
+; CHECK-NEXT:    store i8 0, i8* [[MEH]], align 1
+; CHECK-NEXT:    store i8 0, i8* [[BLAH]], align 1
+; CHECK-NEXT:    ret void
+;
+entry:
+  br label %while.cond
+
+while.cond:
+  %meh = phi i8* [ %blah, %entry ], [ null, %while.body ]
+  switch i32 undef, label %while.body [
+  i32 666, label %while.end
+  ]
+
+while.body:
+  br label %while.cond
+
+while.end:
+;; These two stores will initially be considered equivalent, but then proven not.
+;; the second store would previously end up deciding it's equivalent to a previous
+;; store, but it was really just finding an optimistic version of itself
+;; in the congruence class.
+  store i8 0, i8* %meh, align 1
+  store i8 0, i8* %blah, align 1
+  ret void
+}