[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:
> 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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88438