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

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D140166#4019070 <https://reviews.llvm.org/D140166#4019070>, @ChuanqiXu wrote:

> In D140166#4018476 <https://reviews.llvm.org/D140166#4018476>, @dblaikie wrote:
>
>>> Note that there is only a coroutine `a()` in the above example. The frontend generated code (with -g) will consist of 1144 lines of IR codes. I get it by:
>>>
>>>   clang++ -std=c++20 src.cpp -O3 -S -emit-llvm -g -Xclang -disable-llvm-passes -o frontend-generated.ll
>>>
>>> And for the optimized (but before coroutine splitting) IR, it'll still consist of 645 lines of IR code. (I get it by inserting codes in CoroSplit.cpp). So I feel it is too lengthy for a lit test.
>>
>> That's with all LLVM passes disabled - it might be interesting if there's something more manageable with some optimization passes applied? (I guess if you don't disable them from clang, -O3 ends up lowering the IR beyond the coroutine abstracitons - so you'd have to do this manually with opt or something to only apply some optimizations and stop before the coroutines were lowered too far/beyond what's needed for this test)
>
> As far as I know, I don't the method to generate the IR before Coro-splitting automatically. In the past I always tried to insert codes in CoroSplit pass manually. I didn't feel tired for it.

Sorry, I didn't follow this - could you rephrase?

>> Though, yeah, it still wouldn't surprise me if it's infeasibly long.
>>
>> That said - I'm still confused about the CHECK-NOT for the label - that looked roughly OK to me/didn't seem too brittle (at least with checking for the end of line, if that's practical)
>
> In the test, we don't care about the inserted instructions. We only care about that the `@llvm.dbg.declare` lives in the `DEFAULT_DEST` BB. So it is slightly better to me if we only check the interesting part. The good part if one day someone else made a similar change (insert some instructions to the BB or remove some instructions in the BB), then he probably don't need to worry about the test. Personally, I feel bad when I made a change and I ran the test and many test failed but I found they are not related to my change. So I feel better to add `CHECK-NOT` to skip these tests.

Yeah, roughly following here & I think that's my understanding as well.

@MaskRay Could you describe more/restate your concerns with the particular `CHECK-NOT: {{.*}}:` you're referring to?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140166