[PATCH] D85609: Transforms: add ConvergenceControlHeuristic pass

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

nhaehnle marked an inline comment as done.
nhaehnle added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/ConvergenceControlHeuristic.cpp:115
+    if (auto *call = dyn_cast<CallInst>(op->getInstruction())) {
+      // Recreate the call, adding a single additional operand bundle.
+      IRBuilder<> builder(call);
----------------
arsenm wrote:
> Why do you need to recreate it? Can't you just directly add the bundle / mutate the instructon? That would also make invoke support free?
Is that possible? There isn't really any infrastructure for adding or removing operands on a `User`, and with the way that they're allocated it would seem to be a rather invasive change to allow that.


================
Comment at: llvm/lib/Transforms/Utils/ConvergenceControlHeuristic.cpp:125
+      bundles.push_back(m_convergenceInfo.makeOperandBundleDef(token));
+
+      CallInst *newCall =
----------------
arsenm wrote:
> Can you also add a test with an asm call
> Can you also add a test with an asm call

Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85609