[llvm] r367703 - [NewPassManager] Resolve assertion in CGSCCPassManager when CallCounts change.

[Alina Sbirlea via llvm-commits]

Author: asbirlea
Date: Fri Aug  2 11:37:03 2019
New Revision: 367703

URL: http://llvm.org/viewvc/llvm-project?rev=367703&view=rev
Log:
[NewPassManager] Resolve assertion in CGSCCPassManager when CallCounts change.

Summary:
If the CallCounts change after an iteration of the DevirtSCCRepeatedPass, this is not reflected in the local CallCounts structure triggering the assertion checking the before/after sizes.
Since it is valid for the size to change and this only uses the CallCounts for the devirtualizing heuristic, keep a <Function*, CallCount> map instead, and make the devirtualizing decision using the counts for the functions that exist both before and after the pass.

Resolves PR42726.

Reviewers: chandlerc

Subscribers: mehdi_amini, jlebar, sanjoy.google, llvm-commits

Tags: #llvm

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

Added:
    llvm/trunk/test/Other/new-pm-pr42726-cgscc.ll
Modified:
    llvm/trunk/include/llvm/Analysis/CGSCCPassManager.h

Modified: llvm/trunk/include/llvm/Analysis/CGSCCPassManager.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/CGSCCPassManager.h?rev=367703&r1=367702&r2=367703&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Analysis/CGSCCPassManager.h (original)
+++ llvm/trunk/include/llvm/Analysis/CGSCCPassManager.h Fri Aug  2 11:37:03 2019
@@ -88,6 +88,7 @@
 #ifndef LLVM_ANALYSIS_CGSCCPASSMANAGER_H
 #define LLVM_ANALYSIS_CGSCCPASSMANAGER_H
 
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/PriorityWorklist.h"
 #include "llvm/ADT/STLExtras.h"
@@ -583,10 +584,12 @@ public:
                       SmallVectorImpl<WeakTrackingVH> &CallHandles) {
       assert(CallHandles.empty() && "Must start with a clear set of handles.");
 
-      SmallVector<CallCount, 4> CallCounts;
+      SmallDenseMap<Function *, CallCount> CallCounts;
+      CallCount CountLocal = {0, 0};
       for (LazyCallGraph::Node &N : C) {
-        CallCounts.push_back({0, 0});
-        CallCount &Count = CallCounts.back();
+        CallCount &Count =
+            CallCounts.insert(std::make_pair(&N.getFunction(), CountLocal))
+                .first->second;
         for (Instruction &I : instructions(N.getFunction()))
           if (auto CS = CallSite(&I)) {
             if (CS.getCalledFunction()) {
@@ -626,8 +629,6 @@ public:
       // Check that we didn't miss any update scenario.
       assert(!UR.InvalidatedSCCs.count(C) && "Processing an invalid SCC!");
       assert(C->begin() != C->end() && "Cannot have an empty SCC!");
-      assert((int)CallCounts.size() == C->size() &&
-             "Cannot have changed the size of the SCC!");
 
       // Check whether any of the handles were devirtualized.
       auto IsDevirtualizedHandle = [&](WeakTrackingVH &CallH) {
@@ -642,7 +643,7 @@ public:
         if (!F)
           return false;
 
-        LLVM_DEBUG(dbgs() << "Found devirutalized call from "
+        LLVM_DEBUG(dbgs() << "Found devirtualized call from "
                           << CS.getParent()->getParent()->getName() << " to "
                           << F->getName() << "\n");
 
@@ -664,12 +665,20 @@ public:
       // manner of transformations such as DCE and other things, but seems to
       // work well in practice.
       if (!Devirt)
-        for (int i = 0, Size = C->size(); i < Size; ++i)
-          if (CallCounts[i].Indirect > NewCallCounts[i].Indirect &&
-              CallCounts[i].Direct < NewCallCounts[i].Direct) {
-            Devirt = true;
-            break;
+        // Iterate over the keys in NewCallCounts, if Function also exists in
+        // CallCounts, make the check below.
+        for (auto &Pair : NewCallCounts) {
+          auto &CallCountNew = Pair.second;
+          auto CountIt = CallCounts.find(Pair.first);
+          if (CountIt != CallCounts.end()) {
+            const auto &CallCountOld = CountIt->second;
+            if (CallCountOld.Indirect > CallCountNew.Indirect &&
+                CallCountOld.Direct < CallCountNew.Direct) {
+              Devirt = true;
+              break;
+            }
           }
+        }
 
       if (!Devirt) {
         PA.intersect(std::move(PassPA));

Added: llvm/trunk/test/Other/new-pm-pr42726-cgscc.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Other/new-pm-pr42726-cgscc.ll?rev=367703&view=auto
==============================================================================
--- llvm/trunk/test/Other/new-pm-pr42726-cgscc.ll (added)
+++ llvm/trunk/test/Other/new-pm-pr42726-cgscc.ll Fri Aug  2 11:37:03 2019
@@ -0,0 +1,57 @@
+; RUN: opt -aa-pipeline=default -passes="default<O1>" %s -S | FileCheck %s
+; REQUIRES: asserts
+
+declare void @bar()
+declare void @baz(i32*)
+
+; CHECK-LABEL: @foo1()
+define void @foo1() {
+entry:
+  %tag = alloca i32, align 4
+  call void @baz(i32* %tag)
+  %tmp = load i32, i32* %tag, align 4
+  switch i32 %tmp, label %sw.bb799 [
+    i32 10, label %sw.bb239
+  ]
+
+sw.bb239:
+  call void @foo2()
+  br label %cleanup871
+
+sw.bb799:
+  call void @foo3(i32 undef)
+  br label %cleanup871
+
+cleanup871:
+  call void @bar()
+  unreachable
+}
+
+define void @foo2() {
+  call void @foo4()
+  unreachable
+}
+
+define void @foo3(i32 %ptr) {
+  call void @foo1()
+  unreachable
+}
+
+define void @foo4() {
+entry:
+  %tag = alloca i32, align 4
+  call void @baz(i32* %tag)
+  %tmp = load i32, i32* %tag, align 4
+  switch i32 %tmp, label %sw.bb442 [
+    i32 16, label %sw.bb352
+  ]
+
+sw.bb352:
+  call void @foo3(i32 undef)
+  unreachable
+
+sw.bb442:
+  call void @foo2()
+  unreachable
+}
+