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

Tue Dec 20 14:37:07 PST 2022

nickdesaulniers added a comment.

In D140166#4001785 <https://reviews.llvm.org/D140166#4001785>, @StephenTozer wrote:

> Not very familiar with this area, but it looks as though currently `InstCombinerImpl::transformConstExprCastCall` in `InstCombineCalls.cpp` will need updating as well. Specifically it may transform a `CallBr` instruction and insert a Cast for the return, for which it calls `NewCall->getInsertionPointAfterDef()` and asserts that the return is non-null. Not sure what the solution would be, but it might require dropping support for `CallBr` in `transformConstExprCastCall` if the return type is changed: there's already a block in there that bails out of transforming invoke or callbr instructions if they are used in a PHI node, since there's no place to insert the Cast in that case.

Good catch! (I think we were ok since there's already a guard on the callee being a Function and not an InlineAsm and today we don't have frontends generate callbr to Functions; but we could, so I've cleaned that all up, PTAL).

================
Comment at: llvm/test/Transforms/Coroutines/coro-debug.ll:196-198
+; CHECK-NEXT: %1 = load i8*, i8** %coro_hdl.reload.addr
+; CHECK-NEXT: call void @free(i8* %1)
 ; CHECK-NEXT: call void @llvm.dbg.declare(metadata i32 %[[CALLBR_RES]]
----------------
ChuanqiXu wrote:
> We don't care about the inserted checks in the test. It should be fine to check the `llvm.dbg.declare` is in the basic block of `DEFAULT_DEST`. So maybe we can check these 2 lines are not empty or we can check there is no new BB declaration before  `llvm.dbg.declare`.
I'm not sure how best to express that to FileCheck.

`; CHECK-NEXT-NOT: {{.*}}:`

?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140166