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

[Stephen Tozer via Phabricator via llvm-commits]

StephenTozer added a comment.

Small point about the CallBr guard, but no other issues from my PoV!



================
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;
----------------
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.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:3413
       return false; // TODO: Handle multiple return values.
 
     if (!CastInst::isBitOrNoopPointerCastable(NewRetTy, OldRetTy, DL)) {
----------------
See above comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140166