[IndVarSimplify] Don't perform LFTR only to convert the predicate

[nikic]

No description provided.

[github-actions]

⚠️ undef deprecator found issues in your code. ⚠️

You can test this locally with the following command:

git diff -U0 --pickaxe-regex -S '([^a-zA-Z0-9#_-]undef[^a-zA-Z0-9_-]|UndefValue::get)' 88b1d16c4a040cc3d919be13f0baf44f30a5aa13 995d1e152b90801bf7dce924cad3ef393bc91bdc llvm/lib/Transforms/Scalar/IndVarSimplify.cpp llvm/test/Transforms/IndVarSimplify/2011-11-01-lftrptr.ll llvm/test/Transforms/IndVarSimplify/AArch64/widen-loop-comp.ll llvm/test/Transforms/IndVarSimplify/X86/eliminate-trunc.ll llvm/test/Transforms/IndVarSimplify/X86/inner-loop-by-latch-cond.ll llvm/test/Transforms/IndVarSimplify/X86/iv-widen.ll llvm/test/Transforms/IndVarSimplify/X86/loop-invariant-conditions.ll llvm/test/Transforms/IndVarSimplify/X86/pr35406.ll llvm/test/Transforms/IndVarSimplify/X86/pr59615.ll llvm/test/Transforms/IndVarSimplify/X86/widening-vs-and-elimination.ll llvm/test/Transforms/IndVarSimplify/ada-loops.ll llvm/test/Transforms/IndVarSimplify/ashr-expansion.ll llvm/test/Transforms/IndVarSimplify/canonicalize-cmp.ll llvm/test/Transforms/IndVarSimplify/deterministic-sign.ll llvm/test/Transforms/IndVarSimplify/drop-exact.ll llvm/test/Transforms/IndVarSimplify/elim-extend.ll llvm/test/Transforms/IndVarSimplify/eliminate-comparison.ll llvm/test/Transforms/IndVarSimplify/finite-exit-comparisons.ll llvm/test/Transforms/IndVarSimplify/full_widening.ll llvm/test/Transforms/IndVarSimplify/iv-ext-samesign.ll llvm/test/Transforms/IndVarSimplify/iv-sext.ll llvm/test/Transforms/IndVarSimplify/iv-widen-elim-ext.ll llvm/test/Transforms/IndVarSimplify/lftr-address-space-pointers.ll llvm/test/Transforms/IndVarSimplify/lftr-dead-ivs.ll llvm/test/Transforms/IndVarSimplify/lftr-multi-exit.ll llvm/test/Transforms/IndVarSimplify/lftr-pr20680.ll llvm/test/Transforms/IndVarSimplify/lftr-pr31181.ll llvm/test/Transforms/IndVarSimplify/lftr-reuse.ll llvm/test/Transforms/IndVarSimplify/lftr.ll llvm/test/Transforms/IndVarSimplify/no-iv-rewrite.ll llvm/test/Transforms/IndVarSimplify/post-inc-range.ll llvm/test/Transforms/IndVarSimplify/pr79861.ll llvm/test/Transforms/IndVarSimplify/promote-iv-to-eliminate-casts.ll llvm/test/Transforms/IndVarSimplify/rewrite-loop-exit-values-phi.ll llvm/test/Transforms/IndVarSimplify/rlev-add-me.ll llvm/test/Transforms/IndVarSimplify/trip-count-expansion-loop-guard-preserve-nsw.ll llvm/test/Transforms/IndVarSimplify/ult-sub-to-eq.ll llvm/test/Transforms/IndVarSimplify/widen-nonnegative.ll

The following files introduce new uses of undef:

• llvm/test/Transforms/IndVarSimplify/rewrite-loop-exit-values-phi.ll

Undef is now deprecated and should only be used in the rare cases where no replacement is possible. For example, a load of uninitialized memory yields undef. You should use poison values for placeholders instead.

In tests, avoid using undef and having tests that trigger undefined behavior. If you need an operand with some unimportant value, you can add a new argument to the function and use that instead.

For example, this is considered a bad practice:

define void @fn() {
  ...
  br i1 undef, ...
}

Please use the following instead:

define void @fn(i1 %cond) {
  ...
  br i1 %cond, ...
}

Please refer to the Undefined Behavior Manual for more information.

[artagnon]

Oh yeah, I had the same patch yesterday, but the crazy amount of test updates prevented me from putting it up. Not sure who can say if this is a good idea: maybe @topperc?

[nikic]

I think @preames is probably the person with the most context on this.

I think the key question is: What are we trying to achieve with LFTR?

I think that originally, LFTR was probably envisioned as a canonicalizing transform, so that all loops with a known BECount take the same form of an equality comparison of a canonical IV against the BECount. In practice, this doesn't work out in multiple ways, the most notable being that we have to avoid high cost expansions, so we don't actually perform LFTR in many cases where it is, theoretically, possible.

Viewed as a canonical form, I'm not really sure what the value of LFTR is. For passes using SCEV it doesn't matter either way (evidenced by the fact that it could compute the BECount in the first place). For passes not using SCEV, the equality LFTR form is arguably worse than using an inequality exit condition -- equality exit conditions can essentially only be handled by sophisticated analysis like SCEV, while inequalities at least have trivial range implications.

Now, if we instead only consider it as an optimization, then we have to take a closer look at the costs. If LFTR allows us to eliminate instructions in the loop for additional instructions in the preheader, that will often be a profitable transform. This happens for example if LFTR can replace and and/or exit condition with a min/max comparison. However, if all LFTR does is change the predicate, then adding any number of additional preheader instructions is a bad tradeoff (unless it allows eliminating an IV).

There's also the additional complexity that IVs will be rewritten by LSR to be more efficient for the target, and that we have LoopTermFold, which is basically LFTR but performed post-LSR in the backend and targeting at the elimination of IVs.

[artagnon]

Based on my investigation, I concur with what you have said. I couldn't quite figure out the purpose of the LFTR, as it always seemed to be making code worse or doing redundant work that would be better left to the dedicated LSR: LoopTermFold is new and was recently split off from LSR. I was just looking at 7da2417 (indvars: LinearFunctionTestReplace for non-canonical IVs) from fourteen years ago, but I'm not sure what the original justification was: maybe LLVM was somehow better with eq/ne conditions back then, and LSR hadn't matured?

That said, I'm happy to proceed with the gradual removal of the LFTR. My concern is that IVSimplify is a very core transform, and any change to it has to be checked thoroughly, and I fear that we wouldn't be able to audit the crazy-large diffs when running each change against a large corpus.