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

Reid Kleckner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 24 14:08:51 PDT 2021


rnk added inline comments.


================
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
 ;
----------------
lebedev.ri wrote:
> rnk wrote:
> > 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.
> > I expected your code to fire on this test case. Can you explain why this example isn't getting tail merged?
> 
> It fired, we didn't sink anything, and `SimplifyCFGOpt::simplifyUnreachable()` decided to undo it.
Got it, and we want to avoid that because otherwise it will make the overall pass return true to indicate that it changed something, which will make the parent pass manager re-run more passes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104870



More information about the llvm-commits mailing list