Skip to content

[IPO] Prevent removal of some convergent attr #134863

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

Closed
wants to merge 4 commits into from
Closed

[IPO] Prevent removal of some convergent attr #134863

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b7021ef
[IPO] Prevent removal of some convergent attr
Keenuts Apr 7, 2025
73601b7
replace vreg with named values
Keenuts Apr 8, 2025
05f601d
generate full checks instead of check-not
Keenuts Apr 8, 2025
486720a
fix attributor
Keenuts Apr 9, 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
21 changes: 21 additions & 0 deletions llvm/lib/Transforms/IPO/AttributorAttributes.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2902,6 +2902,22 @@ struct AANonConvergentImpl : public AANonConvergent {
}
};

static bool FunctionCalledWithConvergenceToken(const Function *F) {
Copy link
Contributor

@arsenm arsenm Apr 11, 2025

Choose a reason for hiding this comment

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

Start with lowercase

for (auto &Use : F->uses()) {
CallBase *CB = dyn_cast<CallBase>(Use.getUser());
if (!CB)
continue;

// We are not called, just used as an argument.
if (CB->getCalledFunction() != F)
continue;
Comment on lines +2908 to +2913
Copy link
Contributor

@shiltian shiltian Apr 16, 2025

Choose a reason for hiding this comment

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

Suggested change
if (!CB)
continue;
// We are not called, just used as an argument.
if (CB->getCalledFunction() != F)
continue;
if (!CB || CB->getCalledFunction() != F)
continue;


if (CB->getConvergenceControlToken())
return true;
}
return false;
Copy link
Contributor

@arsenm arsenm Apr 11, 2025

Choose a reason for hiding this comment

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

This needs to be a conservatively correct true. If the address is captured you do not know the users. If the function is externally visible, you also do not know the users.

}

struct AANonConvergentFunction final : AANonConvergentImpl {
AANonConvergentFunction(const IRPosition &IRP, Attributor &A)
: AANonConvergentImpl(IRP, A) {}
Expand Down Expand Up @@ -2929,6 +2945,11 @@ struct AANonConvergentFunction final : AANonConvergentImpl {
UsedAssumedInformation)) {
return indicatePessimisticFixpoint();
}

const Function *F = this->getIRPosition().getAssociatedFunction();
if (FunctionCalledWithConvergenceToken(F))
return indicatePessimisticFixpoint();

return ChangeStatus::UNCHANGED;
}

Expand Down
10 changes: 7 additions & 3 deletions llvm/lib/Transforms/IPO/FunctionAttrs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1864,12 +1864,16 @@ static bool InstrBreaksNonConvergent(Instruction &I,
!SCCNodes.contains(CB->getCalledFunction());
}

