[llvm-branch-commits] [llvm] 6a2799c - Revert "[CGSCC] Detect devirtualization in more cases"
Arthur Eubanks via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Mon Nov 23 13:25:15 PST 2020
Author: Arthur Eubanks
Date: 2020-11-23T13:21:05-08:00
New Revision: 6a2799cf8ecf1b649cfa511aec64256a01f79436
URL: https://github.com/llvm/llvm-project/commit/6a2799cf8ecf1b649cfa511aec64256a01f79436
DIFF: https://github.com/llvm/llvm-project/commit/6a2799cf8ecf1b649cfa511aec64256a01f79436.diff
LOG: Revert "[CGSCC] Detect devirtualization in more cases"
This reverts commit 14a68b4aa9732293ad7e16f105b0feb53dc8dbe2.
Causes building self hosted clang to crash when using NPM.
Added:
Modified:
llvm/include/llvm/Analysis/CGSCCPassManager.h
llvm/lib/Analysis/CGSCCPassManager.cpp
llvm/lib/Passes/PassBuilder.cpp
llvm/test/Transforms/Inline/devirtualize-3.ll
llvm/test/Transforms/Inline/devirtualize.ll
Removed:
llvm/test/Transforms/Inline/devirtualize-5.ll
llvm/test/Transforms/Inline/devirtualize-6.ll
################################################################################
diff --git a/llvm/include/llvm/Analysis/CGSCCPassManager.h b/llvm/include/llvm/Analysis/CGSCCPassManager.h
index b2b690ddd28b..755bc92ddccf 100644
--- a/llvm/include/llvm/Analysis/CGSCCPassManager.h
+++ b/llvm/include/llvm/Analysis/CGSCCPassManager.h
@@ -90,7 +90,6 @@
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/DenseSet.h"
-#include "llvm/ADT/MapVector.h"
#include "llvm/ADT/PriorityWorklist.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallPtrSet.h"
@@ -315,16 +314,6 @@ struct CGSCCUpdateResult {
/// for a better technique.
SmallDenseSet<std::pair<LazyCallGraph::Node *, LazyCallGraph::SCC *>, 4>
&InlinedInternalEdges;
-
- /// Weak VHs to keep track of indirect calls for the purposes of detecting
- /// devirtualization.
- ///
- /// This is a map to avoid having duplicate entries. If a Value is
- /// deallocated, its corresponding WeakTrackingVH will be nulled out. When
- /// checking if a Value is in the map or not, also check if the corresponding
- /// WeakTrackingVH is null to avoid issues with a new Value sharing the same
- /// address as a deallocated one.
- SmallMapVector<Value *, WeakTrackingVH, 16> IndirectVHs;
};
/// The core module pass which does a post-order walk of the SCCs and
@@ -607,6 +596,9 @@ class DevirtSCCRepeatedPass
// a pointer that we can update.
LazyCallGraph::SCC *C = &InitialC;
+ // Collect value handles for all of the indirect call sites.
+ SmallVector<WeakTrackingVH, 8> CallHandles;
+
// Struct to track the counts of direct and indirect calls in each function
// of the SCC.
struct CallCount {
@@ -616,37 +608,35 @@ class DevirtSCCRepeatedPass
// Put value handles on all of the indirect calls and return the number of
// direct calls for each function in the SCC.
- auto ScanSCC =
- [](LazyCallGraph::SCC &C,
- SmallMapVector<Value *, WeakTrackingVH, 16> &CallHandles) {
- assert(CallHandles.empty() &&
- "Must start with a clear set of handles.");
-
- SmallDenseMap<Function *, CallCount> CallCounts;
- CallCount CountLocal = {0, 0};
- for (LazyCallGraph::Node &N : C) {
- CallCount &Count =
- CallCounts.insert(std::make_pair(&N.getFunction(), CountLocal))
- .first->second;
- for (Instruction &I : instructions(N.getFunction()))
- if (auto *CB = dyn_cast<CallBase>(&I)) {
- if (CB->getCalledFunction()) {
- ++Count.Direct;
- } else {
- ++Count.Indirect;
- CallHandles.insert({CB, WeakTrackingVH(CB)});
- }
- }
+ auto ScanSCC = [](LazyCallGraph::SCC &C,
+ SmallVectorImpl<WeakTrackingVH> &CallHandles) {
+ assert(CallHandles.empty() && "Must start with a clear set of handles.");
+
+ SmallDenseMap<Function *, CallCount> CallCounts;
+ CallCount CountLocal = {0, 0};
+ for (LazyCallGraph::Node &N : C) {
+ CallCount &Count =
+ CallCounts.insert(std::make_pair(&N.getFunction(), CountLocal))
+ .first->second;
+ for (Instruction &I : instructions(N.getFunction()))
+ if (auto *CB = dyn_cast<CallBase>(&I)) {
+ if (CB->getCalledFunction()) {
+ ++Count.Direct;
+ } else {
+ ++Count.Indirect;
+ CallHandles.push_back(WeakTrackingVH(&I));
+ }
}
+ }
- return CallCounts;
- };
+ return CallCounts;
+ };
- UR.IndirectVHs.clear();
// Populate the initial call handles and get the initial call counts.
- auto CallCounts = ScanSCC(*C, UR.IndirectVHs);
+ auto CallCounts = ScanSCC(*C, CallHandles);
for (int Iteration = 0;; ++Iteration) {
+
if (!PI.runBeforePass<LazyCallGraph::SCC>(Pass, *C))
continue;
@@ -669,22 +659,33 @@ class DevirtSCCRepeatedPass
assert(C->begin() != C->end() && "Cannot have an empty SCC!");
// Check whether any of the handles were devirtualized.
- bool Devirt = llvm::any_of(UR.IndirectVHs, [](auto &P) -> bool {
- if (P.second) {
- CallBase *CB = cast<CallBase>(P.second);
- if (CB->getCalledFunction()) {
- LLVM_DEBUG(dbgs() << "Found devirtualized call: " << *CB << "\n");
- return true;
- }
- }
- return false;
- });
+ auto IsDevirtualizedHandle = [&](WeakTrackingVH &CallH) {
+ if (!CallH)
+ return false;
+ auto *CB = dyn_cast<CallBase>(CallH);
+ if (!CB)
+ return false;
+
+ // If the call is still indirect, leave it alone.
+ Function *F = CB->getCalledFunction();
+ if (!F)
+ return false;
+
+ LLVM_DEBUG(dbgs() << "Found devirtualized call from "
+ << CB->getParent()->getParent()->getName() << " to "
+ << F->getName() << "\n");
+
+ // We now have a direct call where previously we had an indirect call,
+ // so iterate to process this devirtualization site.
+ return true;
+ };
+ bool Devirt = llvm::any_of(CallHandles, IsDevirtualizedHandle);
// Rescan to build up a new set of handles and count how many direct
// calls remain. If we decide to iterate, this also sets up the input to
// the next iteration.
- UR.IndirectVHs.clear();
- auto NewCallCounts = ScanSCC(*C, UR.IndirectVHs);
+ CallHandles.clear();
+ auto NewCallCounts = ScanSCC(*C, CallHandles);
// If we haven't found an explicit devirtualization already see if we
// have decreased the number of indirect calls and increased the number
@@ -789,8 +790,7 @@ ModuleToPostOrderCGSCCPassAdaptor<CGSCCPassT>::run(Module &M,
CGSCCUpdateResult UR = {
RCWorklist, CWorklist, InvalidRefSCCSet, InvalidSCCSet,
- nullptr, nullptr, PreservedAnalyses::all(), InlinedInternalEdges,
- {}};
+ nullptr, nullptr, PreservedAnalyses::all(), InlinedInternalEdges};
// Request PassInstrumentation from analysis manager, will use it to run
// instrumenting callbacks for the passes later.
diff --git a/llvm/lib/Analysis/CGSCCPassManager.cpp b/llvm/lib/Analysis/CGSCCPassManager.cpp
index 95d2ebf5f9fb..627ad03cd045 100644
--- a/llvm/lib/Analysis/CGSCCPassManager.cpp
+++ b/llvm/lib/Analysis/CGSCCPassManager.cpp
@@ -20,7 +20,6 @@
#include "llvm/IR/Instruction.h"
#include "llvm/IR/PassManager.h"
#include "llvm/IR/PassManagerImpl.h"
-#include "llvm/IR/ValueHandle.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Debug.h"
@@ -477,9 +476,9 @@ static LazyCallGraph::SCC &updateCGAndAnalysisManagerForPass(
// First walk the function and handle all called functions. We do this first
// because if there is a single call edge, whether there are ref edges is
// irrelevant.
- for (Instruction &I : instructions(F)) {
- if (auto *CB = dyn_cast<CallBase>(&I)) {
- if (Function *Callee = CB->getCalledFunction()) {
+ for (Instruction &I : instructions(F))
+ if (auto *CB = dyn_cast<CallBase>(&I))
+ if (Function *Callee = CB->getCalledFunction())
if (Visited.insert(Callee).second && !Callee->isDeclaration()) {
Node *CalleeN = G.lookup(*Callee);
if (!CalleeN) {
@@ -499,17 +498,6 @@ static LazyCallGraph::SCC &updateCGAndAnalysisManagerForPass(
else if (!E->isCall())
PromotedRefTargets.insert(CalleeN);
}
- } else {
- // We can miss devirtualization if an indirect call is created then
- // promoted before updateCGAndAnalysisManagerForPass runs.
- auto *Entry = UR.IndirectVHs.find(CB);
- if (Entry == UR.IndirectVHs.end())
- UR.IndirectVHs.insert({CB, WeakTrackingVH(CB)});
- else if (!Entry->second)
- Entry->second = WeakTrackingVH(CB);
- }
- }
- }
// Now walk all references.
for (Instruction &I : instructions(F))
diff --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp
index e5189bdb4fd9..5ad7527fc2ee 100644
--- a/llvm/lib/Passes/PassBuilder.cpp
+++ b/llvm/lib/Passes/PassBuilder.cpp
@@ -1815,7 +1815,7 @@ static Optional<int> parseDevirtPassName(StringRef Name) {
if (!Name.consume_front("devirt<") || !Name.consume_back(">"))
return None;
int Count;
- if (Name.getAsInteger(0, Count) || Count < 0)
+ if (Name.getAsInteger(0, Count) || Count <= 0)
return None;
return Count;
}
diff --git a/llvm/test/Transforms/Inline/devirtualize-3.ll b/llvm/test/Transforms/Inline/devirtualize-3.ll
index 987463a03766..4165b2125dfc 100644
--- a/llvm/test/Transforms/Inline/devirtualize-3.ll
+++ b/llvm/test/Transforms/Inline/devirtualize-3.ll
@@ -1,5 +1,4 @@
; RUN: opt -basic-aa -S -O2 < %s | FileCheck %s
-; RUN: opt -aa-pipeline=basic-aa -S -passes='default<O2>' < %s | FileCheck %s
; PR5009
; CHECK: define i32 @main()
diff --git a/llvm/test/Transforms/Inline/devirtualize-5.ll b/llvm/test/Transforms/Inline/devirtualize-5.ll
deleted file mode 100644
index ab507d1d58a1..000000000000
--- a/llvm/test/Transforms/Inline/devirtualize-5.ll
+++ /dev/null
@@ -1,22 +0,0 @@
-; RUN: opt -abort-on-max-devirt-iterations-reached -passes='cgscc(devirt<1>(inline,instcombine))' -S < %s | FileCheck %s
-; RUN: opt -abort-on-max-devirt-iterations-reached -passes='default<O2>' -S < %s | FileCheck %s
-
-define i32 @i() alwaysinline {
- ret i32 45
-}
-
-; CHECK-LABEL: define i32 @main
-; CHECK-NEXT: ret i32 45
-
-define i32 @main() {
- %a = alloca i32 ()*
- store i32 ()* @i, i32 ()** %a
- %r = call i32 @call(i32 ()** %a)
- ret i32 %r
-}
-
-define i32 @call(i32 ()** %a) alwaysinline {
- %c = load i32 ()*, i32 ()** %a
- %r = call i32 %c()
- ret i32 %r
-}
diff --git a/llvm/test/Transforms/Inline/devirtualize-6.ll b/llvm/test/Transforms/Inline/devirtualize-6.ll
deleted file mode 100644
index fb0d021548b2..000000000000
--- a/llvm/test/Transforms/Inline/devirtualize-6.ll
+++ /dev/null
@@ -1,18 +0,0 @@
-; Make sure we don't detect devirtualization on inlining a function with a direct call
-; RUN: opt -abort-on-max-devirt-iterations-reached -passes='cgscc(devirt<0>(inline))' -S < %s | FileCheck %s
-
-define i32 @i() noinline {
- ret i32 45
-}
-
-; CHECK-NOT: call i32 @call()
-
-define i32 @main() {
- %r = call i32 @call()
- ret i32 %r
-}
-
-define i32 @call() alwaysinline {
- %r = call i32 @i()
- ret i32 %r
-}
diff --git a/llvm/test/Transforms/Inline/devirtualize.ll b/llvm/test/Transforms/Inline/devirtualize.ll
index eaba1a8c19c5..561bb62ae644 100644
--- a/llvm/test/Transforms/Inline/devirtualize.ll
+++ b/llvm/test/Transforms/Inline/devirtualize.ll
@@ -1,5 +1,4 @@
; RUN: opt -S -Os < %s | FileCheck %s
-; RUN: opt -S -aa-pipeline=basic-aa -passes='default<Os>' < %s | FileCheck %s
target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"
target triple = "x86_64-apple-darwin10.0.0"
More information about the llvm-branch-commits
mailing list