[PATCH] D140166: [IR] return nullptr in Instruction::getInsertionPointAfterDef for CallBrInst

Nikita Popov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 17 13:21:42 PST 2023


nikic added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:3381-3383
+  // CallBr's don't have a single point after a def to insert at.
+  if (isa<CallBrInst>(Call))
+    return false;
----------------
nickdesaulniers wrote:
> nikic wrote:
> > StephenTozer wrote:
> > > This might be more aggressive than necessary, since if the return type doesn't change or the call instruction has no use, a PHI does not need to be inserted; this could be moderated by moving this to the block at line 3409 (`if (OldRetTy != NewRetTy)`), and checking whether the return value has any uses. With that said, this is a minor point since as you said this won't even be touched at this point, so doesn't need to block merging imo.
> > This entire transform is not relevant for callbr, it only works on calls to functions, not calls to inline asm. You could replace this with `assert(!isa<CallBrInst>(Call))`.
> adding the assert causes Transforms/InstCombine/freeze.ll to fail, hitting that assertion.
Where did you place the assert? It needs to be after the callee check above, not at the start of the function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140166



More information about the llvm-commits mailing list