static bool FunctionRequiresConvergence(const Function &F) {
for (auto &Use : F.uses()) {
static bool FunctionRequiresConvergence(const Function *F) {
Copy link
Collaborator

@ssahasra ssahasra Apr 11, 2025

Choose a reason for hiding this comment

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

This is identical to FunctionCalledWithConvergenceToken. Can we move the definition to a common place, say Function::hasConvergentCalls()?

for (auto &Use : F->uses()) {
CallBase *CB = dyn_cast<CallBase>(Use.getUser());
if (!CB)
return true;

// We are not called, just used as an argument.
if (CB->getCalledFunction() != F)
continue;

if (CB->getConvergenceControlToken())
return true;
}
Expand Down Expand Up @@ -1981,7 +1985,7 @@ static void inferConvergent(const SCCNodeSet &SCCNodes,
Attribute::Convergent,
// Skip non-convergent functions.
[](const Function &F) {
return !F.isConvergent() || FunctionRequiresConvergence(F);
return !F.isConvergent() || FunctionRequiresConvergence(&F);
},
// Instructions that break non-convergent assumption.
[SCCNodes](Instruction &I) {
Expand Down
68 changes: 57 additions & 11 deletions llvm/test/Transforms/ADCE/convergence.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,34 +1,49 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
; RUN: opt %s -passes=adce -S | FileCheck %s

; CHECK: Function Attrs: convergent
; CHECK-NEXT: define i32 @foo(i32 %a) #0 {
define i32 @foo(i32 %a) #0 {
; CHECK-LABEL: define i32 @foo(
; CHECK-SAME: i32 [[A:%.*]]) #[[ATTR0:[0-9]+]] {
; CHECK-NEXT: [[ENTRY:.*:]]
; CHECK-NEXT: ret i32 [[A]]
;
entry:
; CHECK-NOT: %0 = call token @llvm.experimental.convergence.entry()
%0 = call token @llvm.experimental.convergence.entry()
%tk = call token @llvm.experimental.convergence.entry()
ret i32 %a
}

; CHECK: Function Attrs: convergent
; CHECK-NEXT: define void @bar() #0 {
define void @bar() #0 {
; CHECK-LABEL: define void @bar(
; CHECK-SAME: ) #[[ATTR0]] {
; CHECK-NEXT: [[ENTRY:.*:]]
; CHECK-NEXT: ret void
;
entry:
; CHECK-NOT: %0 = call token @llvm.experimental.convergence.entry()
%0 = call token @llvm.experimental.convergence.anchor()
%tk = call token @llvm.experimental.convergence.anchor()
ret void
}

; CHECK: Function Attrs: convergent
; CHECK-NEXT: define void @baz() #0 {
define void @baz() #0 {
; CHECK-LABEL: define void @baz(
; CHECK-SAME: ) #[[ATTR0]] {
; CHECK-NEXT: [[ENTRY:.*:]]
; CHECK-NEXT: br label %[[HEADER:.*]]
; CHECK: [[HEADER]]:
; CHECK-NEXT: br i1 true, label %[[BODY:.*]], label %[[EXIT:.*]]
; CHECK: [[BODY]]:
; CHECK-NEXT: br label %[[HEADER]]
; CHECK: [[EXIT]]:
; CHECK-NEXT: ret void
;
entry:
; CHECK-NOT: %0 = call token @llvm.experimental.convergence.entry()
%0 = call token @llvm.experimental.convergence.entry()
%tk0 = call token @llvm.experimental.convergence.entry()
br label %header

header:
; CHECK-NOT: %1 = call token @llvm.experimental.convergence.loop() [ "convergencectrl"(token %0) ]
%1 = call token @llvm.experimental.convergence.loop() [ "convergencectrl"(token %0) ]
%tk1 = call token @llvm.experimental.convergence.loop() [ "convergencectrl"(token %tk0) ]
br i1 true, label %body, label %exit

body:
Expand All @@ -38,6 +53,37 @@ exit:
ret void
}

define void @indirect_inner() #0 {
; CHECK-LABEL: define void @indirect_inner(
; CHECK-SAME: ) #[[ATTR0]] {
; CHECK-NEXT: [[ENTRY:.*:]]
; CHECK-NEXT: ret void
;
entry:
%tk0 = call token @llvm.experimental.convergence.entry()
ret void
}

define void @indirect() #0 {
; CHECK-LABEL: define void @indirect(
; CHECK-SAME: ) #[[ATTR0]] {
; CHECK-NEXT: [[ENTRY:.*:]]
; CHECK-NEXT: [[TK0:%.*]] = call token @llvm.experimental.convergence.entry()
; CHECK-NEXT: [[VAR:%.*]] = alloca ptr, align 8
; CHECK-NEXT: store ptr @indirect_inner, ptr [[VAR]], align 8
; CHECK-NEXT: [[PTR:%.*]] = load ptr, ptr [[VAR]], align 8
; CHECK-NEXT: call void [[PTR]]() #[[ATTR0]] [ "convergencectrl"(token [[TK0]]) ]
; CHECK-NEXT: ret void
;
entry:
%tk0 = call token @llvm.experimental.convergence.entry()
%var = alloca ptr, align 8
store ptr @indirect_inner, ptr %var, align 8
%ptr = load ptr, ptr %var, align 8
call void %ptr() convergent [ "convergencectrl"(token %tk0) ]
ret void
}

declare token @llvm.experimental.convergence.entry() #1
declare token @llvm.experimental.convergence.anchor() #1
declare token @llvm.experimental.convergence.loop() #1
Expand Down
Loading