[PATCH] D88438: BreakCriticalEdges: do not split the critical edge from a CallBr indirect successor

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added inline comments.


================
Comment at: llvm/test/Transforms/LoopStrengthReduce/callbr-critical-edge-splitting.ll:1
+; RUN: opt < %s -o /dev/null -loop-reduce
+
----------------
nickdesaulniers wrote:
> nickdesaulniers wrote:
> > MaskRay wrote:
> > > Just use the new PM way: `-passes='loop(loop-reduce)'`
> > If I do that for both tests:
> > 1. this one passes before this patch has been applied. (bad)(lol, wut)
> > 2. the other test still fails. (good)
> > 
> > Is there something funny with NPM that I need to specify additional passes in addition to `loop(loop-reduce)`?
> Let me be more precise:
> 
> Before this patch is applied:
> 
> -loop-reduce:
>   callbr-critical-edge-splitting.ll:  fail
>   callbr-critical-edge-splitting2.ll: fail
> 
> -passes=loop-reduce:
>   callbr-critical-edge-splitting.ll:  pass (WTF)
>   callbr-critical-edge-splitting2.ll: fail
> 
> -passes='loop(loop-reduce)':
>   callbr-critical-edge-splitting.ll:  pass (WTF)
>   callbr-critical-edge-splitting2.ll: fail
> 
> So I guess it makes sense to drop `-passes=loop-reduce` if that's covered by `-passes='loop(loop-reduce)'`, but it's curious to me from a TDD perspective why the test is green pre-patch for one pass manager (the new one) and red pre-patch for another pass manager (the old one).
> 
> > The legacy PM test does not add too much value for new tests as we'll soon make a switch.
> 
> But the test is red pre-patch for legacy PM. WTF
The output between -debug-only=loop-pass is a bit different between `-loop-reduce` and `-passes='loop(loop-reduce)'` as well...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88438