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

[via llvm-commits]

Author: Sameer Sahasrabuddhe
Date: 2023-09-11T18:26:07+05:30
New Revision: 08da343750be3ff6940ebca35be6a3bfd38cc5de

URL: https://github.com/llvm/llvm-project/commit/08da343750be3ff6940ebca35be6a3bfd38cc5de
DIFF: https://github.com/llvm/llvm-project/commit/08da343750be3ff6940ebca35be6a3bfd38cc5de.diff

LOG: [Convergence] allow non-convergent ops before entry and loop intrinsics (#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.

Added: 
    

Modified: 
    llvm/docs/ConvergentOperations.rst
    llvm/include/llvm/ADT/GenericConvergenceVerifier.h
    llvm/include/llvm/IR/GenericConvergenceVerifierImpl.h
    llvm/test/Verifier/convergencectrl-invalid.ll

Removed: 
    


################################################################################
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
 }