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

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added inline comments.


================
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]]
----------------
MaskRay wrote:
> ChuanqiXu wrote:
> > nickdesaulniers wrote:
> > > 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: {{.*}}:`
> > > 
> > > ?
> > I feel it is a good way to check there is no new BB declaration before `llvm.dbg.declare`
> Actually I think `utils/update_test_checks.py` may not be a bad choice for this large test file.
> 
> If it is not time to migrate the test, I think the patch as-is using `; CHECK-NEXT: %1 = load i8*, i8** ; ...` looks good.
> It doesn't appear that there is more maintenance burden than `; CHECK-NEXT-NOT: {{.*}}:`
Curiously, I tried removing existing CHECK lines from this test, then running `./llvm/utils/update_test_checks.py` on this. The result doesn't pass `llvm-lit -vv llvm/test/Transforms/Coroutines/coro-debug.ll` which may be why this test did not use `./llvm/utils/update_test_checks.py` in the first place.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140166