From: Daniel Berlin <dberlin@dberlin.org>
Date: Fri, 20 Jan 2017 06:38:41 +0000 (+0000)
Subject: NewGVN: Fix PR 31682, an overactive assert.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=a0431ff170c0fe1e74b353807ef363e017b39be8;p=llvm

NewGVN: Fix PR 31682, an overactive assert.
Part of the assert has been left active for further debugging.
The other part has been turned into a stat for tracking for the
moment.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@292583 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/lib/Transforms/Scalar/NewGVN.cpp b/lib/Transforms/Scalar/NewGVN.cpp
index 0262fa34981..91ccf2ee051 100644
--- a/lib/Transforms/Scalar/NewGVN.cpp
+++ b/lib/Transforms/Scalar/NewGVN.cpp
@@ -85,6 +85,8 @@ STATISTIC(NumGVNLeaderChanges, "Number of leader changes");
 STATISTIC(NumGVNSortedLeaderChanges, "Number of sorted leader changes");
 STATISTIC(NumGVNAvoidedSortedLeaderChanges,
           "Number of avoided sorted leader changes");
+STATISTIC(NumGVNNotMostDominatingLeader,
+          "Number of times a member dominated it's new classes' leader");
 
 //===----------------------------------------------------------------------===//
 //                                GVN Pass
@@ -1071,16 +1073,20 @@ void NewGVN::moveValueToNewCongruenceClass(Instruction *I,
   if (I == OldClass->NextLeader.first)
     OldClass->NextLeader = {nullptr, ~0U};
 
-  // The new instruction and new class leader may either be siblings in the
-  // dominator tree, or the new class leader should dominate the new member
-  // instruction.  We simply check that the member instruction does not properly
-  // dominate the new class leader.
-  assert((!isa<Instruction>(NewClass->RepLeader) || !NewClass->RepLeader ||
-          I == NewClass->RepLeader ||
-          !DT->properlyDominates(
-              I->getParent(),
-              cast<Instruction>(NewClass->RepLeader)->getParent())) &&
-         "New class for instruction should not be dominated by instruction");
+  // It's possible, though unlikely, for us to discover equivalences such
+  // that the current leader does not dominate the old one.
+  // This statistic tracks how often this happens.
+  // We assert on phi nodes when this happens, currently, for debugging, because
+  // we want to make sure we name phi node cycles properly.
+  if (isa<Instruction>(NewClass->RepLeader) && NewClass->RepLeader &&
+      I != NewClass->RepLeader &&
+      DT->properlyDominates(
+          I->getParent(),
+          cast<Instruction>(NewClass->RepLeader)->getParent())) {
+    ++NumGVNNotMostDominatingLeader;
+    assert(!isa<PHINode>(I) &&
+           "New class for instruction should not be dominated by instruction");
+  }
 
   if (NewClass->RepLeader != I) {
     auto DFSNum = InstrDFS.lookup(I);
diff --git a/test/Transforms/NewGVN/pr31682.ll b/test/Transforms/NewGVN/pr31682.ll
new file mode 100644
index 00000000000..108e1e19afb
--- /dev/null
+++ b/test/Transforms/NewGVN/pr31682.ll
@@ -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"
+
+%struct.foo = type { i32, i32, [2 x [4 x [6 x [6 x i16]]]] }
+
+@global = external global %struct.foo*
+
+define void @bar() {
+; CHECK-LABEL: @bar(
+; CHECK-NEXT:  bb:
+; CHECK-NEXT:    [[TMP:%.*]] = load %struct.foo*, %struct.foo** @global
+; CHECK-NEXT:    br label [[BB2:%.*]]
+; CHECK:       bb2:
+; CHECK-NEXT:    [[TMP4:%.*]] = getelementptr [[STRUCT_FOO:%.*]], %struct.foo* [[TMP]], i64 0, i32 1
+; CHECK-NEXT:    br i1 undef, label [[BB2]], label [[BB7:%.*]]
+; CHECK:       bb7:
+; CHECK-NEXT:    br label [[BB10:%.*]]
+; CHECK:       bb10:
+; CHECK-NEXT:    br label [[BB10]]
+;
+bb:
+  %tmp = load %struct.foo*, %struct.foo** @global
+  %tmp1 = getelementptr %struct.foo, %struct.foo* %tmp
+  br label %bb2
+
+bb2:                                              ; preds = %bb2, %bb
+  %tmp3 = phi %struct.foo* [ undef, %bb ], [ %tmp6, %bb2 ]
+  %tmp4 = getelementptr %struct.foo, %struct.foo* %tmp3, i64 0, i32 1
+  %tmp5 = load i32, i32* %tmp4
+  %tmp6 = load %struct.foo*, %struct.foo** @global
+  br i1 undef, label %bb2, label %bb7
+
+bb7:                                              ; preds = %bb2
+  %tmp8 = phi %struct.foo* [ %tmp6, %bb2 ]
+  %tmp9 = getelementptr %struct.foo, %struct.foo* %tmp8, i64 0, i32 1
+  br label %bb10
+
+bb10:                                             ; preds = %bb10, %bb7
+  %tmp11 = load i32, i32* %tmp9
+  br label %bb10
+}