[PATCH] D104870: [SimplifyCFG] Tail-merging all blocks with `unreachable` terminator

[Reid Kleckner via Phabricator via llvm-commits]

rnk added subscribers: jmorse, aheejin, nhaehnle, aprantl, dschuff.
rnk added a comment.

This change deserves some discussion. To address the issues with regions, we shouldn't rely on my hazily remembered understandings, we should consult the experts.

- For wasm, let's try @aheejin and @dschuff
- For GPUs, let's try @nhaehnle and @arsenm
- WinEH isn't an issue, we future proofed it against this transform
- For debug info, let's try @aprantl and @jmorse

To those I just added to the review, the question is, will tail merging calls to noreturn functions in IR (BranchFolding already does it in codegen) be an issue for the subsystems you contribute to.



================
Comment at: llvm/test/Transforms/SimplifyCFG/tail-merge-noreturn.ll:129-130
 ; CHECK:       a2:
 ; CHECK-NEXT:    call void @assert_fail_1_alt(i32 0)
 ; CHECK-NEXT:    unreachable
 ;
----------------
I expected your code to fire on this test case. Can you explain why this example isn't getting tail merged?

Consider this example: https://gcc.godbolt.org/z/ox16a9P1z
```
[[noreturn]] void abort1();
[[noreturn]] void abort2();
[[noreturn]] void abort3();
bool cond();
void doAsserts() {
    if (cond()) abort1();
    if (cond()) abort2();
    if (cond()) abort3();
}
```

I think it is more canonical to leave these unreachable terminators in place after the calls to noreturn functions, rather than merging the unreachables together.

I just want to make sure your transform isn't firing, creating BBs, and then a later part of simplifycfg rolls the unreachables back up into place after the calls.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104870