[llvm] [LLVM] convergence verifier should visit all instructions (PR #66200)

[Sameer Sahasrabuddhe via llvm-commits]

https://github.com/ssahasra updated https://github.com/llvm/llvm-project/pull/66200

>From da7edf4c078879c7d3b071ba6e10435e2b6e34f5 Mon Sep 17 00:00:00 2001
From: Sameer Sahasrabuddhe <sameer.sahasrabuddhe at amd.com>
Date: Wed, 20 Sep 2023 14:59:06 +0530
Subject: [PATCH] [LLVM] detect start of each basic block in convergence
 verifier

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 fa6dd7a24af2b02f236ec3b980d9407e86c2c4aa.
---
 llvm/include/llvm/ADT/GenericConvergenceVerifier.h    |  1 +
 llvm/include/llvm/IR/GenericConvergenceVerifierImpl.h | 10 +++++-----
 llvm/lib/IR/Verifier.cpp                              | 11 ++++++-----
 llvm/test/Verifier/convergencectrl-invalid.ll         |  7 ++++++-
 4 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/llvm/include/llvm/ADT/GenericConvergenceVerifier.h b/llvm/include/llvm/ADT/GenericConvergenceVerifier.h
index 71b7b41ef96661c..0810a0701322907 100644
--- a/llvm/include/llvm/ADT/GenericConvergenceVerifier.h
+++ b/llvm/include/llvm/ADT/GenericConvergenceVerifier.h
@@ -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);
 
diff --git a/llvm/include/llvm/IR/GenericConvergenceVerifierImpl.h b/llvm/include/llvm/IR/GenericConvergenceVerifierImpl.h
index c57f828cb1de746..2ba81015cb7b641 100644
--- a/llvm/include/llvm/IR/GenericConvergenceVerifierImpl.h
+++ b/llvm/include/llvm/IR/GenericConvergenceVerifierImpl.h
@@ -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),
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 10d176380ebed94..5dac691e17cd6ef 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -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;
 
@@ -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();
@@ -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);
@@ -3568,7 +3569,7 @@ void Verifier::visitCallBase(CallBase &Call) {
   if (Call.isInlineAsm())
     verifyInlineAsmCall(Call);
 
-  CV.visit(Call);
+  ConvergenceVerifyHelper.visit(Call);
 
   visitInstruction(Call);
 }
diff --git a/llvm/test/Verifier/convergencectrl-invalid.ll b/llvm/test/Verifier/convergencectrl-invalid.ll
index 2f7b311973d7e1e..e1fffcd1c603347 100644
--- a/llvm/test/Verifier/convergencectrl-invalid.ll
+++ b/llvm/test/Verifier/convergencectrl-invalid.ll
@@ -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