[PATCH] D85603: IR: Add convergence control operand bundle and intrinsics

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

Needs some bitcode and IR parser tests. Some verifier checks for the rules would be nice too



================
Comment at: llvm/docs/ConvergentOperations.rst:295-296
+
+Behavior is undefined if this intrinsic appears in a function that isn't
+``convergent``.
+
----------------
Should this just be a verifier error? Why make it undefined?


================
Comment at: llvm/docs/ConvergentOperations.rst:298-299
+
+Behavior is undefined if this intrinsic appears inside of another convergence
+region or outside of a function's entry block.
+
----------------
This should also just be a verifier check?


================
Comment at: llvm/docs/ConvergentOperations.rst:372-388
+1. Every cycle in the CFG that contains a use of a convergence token other
+   than a use by
+   :ref:`llvm.experimental.convergence.loop <llvm.experimental.convergence.loop>`
+   must also contain the definition of the token.
+
+2. Every cycle in the CFG that contains two or more static uses of a
+   convergence token by
----------------
Is there a verifier implemented for these rules?


================
Comment at: llvm/docs/ConvergentOperations.rst:446
+  while (counter > 0) {
+    %tok = call tok @llvm.experimental.convergence.anchor()
+    call void @convergent.operation() [ "convergencectrl"(token %tok) ]
----------------
This and a lot of the later examples use "call tok" instead of the proper "call token"


================
Comment at: llvm/docs/ConvergentOperations.rst:517
+
+  %tok = @call tok llvm.experimental.convergence.anchor()
+  if (condition) {
----------------
@ in wrong place for the call


================
Comment at: llvm/docs/ConvergentOperations.rst:531
+  if (condition) {
+    %tok = @call tok llvm.experimental.convergence.anchor()
+    call void @convergent.operation() [ "convergencectrl"(token %tok) ]
----------------
%tok defined in both branches looks like broken SSA to me


================
Comment at: llvm/docs/ConvergentOperations.rst:544
+  if (condition) {
+  call void @convergent.operation() [ "convergencectrl"(token %tok) ]
+  } else {
----------------
Indentation


================
Comment at: llvm/docs/LangRef.rst:1607
+    Note that `convergent` operations can involve communication that is
+    considered to be not through memory and does notnecessarily imply an
+    ordering between threads for the purposes of the memory model. Therefore,
----------------
Missing space in notnecessarily


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85603