[llvm] 932e4f8 - [FunctionAttrs][NPM] Fix handling of convergent
Arthur Eubanks via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 23 21:10:02 PST 2020
Author: Arthur Eubanks
Date: 2020-11-23T21:09:41-08:00
New Revision: 932e4f88157194986fa9624edfea9abb9fbde77f
URL: https://github.com/llvm/llvm-project/commit/932e4f88157194986fa9624edfea9abb9fbde77f
DIFF: https://github.com/llvm/llvm-project/commit/932e4f88157194986fa9624edfea9abb9fbde77f.diff
LOG: [FunctionAttrs][NPM] Fix handling of convergent
The legacy pass didn't properly detect indirect calls.
We can still remove the convergent attribute when there are indirect
calls. The LangRef says:
> When it appears on a call/invoke, the convergent attribute indicates
that we should treat the call as though we’re calling a convergent
function. This is particularly useful on indirect calls; without this we
may treat such calls as though the target is non-convergent.
So don't skip handling of convergent when there are unknown calls.
Reviewed By: jdoerfert
Differential Revision: https://reviews.llvm.org/D89826
Added:
Modified:
llvm/lib/Transforms/IPO/FunctionAttrs.cpp
llvm/test/Transforms/FunctionAttrs/convergent.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
index 2e5d9095989e..52f196c67bf8 100644
--- a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
@@ -13,6 +13,7 @@
//===----------------------------------------------------------------------===//
#include "llvm/Transforms/IPO/FunctionAttrs.h"
+#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/SCCIterator.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SetVector.h"
@@ -1218,6 +1219,11 @@ bool AttributeInferer::run(const SCCNodeSet &SCCNodes) {
return Changed;
}
+struct SCCNodesResult {
+ SCCNodeSet SCCNodes;
+ bool HasUnknownCall;
+};
+
} // end anonymous namespace
/// Helper for non-Convergent inference predicate InstrBreaksAttribute.
@@ -1265,15 +1271,10 @@ static bool InstrBreaksNoFree(Instruction &I, const SCCNodeSet &SCCNodes) {
return true;
}
-/// Infer attributes from all functions in the SCC by scanning every
-/// instruction for compliance to the attribute assumptions. Currently it
-/// does:
-/// - removal of Convergent attribute
-/// - addition of NoUnwind attribute
+/// Attempt to remove convergent function attribute when possible.
///
/// Returns true if any changes to function attributes were made.
-static bool inferAttrsFromFunctionBodies(const SCCNodeSet &SCCNodes) {
-
+static bool inferConvergent(const SCCNodeSet &SCCNodes) {
AttributeInferer AI;
// Request to remove the convergent attribute from all functions in the SCC
@@ -1295,6 +1296,18 @@ static bool inferAttrsFromFunctionBodies(const SCCNodeSet &SCCNodes) {
F.setNotConvergent();
},
/* RequiresExactDefinition= */ false});
+ // Perform all the requested attribute inference actions.
+ return AI.run(SCCNodes);
+}
+
+/// Infer attributes from all functions in the SCC by scanning every
+/// instruction for compliance to the attribute assumptions. Currently it
+/// does:
+/// - addition of NoUnwind attribute
+///
+/// Returns true if any changes to function attributes were made.
+static bool inferAttrsFromFunctionBodies(const SCCNodeSet &SCCNodes) {
+ AttributeInferer AI;
if (!DisableNoUnwindInference)
// Request to infer nounwind attribute for all the functions in the SCC if
@@ -1376,27 +1389,57 @@ static bool addNoRecurseAttrs(const SCCNodeSet &SCCNodes) {
return true;
}
+static SCCNodesResult createSCCNodeSet(ArrayRef<Function *> Functions) {
+ SCCNodesResult Res;
+ Res.HasUnknownCall = false;
+ for (Function *F : Functions) {
+ if (!F || F->hasOptNone() || F->hasFnAttribute(Attribute::Naked)) {
+ // Treat any function we're trying not to optimize as if it were an
+ // indirect call and omit it from the node set used below.
+ Res.HasUnknownCall = true;
+ continue;
+ }
+ // Track whether any functions in this SCC have an unknown call edge.
+ // Note: if this is ever a performance hit, we can common it with
+ // subsequent routines which also do scans over the instructions of the
+ // function.
+ if (!Res.HasUnknownCall) {
+ for (Instruction &I : instructions(*F)) {
+ if (auto *CB = dyn_cast<CallBase>(&I)) {
+ if (!CB->getCalledFunction()) {
+ Res.HasUnknownCall = true;
+ break;
+ }
+ }
+ }
+ }
+ Res.SCCNodes.insert(F);
+ }
+ return Res;
+}
+
template <typename AARGetterT>
-static bool deriveAttrsInPostOrder(SCCNodeSet &SCCNodes,
- AARGetterT &&AARGetter,
- bool HasUnknownCall) {
+static bool deriveAttrsInPostOrder(ArrayRef<Function *> Functions,
+ AARGetterT &&AARGetter) {
+ SCCNodesResult Nodes = createSCCNodeSet(Functions);
bool Changed = false;
// Bail if the SCC only contains optnone functions.
- if (SCCNodes.empty())
+ if (Nodes.SCCNodes.empty())
return Changed;
- Changed |= addArgumentReturnedAttrs(SCCNodes);
- Changed |= addReadAttrs(SCCNodes, AARGetter);
- Changed |= addArgumentAttrs(SCCNodes);
+ Changed |= addArgumentReturnedAttrs(Nodes.SCCNodes);
+ Changed |= addReadAttrs(Nodes.SCCNodes, AARGetter);
+ Changed |= addArgumentAttrs(Nodes.SCCNodes);
+ Changed |= inferConvergent(Nodes.SCCNodes);
// If we have no external nodes participating in the SCC, we can deduce some
// more precise attributes as well.
- if (!HasUnknownCall) {
- Changed |= addNoAliasAttrs(SCCNodes);
- Changed |= addNonNullAttrs(SCCNodes);
- Changed |= inferAttrsFromFunctionBodies(SCCNodes);
- Changed |= addNoRecurseAttrs(SCCNodes);
+ if (!Nodes.HasUnknownCall) {
+ Changed |= addNoAliasAttrs(Nodes.SCCNodes);
+ Changed |= addNonNullAttrs(Nodes.SCCNodes);
+ Changed |= inferAttrsFromFunctionBodies(Nodes.SCCNodes);
+ Changed |= addNoRecurseAttrs(Nodes.SCCNodes);
}
return Changed;
@@ -1415,35 +1458,12 @@ PreservedAnalyses PostOrderFunctionAttrsPass::run(LazyCallGraph::SCC &C,
return FAM.getResult<AAManager>(F);
};
- // Fill SCCNodes with the elements of the SCC. Also track whether there are
- // any external or opt-none nodes that will prevent us from optimizing any
- // part of the SCC.
- SCCNodeSet SCCNodes;
- bool HasUnknownCall = false;
+ SmallVector<Function *, 8> Functions;
for (LazyCallGraph::Node &N : C) {
- Function &F = N.getFunction();
- if (F.hasOptNone() || F.hasFnAttribute(Attribute::Naked)) {
- // Treat any function we're trying not to optimize as if it were an
- // indirect call and omit it from the node set used below.
- HasUnknownCall = true;
- continue;
- }
- // Track whether any functions in this SCC have an unknown call edge.
- // Note: if this is ever a performance hit, we can common it with
- // subsequent routines which also do scans over the instructions of the
- // function.
- if (!HasUnknownCall)
- for (Instruction &I : instructions(F))
- if (auto *CB = dyn_cast<CallBase>(&I))
- if (!CB->getCalledFunction()) {
- HasUnknownCall = true;
- break;
- }
-
- SCCNodes.insert(&F);
+ Functions.push_back(&N.getFunction());
}
- if (deriveAttrsInPostOrder(SCCNodes, AARGetter, HasUnknownCall))
+ if (deriveAttrsInPostOrder(Functions, AARGetter))
return PreservedAnalyses::none();
return PreservedAnalyses::all();
@@ -1486,26 +1506,12 @@ Pass *llvm::createPostOrderFunctionAttrsLegacyPass() {
template <typename AARGetterT>
static bool runImpl(CallGraphSCC &SCC, AARGetterT AARGetter) {
-
- // Fill SCCNodes with the elements of the SCC. Used for quickly looking up
- // whether a given CallGraphNode is in this SCC. Also track whether there are
- // any external or opt-none nodes that will prevent us from optimizing any
- // part of the SCC.
- SCCNodeSet SCCNodes;
- bool ExternalNode = false;
+ SmallVector<Function *, 8> Functions;
for (CallGraphNode *I : SCC) {
- Function *F = I->getFunction();
- if (!F || F->hasOptNone() || F->hasFnAttribute(Attribute::Naked)) {
- // External node or function we're trying not to optimize - we both avoid
- // transform them and avoid leveraging information they provide.
- ExternalNode = true;
- continue;
- }
-
- SCCNodes.insert(F);
+ Functions.push_back(I->getFunction());
}
- return deriveAttrsInPostOrder(SCCNodes, AARGetter, ExternalNode);
+ return deriveAttrsInPostOrder(Functions, AARGetter);
}
bool PostOrderFunctionAttrsLegacyPass::runOnSCC(CallGraphSCC &SCC) {
diff --git a/llvm/test/Transforms/FunctionAttrs/convergent.ll b/llvm/test/Transforms/FunctionAttrs/convergent.ll
index 8b764f502307..277b1de3a407 100644
--- a/llvm/test/Transforms/FunctionAttrs/convergent.ll
+++ b/llvm/test/Transforms/FunctionAttrs/convergent.ll
@@ -1,7 +1,4 @@
-; FIXME: convert CHECK-INDIRECT into CHECK (and remove -check-prefixes) as soon
-; FIXME: as new-pass-manager's handling of indirect_non_convergent_call is fixed
-;
-; RUN: opt -function-attrs -S < %s | FileCheck %s --check-prefixes=CHECK,CHECK-INDIRECT
+; RUN: opt -function-attrs -S < %s | FileCheck %s
; RUN: opt -passes=function-attrs -S < %s | FileCheck %s
; CHECK: Function Attrs
@@ -54,8 +51,8 @@ define i32 @indirect_convergent_call(i32 ()* %f) convergent {
; "Function Attrs" comment in the output.
;
; CHECK: Function Attrs
-; CHECK-INDIRECT-NOT: convergent
-; CHECK-INDIRECT-NEXT: define i32 @indirect_non_convergent_call(
+; CHECK-NOT: convergent
+; CHECK-NEXT: define i32 @indirect_non_convergent_call(
define i32 @indirect_non_convergent_call(i32 ()* %f) convergent norecurse {
%a = call i32 %f()
ret i32 %a
More information about the llvm-commits
mailing list