[PATCH] D116692: [SimplifyCFG] Tail-merging all blocks with `unreachable` terminator, final take

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/test/Transforms/PhaseOrdering/AArch64/peel-multiple-unreachable-exits-for-vectorization.ll:39
-; CHECK-NEXT:    br i1 [[MIN_ITERS_CHECK]], label [[LOOP_PREHEADER22:%.*]], label [[VECTOR_PH:%.*]]
-; CHECK:       vector.ph:
-; CHECK-NEXT:    [[N_MOD_VF:%.*]] = and i64 [[TMP1]], 3
----------------
lebedev.ri wrote:
> lebedev.ri wrote:
> > rnk wrote:
> > > It looks to me like tail merging unreachable blocks is preventing vectorization in this test case and the next, which seems like a blocking issue. The test was added here, if that helps understand why it no longer works:
> > > https://reviews.llvm.org/rG39cc0b8c68b8d316954ecfac0d1f8498ea42866c
> > > @fhahn 
> > Filed https://github.com/llvm/llvm-project/issues/53020
> CC @aqjune @nikic
> 
> I'm not actually sure that we can solve this within LV itself,
> but i would love to be proven wrong here.
> 
> I'm pretty sure LV does expand the backedge taken count,
> so i suppose only not being allowed to expand BTC wouldn't help here.
> 
> As i see it, the options are:
> * ignore this failure
> * adjust the test to mask the failure (i would hope adding `noundef`'s should help?), potentially coupled with:
> * are there some missing reasoning bits in `impliesPoison()` and friends that could prevent this regression?
> * Introduce UB-safe mode for SCEVExpander, lift backedge taken count poison-safety restriction
> * Prevent simplifycfg from merging conditions like that (as in, iff plain `and`/`or` isn't going to be used)
> 
I think the proper way to address this (and other poison-safety issues in SCEV) is to add umin variants in both IR and SCEV that don't propagate op2 poison if op1 is zero.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116692