[PATCH] D94470: [LSR] Don't break a critical edge if parent ends with "callbr"

[Bill Wendling via Phabricator via llvm-commits]

void added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp:5594-5595
+
+       // Also bail out if we have a PHI with a value from a block ending in a
+       // CallBrInst, because those also can't be split for similar reasons.
+       for (BasicBlock *PredBB : PN->blocks())
----------------
nickdesaulniers wrote:
> Can this comment be made more precise, I don't think what's described is what's being tested?
It's pretty accurate as is. :) I'm not sure how else to word it.


================
Comment at: llvm/test/CodeGen/Generic/callbr-critical-edge-splitting.ll:1
+; RUN: llc < %s -o /dev/null
+
----------------
MaskRay wrote:
> Can you write a `opt -passes='loop(loop-reduce)' ` test?
> 
> Checking a command returns 0 is not a useful test. Checking some desired instructions makes the test not only useful for one particular regression, but also useful for some other scenarios.
It replicates only with llc, not with opt.


================
Comment at: llvm/test/CodeGen/Generic/callbr-critical-edge-splitting.ll:21
+do.body.i.i.do.body.i.i_crit_edge:                ; preds = %do.body.i.i.do.body.i.i_crit_edge, %cond.true.i
+  %pgocount711 = phi i64 [ %0, %do.body.i.i.do.body.i.i_crit_edge ], [ 0, %cond.true.i ]
+  %0 = add nuw nsw i64 %pgocount711, 1
----------------
nickdesaulniers wrote:
> (I'm surprised we can refer to values in the IR before they've been assigned to).
Might be some PHI node weirdness.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94470