[PATCH] D91800: [PassManager] Run Induction Variable Simplification pass *after* Recognize loop idioms pass, not before

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri updated this revision to Diff 307523.
lebedev.ri edited the summary of this revision.
lebedev.ri added a reviewer: echristo.
lebedev.ri added a comment.
Herald added a subscriber: javed.absar.

In D91800#2408151 <https://reviews.llvm.org/D91800#2408151>, @dmgreen wrote:

> I have certainly seen places where we have managed to recognize a loop as a memcpy/memset, but not remove the remaining now empty loop. Here is some more fuel for your fire if you need it: https://godbolt.org/z/eW4rsY

Thank you for the yet another motivational example, test added!

In D91800#2415347 <https://reviews.llvm.org/D91800#2415347>, @mkazantsev wrote:

> The change looks very reasonable, though I don't know what motivation was behind the current ordering. There might have been some (arcane) reasons behind it. Fine by me, but please have someone else to take a look.

Yep. LoopIdiom runs on two types of loops: countable ones, and uncountable ones.
For uncountable ones, IndVars obviously didn't make any change to them, since they are uncountable, so for them the order should be irrelevant.
For countable ones, well, they should have been countable before IndVars for IndVars to make any change to them,
and since SCEV is used on them, it shouldn't matter if IndVars have already canonicalized them.
So i don't really see why we'd want the current ordering.

With that, does anyone feel confident accepting? :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91800

Files:
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
  llvm/test/CodeGen/AMDGPU/opt-pipeline.ll
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-thinlto-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-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
  llvm/test/Other/opt-O2-pipeline.ll
  llvm/test/Other/opt-O3-pipeline-enable-matrix.ll
  llvm/test/Other/opt-O3-pipeline.ll
  llvm/test/Other/opt-Os-pipeline.ll
  llvm/test/Transforms/PhaseOrdering/ARM/arm_fill_q7.ll
  llvm/test/Transforms/PhaseOrdering/X86/loop-idiom-vs-indvars.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D91800.307523.patch
Type: text/x-patch
Size: 18480 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201125/964c30b8/attachment.bin>