Skip to content

[Convergence] allow non-convergent ops before entry and loop intrinsics #65939

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 1 commit into from
Sep 11, 2023
Merged

[Convergence] allow non-convergent ops before entry and loop intrinsics #65939

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 5 additions & 3 deletions llvm/docs/ConvergentOperations.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -607,8 +607,9 @@ those in the caller.
only if both threads entered the function by executing converged
dynamic instances of the call-site.

This intrinsic can occur at most once in a function, and only at the start of
the entry block of the function.
This intrinsic can occur at most once in a function, and only in the the entry
block of the function. If this intrinsic occurs in a basic block, then it must
precede any other convergent operation in the same basic block.

It is an error if this intrinsic appears in a non-convergent function.

Expand Down Expand Up @@ -669,7 +670,8 @@ threads execute converged dynamic instances of ``U`` if and only if:
It is an error to omit the ``convergencectrl`` operand bundle on a
call to this intrinsic.

This intrinsic can only occur at the start of a basic block.
If this intrinsic occurs in a basic block, then it must precede any other
convergent operation in the same basic block.

.. _convergence_cycle_heart:

Expand Down
2 changes: 2 additions & 0 deletions llvm/include/llvm/ADT/GenericConvergenceVerifier.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ template <typename ContextT> class GenericConvergenceVerifier {
// and not the token values.
DenseMap<const InstructionT *, const InstructionT *> Tokens;

bool SeenFirstConvOp = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

The value will not be reset across basic block, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It gets reset at the start of every block in the visit method.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think not? The visit() method you pointed out is only called in Verifier::visitCallBase(). So it cannot reset the flag in a new block.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ouch! You are right. I was paying too much attention on the MachineConvergenceVerifier (not checked in yet), and didn't see how the ConvergenceVerifier traverses the function. I have a fix in my branch, will push it out real soo now. Thanks for catching it!


static bool isInsideConvergentFunction(const InstructionT &I);
static bool isConvergent(const InstructionT &I);
const InstructionT *findAndCheckConvergenceTokenUsed(const InstructionT &I);
Expand Down
19 changes: 14 additions & 5 deletions llvm/include/llvm/IR/GenericConvergenceVerifierImpl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,13 @@ template <class ContextT>
void GenericConvergenceVerifier<ContextT>::visit(const InstructionT &I) {
auto ID = ContextT::getIntrinsicID(I);
auto *TokenDef = findAndCheckConvergenceTokenUsed(I);

bool IsCtrlIntrinsic = true;

// If this is the first instruction in the block, then we have not seen a
// convergent op yet.
if (!I.getPrevNode())
SeenFirstConvOp = false;

switch (ID) {
case Intrinsic::experimental_convergence_entry:
Check(isInsideConvergentFunction(I),
Expand All @@ -82,8 +86,9 @@ void GenericConvergenceVerifier<ContextT>::visit(const InstructionT &I) {
Check(I.getParent()->isEntryBlock(),
"Entry intrinsic can occur only in the entry block.",
{Context.print(&I)});
Check(I.getParent()->getFirstNonPHI() == &I,
"Entry intrinsic can occur only at the start of the basic block.",
Check(!SeenFirstConvOp,
"Entry intrinsic cannot be preceded by a convergent operation in the "
"same basic block.",
{Context.print(&I)});
LLVM_FALLTHROUGH;
case Intrinsic::experimental_convergence_anchor:
Expand All @@ -95,15 +100,19 @@ void GenericConvergenceVerifier<ContextT>::visit(const InstructionT &I) {
case Intrinsic::experimental_convergence_loop:
Check(TokenDef, "Loop intrinsic must have a convergencectrl token operand.",
{Context.print(&I)});
Check(I.getParent()->getFirstNonPHI() == &I,
"Loop intrinsic can occur only at the start of the basic block.",
Check(!SeenFirstConvOp,
"Loop intrinsic cannot be preceded by a convergent operation in the "
"same basic block.",
{Context.print(&I)});
break;
default:
IsCtrlIntrinsic = false;
break;
}

if (isConvergent(I))
SeenFirstConvOp = true;

if (TokenDef || IsCtrlIntrinsic) {
Check(isConvergent(I),
"Convergence control token can only be used in a convergent call.",
Expand Down
6 changes: 4 additions & 2 deletions llvm/test/Verifier/convergencectrl-invalid.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,11 @@ B:
br label %B
}

; CHECK: Entry intrinsic can occur only at the start of the basic block.
; CHECK: Entry intrinsic cannot be preceded by a convergent operation in the same basic block.
; CHECK: %t60_tok1
define void @entry_at_start(i32 %x, i32 %y) convergent {
%z = add i32 %x, %y
call void @f()
%t60_tok1 = call token @llvm.experimental.convergence.entry()
ret void
}
Expand All @@ -124,14 +125,15 @@ define void @entry_in_convergent(i32 %x, i32 %y) {
ret void
}

; CHECK: Loop intrinsic can occur only at the start of the basic block.
; CHECK: Loop intrinsic cannot be preceded by a convergent operation in the same basic block.
; CHECK: %t60_tok3
define void @loop_at_start(i32 %x, i32 %y) convergent {
A:
%t60_tok3 = call token @llvm.experimental.convergence.entry()
br label %B
B:
%z = add i32 %x, %y
call void @f()
%h1 = call token @llvm.experimental.convergence.loop() [ "convergencectrl"(token %t60_tok3) ]
ret void
}
Expand Down