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

[Sameer Sahasrabuddhe via Phabricator via llvm-commits]

sameerds marked 9 inline comments as done.
sameerds added inline comments.


================
Comment at: llvm/docs/ConvergentOperations.rst:140
+
+  end:
+  }
----------------
arsenm wrote:
> Block is missing a terminator. Also should have a token use?
Tokens are used only on convergent operations. A token doesn't need to be kept alive beyond the last convergent op that uses it.


================
Comment at: llvm/lib/IR/Verifier.cpp:2559
+  for (BasicBlock *BB : RPOT) {
+    SmallVector<CallBase *, 8> LiveTokens;
+    auto LTIt = LiveTokenMap.find(BB);
----------------
arsenm wrote:
> Don't reconstruct each iteration?
Reconstruction on each iteration is not something I've thought about much. But the programmer's manual only mentions this for std::vector and not SmallVector. Moved the declaration out of the loop anyway.


================
Comment at: llvm/test/Analysis/UniformityAnalysis/AMDGPU/join-at-loop-heart.ll:9
-
-define void @nested_loop_extension() {
-entry:
----------------
arsenm wrote:
> Why was this deleted?
This test was incorrectly added with the change that introduced uniformity analysis. The current semantics disallow a heart anywhere other than a loop header, so the example in this test is now invalid.


================
Comment at: llvm/test/Verifier/convergencectrl-invalid.ll:219
+}
+
+declare void @f() convergent
----------------
arsenm wrote:
> Need some tests with invoke and demonstrate the exception issues
Well it turns out that EH landing pads occur in blocks where we can't have a call to the entry() or loop() intrinsics. There are some rules about the predecessor blocks, which prevent landing pads in the entry block or in a loop header. So there is nothing to test here.

Note to self: If there is no conflict, might as well remove the comments from Verifier.cpp.

FIXME: A loop intrinsic is required to be the first non-PHI only if it is a true heart (in a loop header). Verifier should not complain if it occurs in any other block.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147116