[PATCH] D147116: [RFC] Introduce convergence control intrinsics

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/docs/ConvergentOperations.rst:140
+
+  end:
+  }
----------------
Block is missing a terminator. Also should have a token use?


================
Comment at: llvm/docs/ConvergentOperations.rst:390
+  then:
+    %baseOffset.1 = atomicrmw add i32* @bufferAllocationCount, i32 %numThreads monotonic
+    br label %end
----------------
Use opaque pointers


================
Comment at: llvm/include/llvm/IR/Intrinsics.td:2505
+def int_experimental_convergence_entry
+  : Intrinsic<[llvm_token_ty], [], [IntrNoMem, IntrConvergent]>;
+def int_experimental_convergence_anchor
----------------
Needs a lot more intrinsic properties. Should really be DefaultAttrsIntrinsic +Convergent+NoMem


================
Comment at: llvm/lib/IR/Verifier.cpp:41
 //  * Landingpad instructions must be in a function with a personality function.
+//  * Convergence control intrinsics are introduced in ConvertentOperations.rst.
+//    The applied restrictions are too numerous to list here.
----------------
Typo ConvertentOperations


================
Comment at: llvm/test/Analysis/UniformityAnalysis/AMDGPU/join-at-loop-heart.ll:9
-
-define void @nested_loop_extension() {
-entry:
----------------
Why was this deleted?


================
Comment at: llvm/test/Verifier/convergencectrl-invalid.ll:171
+  %b = call token @llvm.experimental.convergence.anchor()
+  br i1 undef, label %C, label %D
+
----------------
Avoid undef


================
Comment at: llvm/test/Verifier/convergencectrl-invalid.ll:219
+}
+
+declare void @f() convergent
----------------
Need some tests with invoke and demonstrate the exception issues


================
Comment at: llvm/test/Verifier/convergencectrl-valid.ll:1
+; RUN: llvm-as < %s -o /dev/null
+
----------------
These kinds of tests should go in test/Assembler and roundtrip through llvm-as/llvm-dis

Also should get a bitcode compatibility test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147116/new/

https://reviews.llvm.org/D147116