Skip to content

[FunctionAttrs] Don't bail out on unknown calls #150958

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 29, 2025
Merged

[FunctionAttrs] Don't bail out on unknown calls #150958

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ee41e32
[FunctionAttrs] Don't bail out on unknown calls
nikic Jul 28, 2025
62e8ffe
drop norecurse change for now
nikic Jul 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 11 additions & 31 deletions llvm/lib/Transforms/IPO/FunctionAttrs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1863,7 +1863,6 @@ void AttributeInferer::run(const SCCNodeSet &SCCNodes,

struct SCCNodesResult {
SCCNodeSet SCCNodes;
bool HasUnknownCall;
};

} // end anonymous namespace
Expand Down Expand Up @@ -2087,11 +2086,13 @@ static void addNoRecurseAttrs(const SCCNodeSet &SCCNodes,
for (auto &BB : *F)
for (auto &I : BB.instructionsWithoutDebug())
if (auto *CB = dyn_cast<CallBase>(&I)) {
if (CB->hasFnAttr(Attribute::NoRecurse))
continue;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this actually legal? norecurse on a call just means that the callee doesn't call itself; it doesn't necessarily mean it doesn't call the caller. For example:

void foo(), bar(int);
void foo() { bar(0); }
void bar(int b) { if (b) foo(); }

"foo" is norecurse, but bar isn't.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a general issue with norecurse inference, independently of whether the attribute is on the call-site or the declaration. See the discussion on #139943 (comment). I expect we'll change the definition of norecurse to justify the correctness of the existing inference (which would also make this change valid).

But I'm happy to drop this change until that has happened.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, didn't realize there was already an active discussion. If the current norecurse inference is self-consistent, that fine for now.


Function *Callee = CB->getCalledFunction();
if (!Callee || Callee == F ||
(!Callee->doesNotRecurse() &&
!(Callee->isDeclaration() &&
Callee->hasFnAttribute(Attribute::NoCallback))))
!(Callee->isDeclaration() &&
Callee->hasFnAttribute(Attribute::NoCallback)))
// Function calls a potentially recursive function.
return;
}
Expand Down Expand Up @@ -2227,29 +2228,13 @@ static void addWillReturn(const SCCNodeSet &SCCNodes,

static SCCNodesResult createSCCNodeSet(ArrayRef<Function *> Functions) {
SCCNodesResult Res;
Res.HasUnknownCall = false;
for (Function *F : Functions) {
if (!F || F->hasOptNone() || F->hasFnAttribute(Attribute::Naked) ||
F->isPresplitCoroutine()) {
// 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;
// Omit any functions we're trying not to optimize from the set.
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;
Expand Down Expand Up @@ -2282,15 +2267,10 @@ deriveAttrsInPostOrder(ArrayRef<Function *> Functions, AARGetterT &&AARGetter,
addColdAttrs(Nodes.SCCNodes, Changed);
addWillReturn(Nodes.SCCNodes, Changed);
addNoUndefAttrs(Nodes.SCCNodes, Changed);

// If we have no external nodes participating in the SCC, we can deduce some
// more precise attributes as well.
if (!Nodes.HasUnknownCall) {
addNoAliasAttrs(Nodes.SCCNodes, Changed);
addNonNullAttrs(Nodes.SCCNodes, Changed);
inferAttrsFromFunctionBodies(Nodes.SCCNodes, Changed);
addNoRecurseAttrs(Nodes.SCCNodes, Changed);
}
addNoAliasAttrs(Nodes.SCCNodes, Changed);
addNonNullAttrs(Nodes.SCCNodes, Changed);
inferAttrsFromFunctionBodies(Nodes.SCCNodes, Changed);
addNoRecurseAttrs(Nodes.SCCNodes, Changed);

// Finally, infer the maximal set of attributes from the ones we've inferred
// above. This is handling the cases where one attribute on a signature
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/Transforms/FunctionAttrs/noalias.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ define ptr @return_unknown_call(ptr %fn) {
}

define ptr @return_unknown_noalias_call(ptr %fn) {
; CHECK-LABEL: define ptr @return_unknown_noalias_call(
; CHECK-LABEL: define noalias ptr @return_unknown_noalias_call(
; CHECK-SAME: ptr readonly captures(none) [[FN:%.*]]) {
; CHECK-NEXT: [[A:%.*]] = call noalias ptr [[FN]]()
; CHECK-NEXT: ret ptr [[A]]
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/Transforms/FunctionAttrs/nonnull.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1412,7 +1412,7 @@ define ptr @unknown_func(ptr %fn) {
}

define ptr @unknown_nonnull_func(ptr %fn) {
; FNATTRS-LABEL: define ptr @unknown_nonnull_func(
; FNATTRS-LABEL: define nonnull ptr @unknown_nonnull_func(
; FNATTRS-SAME: ptr readonly captures(none) [[FN:%.*]]) {
; FNATTRS-NEXT: [[RES:%.*]] = call nonnull ptr [[FN]]()
; FNATTRS-NEXT: ret ptr [[RES]]
Expand Down
5 changes: 3 additions & 2 deletions llvm/test/Transforms/FunctionAttrs/norecurse.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,9 +258,10 @@ define void @unknown_call(ptr %fn) {
}

define void @unknown_norecurse_call(ptr %fn) {
; FNATTRS: Function Attrs: norecurse
; FNATTRS-LABEL: define {{[^@]+}}@unknown_norecurse_call
; FNATTRS-SAME: (ptr readonly captures(none) [[FN:%.*]]) {
; FNATTRS-NEXT: call void [[FN]]() #[[ATTR7:[0-9]+]]
; FNATTRS-SAME: (ptr readonly captures(none) [[FN:%.*]]) #[[ATTR7:[0-9]+]] {
; FNATTRS-NEXT: call void [[FN]]() #[[ATTR7]]
; FNATTRS-NEXT: ret void
;
; ATTRIBUTOR-LABEL: define {{[^@]+}}@unknown_norecurse_call
Expand Down
5 changes: 3 additions & 2 deletions llvm/test/Transforms/FunctionAttrs/nounwind.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,9 +418,10 @@ define void @unknown_call(ptr %fn) {
}

define void @unknown_nounwind_call(ptr %fn) {
; FNATTRS: Function Attrs: nounwind
; FNATTRS-LABEL: define {{[^@]+}}@unknown_nounwind_call
; FNATTRS-SAME: (ptr readonly captures(none) [[FN:%.*]]) {
; FNATTRS-NEXT: call void [[FN]]() #[[ATTR2:[0-9]+]]
; FNATTRS-SAME: (ptr readonly captures(none) [[FN:%.*]]) #[[ATTR2:[0-9]+]] {
; FNATTRS-NEXT: call void [[FN]]() #[[ATTR2]]
; FNATTRS-NEXT: ret void
;
; ATTRIBUTOR: Function Attrs: nounwind
Expand Down