[PATCH] D152730: [ConstraintElim] Add A < B if A is an increasing phi for A != B.

[Florian Hahn via Phabricator via llvm-commits]

fhahn updated this revision to Diff 551184.
fhahn added a comment.
Herald added a subscriber: wenlei.



In D152730#4550104 <https://reviews.llvm.org/D152730#4550104>, @nikic wrote:

> The last time I looked at this, I think the logic was correct, but I was having some serious doubts that doing this in ConstraintElim is really a good idea. A big red flag here is that this will only work if the "primary" loop condition comes first, while in canonical (rotated) form, it would be last, on the loop latch. It happens to work out for the motivating example because ConstraintElim runs fairly early, but that also means we are locked into that pipeline position, and that this may not generalize to other frontends. There are other weaknesses (e.g. the fact that this only works on pre-inc IV, even though post-inc is canonical), but support for those can be added. The dependence on the order of exits seems like a fundamental limitation that is hard to avoid.
>
> optimizeLoopExits() in IndVarSimplify is normally the transform for eliminating such exits. That one can handle exits in any order, and I does eliminate such checks in many other cases. What prevents it from optimizing this particular case? Is that a fundamental limitation of optimizeLoopExits(), or something that can be fixed? Is there some other reason we should be doing this in ConstraintElim rather than IndVars (or other SCEV-based transform)?

My main motivation for this is to be able to easily combine the info about the induction with other facts  that are available to ConstraintElimination (in particular that the offset is signed positive from outside the loop); it might be possible to handle this pattern directly in `optimizeLoopExits` (possibly in combination with `applyLoopGuards`), but augmenting ConstraintElimination with info about induction phis should allow additional optimizations by combining other facts in a generic way.

ConstraintElimination at the moment is deliberately placed before LoopRotation, and it works quite well in practice for less-than/greater-than loop conditions. A more generic way to handle this in ConstraintElimination would be to compute the final value of inductions based on the symbolic trip count and inject that as upper bound for the induction at entry of the header block, but that is a bit more expensive. I experimented with a few different options to rework the patch using SCEV and the more general handling adds about +0.3 - 0.4% compile time overall. (//github.com/apple/llvm-project/commit/83e1361ff2eee1a14e96025914f8d14ff347f01b)

Using SCEV to detect induction monotonic Add recurrence phis and getting the step doesn't seem more expensive than the current implementation and I sketched that together with adding the facts to the initial worklist (rather than later) in the updated patch.  I've also put together a variant of this patch that uses SCEV to identify the recurrence + get the step (https://github.com/fhahn/llvm-project/commit/e655ef3b8c62929581c839803d9872d93abf327b).

WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152730

Files:
  llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-lto-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
  llvm/test/Transforms/ConstraintElimination/loops-header-tested-pointer-cmps.ll
  llvm/test/Transforms/ConstraintElimination/monotonic-int-phis.ll
  llvm/test/Transforms/ConstraintElimination/monotonic-pointer-phis-constant-upper-offset.ll
  llvm/test/Transforms/ConstraintElimination/monotonic-pointer-phis-custom-datalayout.ll
  llvm/test/Transforms/ConstraintElimination/monotonic-pointer-phis-early-exits.ll
  llvm/test/Transforms/ConstraintElimination/monotonic-pointer-phis-struct-types.ll
  llvm/test/Transforms/ConstraintElimination/monotonic-pointer-phis.ll
  llvm/test/Transforms/PhaseOrdering/iterator-with-runtime-check.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D152730.551184.patch
Type: text/x-patch
Size: 38828 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230817/a8b3a717/attachment.bin>