[PATCH] D152431: [Inliner] Handle convergence control when inlining a call
Nicolai Hähnle via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 19 23:55:44 PDT 2023
nhaehnle added a comment.
Thanks, I like this much more :)
================
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();
----------------
sameerds wrote:
> nhaehnle wrote:
> > 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...
> Yes. We enforce it in D147116.
Okay thanks, I missed that the first time around.
This needs a little more thought on the various `getFirstWhatever` methods in `BasicBlock` though. My main immediate concern is with `IRBuilder::setInsertPointPastAllocas`, which uses `BasicBlock::getFirstNonPHIOrDbgOrAlloca`. We'd want this to set the insert point past the entry/loop intrinsic and any allocas.
I wouldn't be surprised if some of the other helpers there should also be adjusted -- for example, `getFirstInsertionPt` -- but that is less clear to me.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D152431/new/
https://reviews.llvm.org/D152431
More information about the llvm-commits
mailing list