Skip to content

Commit

Permalink
[LLVM] convergence verifier should visit all instructions (#66200)
Browse files Browse the repository at this point in the history
The entry and loop intrinsics for convergence control cannot be preceded
by convergent operations in their respective basic blocks. To check
that, the verifier needs to reset its state at the start of the block.
This was missed in the previous commit
fa6dd7a.
  • Loading branch information
ssahasra authored Sep 20, 2023
1 parent e9dfe08 commit ee49453
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 11 deletions.
1 change: 1 addition & 0 deletions llvm/include/llvm/ADT/GenericConvergenceVerifier.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ template <typename ContextT> class GenericConvergenceVerifier {
}

void clear();
void visit(const BlockT &BB);
void visit(const InstructionT &I);
void verify(const DominatorTreeT &DT);

Expand Down
10 changes: 5 additions & 5 deletions llvm/include/llvm/IR/GenericConvergenceVerifierImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,17 +67,17 @@ template <class ContextT> void GenericConvergenceVerifier<ContextT>::clear() {
ConvergenceKind = NoConvergence;
}

template <class ContextT>
void GenericConvergenceVerifier<ContextT>::visit(const BlockT &BB) {
SeenFirstConvOp = false;
}

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 Down
11 changes: 6 additions & 5 deletions llvm/lib/IR/Verifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ class Verifier : public InstVisitor<Verifier>, VerifierSupport {
SmallVector<const DILocalVariable *, 16> DebugFnArgs;

TBAAVerifier TBAAVerifyHelper;
ConvergenceVerifier CV;
ConvergenceVerifier ConvergenceVerifyHelper;

SmallVector<IntrinsicInst *, 4> NoAliasScopeDecls;

Expand Down Expand Up @@ -408,15 +408,15 @@ class Verifier : public InstVisitor<Verifier>, VerifierSupport {
auto FailureCB = [this](const Twine &Message) {
this->CheckFailed(Message);
};
CV.initialize(OS, FailureCB, F);
ConvergenceVerifyHelper.initialize(OS, FailureCB, F);

Broken = false;
// FIXME: We strip const here because the inst visitor strips const.
visit(const_cast<Function &>(F));
verifySiblingFuncletUnwinds();

if (CV.sawTokens())
CV.verify(DT);
if (ConvergenceVerifyHelper.sawTokens())
ConvergenceVerifyHelper.verify(DT);

InstsInThisBlock.clear();
DebugFnArgs.clear();
Expand Down Expand Up @@ -2867,6 +2867,7 @@ void Verifier::visitFunction(const Function &F) {
//
void Verifier::visitBasicBlock(BasicBlock &BB) {
InstsInThisBlock.clear();
ConvergenceVerifyHelper.visit(BB);

// Ensure that basic blocks have terminators!
Check(BB.getTerminator(), "Basic Block does not have terminator!", &BB);
Expand Down Expand Up @@ -3568,7 +3569,7 @@ void Verifier::visitCallBase(CallBase &Call) {
if (Call.isInlineAsm())
verifyInlineAsmCall(Call);

CV.visit(Call);
ConvergenceVerifyHelper.visit(Call);

visitInstruction(Call);
}
Expand Down
7 changes: 6 additions & 1 deletion llvm/test/Verifier/convergencectrl-invalid.ll
Original file line number Diff line number Diff line change
Expand Up @@ -126,13 +126,18 @@ define void @entry_in_convergent(i32 %x, i32 %y) {
}

; CHECK: Loop intrinsic cannot be preceded by a convergent operation in the same basic block.
; CHECK: %t60_tok3
; CHECK-NEXT: %h1
; CHECK-SAME: %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
; This is not an error
%h2 = call token @llvm.experimental.convergence.loop() [ "convergencectrl"(token %t60_tok3) ]
br label %C
C:
call void @f()
%h1 = call token @llvm.experimental.convergence.loop() [ "convergencectrl"(token %t60_tok3) ]
ret void
Expand Down

0 comments on commit ee49453

Please sign in to comment.