[PATCH] D84108: [SimplifyCFG][LoopRotate] SimplifyCFG: disable common instruction hoisting by default, enable late in pipeline

Sat Jul 18 13:30:27 PDT 2020

lebedev.ri created this revision.
lebedev.ri added reviewers: reames, fhahn, mkazantsev, spatel.
lebedev.ri added a project: LLVM.
Herald added subscribers: dexonsmith, hiraditya, kristof.beyls, nemanjai.

I've been looking at missed vectorizations in one codebase. One particular thing
that stands out is that some of the loops reach vectorizer in a rather mangled
form, with weird PHI's, and some of the loops aren't even in a rotated form.

After taking a more detailed look, that happened because the loop's headers were
too big by then. It is evident that SimplifyCFG's common code hoisting transform
is at fault there, because the pattern it handles is precisely the unrotated
loop basic block structure.

Surprizingly, `SimplifyCFGOpt::HoistThenElseCodeToIf()` is enabled by default,
and is always run, unlike it's friend, common code sinking transform,
`SinkCommonCodeFromPredecessors()`, which is not enabled by default and is only
run once very late in the pipeline.

I'm proposing to harmonize this, and disable common code hoisting until //late//
in pipeline. Definition of //late// may vary, currently i've picked the same one
as for code sinking, but i suppose we could enable it as soon as right after
loop rotation happens.

Experimentation shows that this does indeed unsurprizingly help, more loops got
rotated, although other issues remain elsewhere.

Now, this undoubtedly seriously shakes phase ordering. This will undoubtedly be
a mixed bag in terms of both compile- and run- time performance, codesize.
Since we no longer aggressively hoist+deduplicate common code, we don't pay the
price of said hoisting (which wasn't big). That may allow more loops to be
rotated, so we pay that price. That, in turn, that may enable all the transforms
that require canonical (rotated) loop form, including but not limited to
vectorization, so we pay that too. And in general, no deduplication means
more [duplicate] instructions going through the optimizations. But there's still
late hoisting, some of them will be caught late.

As per benchmarks i've run F12360204: rsbench.txt <https://reviews.llvm.org/F12360204>, this is mostly within the noise,
there are some small improvements, some small regressions. One big regression
i saw i fixed in rG8d487668d09fb0e4e54f36207f07c1480ffabbfd <https://reviews.llvm.org/rG8d487668d09fb0e4e54f36207f07c1480ffabbfd>, but i'm sure
this will expose many more pre-existing missed optimizations, as usual :S

llvm-compile-time-tracker.com thoughts on this:
http://llvm-compile-time-tracker.com/compare.php?from=9dceb32f300d021bf6ace6d27ffaf670799e2e8d&to=a9beb54ebaea0c8382ef0a23985b6b661b407ad9&stat=instructions

- this does regress compile-time by +0.5% geomean (unsurprizingly)
- size impact varies; for ThinLTO it's actually an improvement

If we look at stats before/after diffs, some excerpts:

- RawSpeed (the target) F12360200: total-rs-diff.csv <https://reviews.llvm.org/F12360200>
  - -14 (-73.68%) loops not rotated due to the header size (yay)
  - -272 (-0.67%) `"Number of live out of a loop variables"` - good for vectorizer
  - -3937 (-64.19%) common instructions hoisted
  - +561 (+0.06%) x86 asm instructions
  - -2 basic blocks
  - +2418 (+0.11%) IR instructions
- vanilla test-suite + RawSpeed + darktable  F12360201: total-diff.csv <https://reviews.llvm.org/F12360201>
  - -36396 (-65.29%) common instructions hoisted
  - +1676 (+0.02%) x86 asm instructions
  - +662 (+0.06%) basic blocks
  - +4395 (+0.04%) IR instructions

Thoughts?

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84108

Files:
  llvm/include/llvm/Transforms/Utils/SimplifyCFGOptions.h
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
  llvm/lib/Target/ARM/ARMTargetMachine.cpp
  llvm/lib/Target/Hexagon/HexagonTargetMachine.cpp
  llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
  llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp
  llvm/lib/Transforms/Utils/SimplifyCFG.cpp
  llvm/test/CodeGen/X86/pr39187-g.ll
  llvm/test/Transforms/GVNSink/indirect-call.ll
  llvm/test/Transforms/GVNSink/sink-common-code.ll
  llvm/test/Transforms/PGOProfile/chr.ll
  llvm/test/Transforms/PhaseOrdering/loop-rotation-vs-common-code-hoisting.ll
  llvm/test/Transforms/SimplifyCFG/2008-12-16-DCECond.ll
  llvm/test/Transforms/SimplifyCFG/AArch64/prefer-fma.ll
  llvm/test/Transforms/SimplifyCFG/BrUnwind.ll
  llvm/test/Transforms/SimplifyCFG/HoistCode.ll
  llvm/test/Transforms/SimplifyCFG/PowerPC/prefer-fma.ll
  llvm/test/Transforms/SimplifyCFG/UncondBranchToReturn.ll
  llvm/test/Transforms/SimplifyCFG/X86/empty-cleanuppad.ll
  llvm/test/Transforms/SimplifyCFG/X86/remove-debug.ll
  llvm/test/Transforms/SimplifyCFG/hoist-common-code.ll
  llvm/test/Transforms/SimplifyCFG/hoist-dbgvalue-inlined.ll
  llvm/test/Transforms/SimplifyCFG/hoist-with-range.ll
  llvm/test/Transforms/SimplifyCFG/pr39807.ll
  llvm/test/Transforms/SimplifyCFG/preserve-load-metadata-2.ll
  llvm/test/Transforms/SimplifyCFG/preserve-load-metadata-3.ll
  llvm/test/Transforms/SimplifyCFG/preserve-load-metadata.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D84108.279023.patch
Type: text/x-patch
Size: 25774 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200718/ac306f68/attachment.bin>