[PATCH] D152431: [Inliner] Handle convergence control when inlining a call

[Nicolai Hähnle via Phabricator via llvm-commits]

nhaehnle added a comment.

The proliferation of the ConvergenceControlUsage makes me a bit uncomfortable. It adds the overhead of an additional iteration over entire functions every time a function is inlined or perhaps the inlining is even attempted. And the only thing we get in exchange is a safety check against mixing controlled and uncontrolled convergent operations. Is it really worth it? I'd expect that in general, a compiler either always emits controlled or always emits uncontrolled operations, which means this isn't actually a scenario we need to worry about too much.



================
Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:2303-2312
+  if (ConvergenceControlToken) {
+    auto *I = FirstNewBlock->getFirstNonPHI();
+    if (auto *IntrinsicCall = dyn_cast<IntrinsicInst>(I)) {
+      if (IntrinsicCall->getIntrinsicID() ==
+          Intrinsic::experimental_convergence_entry) {
+        IntrinsicCall->replaceAllUsesWith(ConvergenceControlToken);
+        IntrinsicCall->eraseFromParent();
----------------
This looks like it's making an assumption that the `convergence.entry` is the very first instruction. Do we really want to enforce that? Definitely needs a verifier check in that case...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152431