[llvm] [Convergence] allow non-convergent ops before entry and loop intrinsics (PR #65939)

[Sameer Sahasrabuddhe via llvm-commits]

https://github.com/ssahasra created https://github.com/llvm/llvm-project/pull/65939:

The only real requirement is that entry and loop intrinsics should not be preceded by convergent operations in the same basic block. They do not need to be the first in the block.

Relaxing the constraint on the entry and loop intrinsics avoids having to make changes in the construction of LLVM IR, such as getFirstInsertionPt(). It also avoids added complexity in the lowering to Machine IR, where COPY instructions may be added to the start of the basic block.

>From fa6dd7a24af2b02f236ec3b980d9407e86c2c4aa Mon Sep 17 00:00:00 2001
From: Sameer Sahasrabuddhe <sameer.sahasrabuddhe at amd.com>
Date: Mon, 11 Sep 2023 12:33:13 +0530
Subject: [PATCH] [Convergence] allow non-convergent ops before entry and loop
 intrinsics

The only real requirement is that entry and loop intrinsics should
not be preceded by convergent operations in the same basic block. They do not
need to be the first in the block.

Relaxing the constraint on the entry and loop intrinsics avoids having to make
changes in the construction of LLVM IR, such as getFirstInsertionPt(). It also
avoids added complexity in the lowering to Machine IR, where COPY instructions
may be added to the start of the basic block.
---
 llvm/docs/ConvergentOperations.rst            |  8 +++++---
 .../llvm/ADT/GenericConvergenceVerifier.h     |  2 ++
 .../llvm/IR/GenericConvergenceVerifierImpl.h  | 19 ++++++++++++++-----
 llvm/test/Verifier/convergencectrl-invalid.ll |  6 ++++--
 4 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/llvm/docs/ConvergentOperations.rst b/llvm/docs/ConvergentOperations.rst
index ae9e017b2d89c33..5dd3ac2f3d98b97 100644
--- a/llvm/docs/ConvergentOperations.rst
+++ b/llvm/docs/ConvergentOperations.rst
@@ -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.
 
@@ -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:
 
diff --git a/llvm/include/llvm/ADT/GenericConvergenceVerifier.h b/llvm/include/llvm/ADT/GenericConvergenceVerifier.h
index e35fac7f2e99c23..71b7b41ef96661c 100644
--- a/llvm/include/llvm/ADT/GenericConvergenceVerifier.h
+++ b/llvm/include/llvm/ADT/GenericConvergenceVerifier.h
@@ -63,6 +63,8 @@ template <typename ContextT> class GenericConvergenceVerifier {
   // and not the token values.
   DenseMap<const InstructionT *, const InstructionT *> Tokens;
 
+  bool SeenFirstConvOp = false;
+
   static bool isInsideConvergentFunction(const InstructionT &I);
   static bool isConvergent(const InstructionT &I);
   const InstructionT *findAndCheckConvergenceTokenUsed(const InstructionT &I);
diff --git a/llvm/include/llvm/IR/GenericConvergenceVerifierImpl.h b/llvm/include/llvm/IR/GenericConvergenceVerifierImpl.h
index 165896ad9aa6462..c57f828cb1de746 100644
--- a/llvm/include/llvm/IR/GenericConvergenceVerifierImpl.h
+++ b/llvm/include/llvm/IR/GenericConvergenceVerifierImpl.h
@@ -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),
@@ -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:
@@ -95,8 +100,9 @@ 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:
@@ -104,6 +110,9 @@ void GenericConvergenceVerifier<ContextT>::visit(const InstructionT &I) {
     break;
   }
 
+  if (isConvergent(I))
+    SeenFirstConvOp = true;
+
   if (TokenDef || IsCtrlIntrinsic) {
     Check(isConvergent(I),
           "Convergence control token can only be used in a convergent call.",
diff --git a/llvm/test/Verifier/convergencectrl-invalid.ll b/llvm/test/Verifier/convergencectrl-invalid.ll
index 63591e0316208af..2f7b311973d7e1e 100644
--- a/llvm/test/Verifier/convergencectrl-invalid.ll
+++ b/llvm/test/Verifier/convergencectrl-invalid.ll
@@ -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
 }
@@ -124,7 +125,7 @@ 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:
@@ -132,6 +133,7 @@ A:
   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
 }