[PhaseOrdering] Remove RUN lines for legacy PM (NFC)

[InstCombine] Add general constant support to eq/ne icmp(add(X,C1),add(Y,C2)) -> icmp(add(X,C1-C2),Y) fold

A further extension for Issue #32161

For eq/ne comparisons - the sign mismatch and bounds

[InstCombine] Add general constant support to eq/ne icmp(add(X,C1),add(Y,C2)) -> icmp(add(X,C1-C2),Y) fold

A further extension for Issue #32161

For eq/ne comparisons - the sign mismatch and bounds constraints are redundant, so if the that fold fails, fallback and just fold the constants directly.

https://alive2.llvm.org/ce/z/cdodNQ

The loop rotation test change looks mostly benign - the backend doesn't seem to suffer? https://gcc.godbolt.org/z/dErMY78To

Differential Revision: https://reviews.llvm.org/D121551

show more ...

[PhaseOrdering] loop-rotation-vs-common-code-hoisting.ll - merge equivalent check-prefixes

[SCEVExpander] Drop poison generating flags when reusing instructions

The basic problem we have is that we're trying to reuse an instruction which is mapped to some SCEV. Since we can have multiple

[SCEVExpander] Drop poison generating flags when reusing instructions

The basic problem we have is that we're trying to reuse an instruction which is mapped to some SCEV. Since we can have multiple such instructions (potentially with different flags), this is analogous to our need to drop flags when performing CSE. A trivial implementation would simply drop flags on any instruction we decided to reuse, and that would be correct.

This patch is almost that trivial patch except that we preserve flags on the reused instruction when existing users would imply UB on overflow already. Adding new users can, at most, refine this program to one which doesn't execute UB which is valid.

In practice, this fixes two conceptual problems with the previous code: 1) a binop could have been canonicalized into a form with different opcode or operands, or 2) the inbounds GEP case which was simply unhandled.

On the test changes, most are pretty straight forward. We loose some flags (in some cases, they'd have been dropped on the next CSE pass anyways). The one that took me the longest to understand was the ashr-expansion test. What's happening there is that we're considering reuse of the mul, previously we disallowed it entirely, now we allow it with no flags. The surrounding diffs are all effects of generating the same mul with a different operand order, and then doing simple DCE.

The loss of the inbounds is unfortunate, but even there, we can recover most of those once we actually treat branch-on-poison as immediate UB.

Differential Revision: https://reviews.llvm.org/D112734

show more ...

[NewPM] Remove SpeculateAroundPHIs pass

Addition of this pass has been botched.
There is no particular reason why it had to be sold as an inseparable part
of new-pm transition. It was added when old

[NewPM] Remove SpeculateAroundPHIs pass

Addition of this pass has been botched.
There is no particular reason why it had to be sold as an inseparable part
of new-pm transition. It was added when old-pm was still the default,
and very *very* few users were actually tracking new-pm,
so it's effects weren't measured.

Which means, some of the turnoil of the new-pm transition
are actually likely regressions due to this pass.

Likewise, there has been a number of post-commit feedback
(post new-pm switch), namely
* https://reviews.llvm.org/D37467#2787157 (regresses HW-loops)
* https://reviews.llvm.org/D37467#2787259 (should not be in middle-end, should run after LSR, not before)
* https://reviews.llvm.org/D95789 (an attempt to fix bad loop backedge metadata)
and in the half year past, the pass authors (google) still haven't found time to respond to any of that.

Hereby it is proposed to backout the pass from the pipeline,
until someone who cares about it can address the issues reported,
and properly start the process of adding a new pass into the pipeline,
with proper performance evaluation.

Furthermore, neither google nor facebook reports any perf changes
from this change, so i'm dropping the pass completely.
It can always be re-reverted should/if anyone want to pick it up again.

Reviewed By: aeubanks

Differential Revision: https://reviews.llvm.org/D104099

show more ...

[PassManager] Run additional LICM before LoopRotate

Loop rotation often has to perform code duplication
from header into preheader, which introduces PHI nodes.

>>! In D99204, @thopre wrote:
>
> Wit

[PassManager] Run additional LICM before LoopRotate

Loop rotation often has to perform code duplication
from header into preheader, which introduces PHI nodes.

>>! In D99204, @thopre wrote:
>
> With loop peeling, it is important that unnecessary PHIs be avoided or
> it will leads to spurious peeling. One source of such PHIs is loop
> rotation which creates PHIs for invariant loads. Those PHIs are
> particularly problematic since loop peeling is now run as part of simple
> loop unrolling before GVN is run, and are thus a source of spurious
> peeling.
>
> Note that while some of the load can be hoisted and eventually
> eliminated by instruction combine, this is not always possible due to
> alignment issue. In particular, the motivating example [1] was a load
> inside a class instance which cannot be hoisted because the `this'
> pointer has an alignment of 1.
>
> [1] http://lists.llvm.org/pipermail/llvm-dev/attachments/20210312/4ce73c47/attachment.cpp

Now, we could enhance LoopRotate to avoid duplicating code when not needed,
but instead hoist loop-invariant code, but isn't that a code duplication? (*sic*)
We have LICM, and in fact we already run it right after LoopRotation.

We could try to move it to before LoopRotation,
that is basically free from compile-time perspective:
https://llvm-compile-time-tracker.com/compare.php?from=6c93eb4477d88af046b915bc955c03693b2cbb58&to=a4bee6d07732b1184c436da489040b912f0dc271&stat=instructions
But, looking at stats, i think it isn't great that we would no longer do LICM after LoopRotation, in particular:
| statistic name | LoopRotate-LICM | LICM-LoopRotate | Δ | % | abs(%) |
| asm-printer.EmittedInsts | 9015930 | 9015799 | -131 | 0.00% | 0.00% |
| indvars.NumElimCmp | 3536 | 3544 | 8 | 0.23% | 0.23% |
| indvars.NumElimExt | 36725 | 36580 | -145 | -0.39% | 0.39% |
| indvars.NumElimIV | 1197 | 1187 | -10 | -0.84% | 0.84% |
| indvars.NumElimIdentity | 143 | 136 | -7 | -4.90% | 4.90% |
| indvars.NumElimRem | 4 | 5 | 1 | 25.00% | 25.00% |
| indvars.NumLFTR | 29842 | 29890 | 48 | 0.16% | 0.16% |
| indvars.NumReplaced | 2293 | 2227 | -66 | -2.88% | 2.88% |
| indvars.NumSimplifiedSDiv | 6 | 8 | 2 | 33.33% | 33.33% |
| indvars.NumWidened | 26438 | 26329 | -109 | -0.41% | 0.41% |
| instcount.TotalBlocks | 1178338 | 1173840 | -4498 | -0.38% | 0.38% |
| instcount.TotalFuncs | 111825 | 111829 | 4 | 0.00% | 0.00% |
| instcount.TotalInsts | 9905442 | 9896139 | -9303 | -0.09% | 0.09% |
| lcssa.NumLCSSA | 425871 | 423961 | -1910 | -0.45% | 0.45% |
| licm.NumHoisted | 378357 | 378753 | 396 | 0.10% | 0.10% |
| licm.NumMovedCalls | 2193 | 2208 | 15 | 0.68% | 0.68% |
| licm.NumMovedLoads | 35899 | 31821 | -4078 | -11.36% | 11.36% |
| licm.NumPromoted | 11178 | 11154 | -24 | -0.21% | 0.21% |
| licm.NumSunk | 13359 | 13587 | 228 | 1.71% | 1.71% |
| loop-delete.NumDeleted | 8547 | 8402 | -145 | -1.70% | 1.70% |
| loop-instsimplify.NumSimplified | 12876 | 11890 | -986 | -7.66% | 7.66% |
| loop-peel.NumPeeled | 1008 | 925 | -83 | -8.23% | 8.23% |
| loop-rotate.NumNotRotatedDueToHeaderSize | 368 | 365 | -3 | -0.82% | 0.82% |
| loop-rotate.NumRotated | 42015 | 42003 | -12 | -0.03% | 0.03% |
| loop-simplifycfg.NumLoopBlocksDeleted | 240 | 242 | 2 | 0.83% | 0.83% |
| loop-simplifycfg.NumLoopExitsDeleted | 497 | 20 | -477 | -95.98% | 95.98% |
| loop-simplifycfg.NumTerminatorsFolded | 618 | 336 | -282 | -45.63% | 45.63% |
| loop-unroll.NumCompletelyUnrolled | 11028 | 11032 | 4 | 0.04% | 0.04% |
| loop-unroll.NumUnrolled | 12608 | 12529 | -79 | -0.63% | 0.63% |
| mem2reg.NumDeadAlloca | 10222 | 10221 | -1 | -0.01% | 0.01% |
| mem2reg.NumPHIInsert | 192110 | 192106 | -4 | 0.00% | 0.00% |
| mem2reg.NumSingleStore | 637650 | 637643 | -7 | 0.00% | 0.00% |
| scalar-evolution.NumBruteForceTripCountsComputed | 814 | 812 | -2 | -0.25% | 0.25% |
| scalar-evolution.NumTripCountsComputed | 283108 | 282934 | -174 | -0.06% | 0.06% |
| scalar-evolution.NumTripCountsNotComputed | 106712 | 106718 | 6 | 0.01% | 0.01% |
| simple-loop-unswitch.NumBranches | 5178 | 4752 | -426 | -8.23% | 8.23% |
| simple-loop-unswitch.NumCostMultiplierSkipped | 914 | 503 | -411 | -44.97% | 44.97% |
| simple-loop-unswitch.NumSwitches | 20 | 18 | -2 | -10.00% | 10.00% |
| simple-loop-unswitch.NumTrivial | 183 | 95 | -88 | -48.09% | 48.09% |

... but that actually regresses LICM (-12% `licm.NumMovedLoads`),
loop-simplifycfg (`NumLoopExitsDeleted`, `NumTerminatorsFolded`),
simple-loop-unswitch (`NumTrivial`).

What if we instead have LICM both before and after LoopRotate?
| statistic name | LoopRotate-LICM | LICM-LoopRotate-LICM | Δ | % | abs(%) |
| asm-printer.EmittedInsts | 9015930 | 9014474 | -1456 | -0.02% | 0.02% |
| indvars.NumElimCmp | 3536 | 3546 | 10 | 0.28% | 0.28% |
| indvars.NumElimExt | 36725 | 36681 | -44 | -0.12% | 0.12% |
| indvars.NumElimIV | 1197 | 1185 | -12 | -1.00% | 1.00% |
| indvars.NumElimIdentity | 143 | 146 | 3 | 2.10% | 2.10% |
| indvars.NumElimRem | 4 | 5 | 1 | 25.00% | 25.00% |
| indvars.NumLFTR | 29842 | 29899 | 57 | 0.19% | 0.19% |
| indvars.NumReplaced | 2293 | 2299 | 6 | 0.26% | 0.26% |
| indvars.NumSimplifiedSDiv | 6 | 8 | 2 | 33.33% | 33.33% |
| indvars.NumWidened | 26438 | 26404 | -34 | -0.13% | 0.13% |
| instcount.TotalBlocks | 1178338 | 1173652 | -4686 | -0.40% | 0.40% |
| instcount.TotalFuncs | 111825 | 111829 | 4 | 0.00% | 0.00% |
| instcount.TotalInsts | 9905442 | 9895452 | -9990 | -0.10% | 0.10% |
| lcssa.NumLCSSA | 425871 | 425373 | -498 | -0.12% | 0.12% |
| licm.NumHoisted | 378357 | 383352 | 4995 | 1.32% | 1.32% |
| licm.NumMovedCalls | 2193 | 2204 | 11 | 0.50% | 0.50% |
| licm.NumMovedLoads | 35899 | 35755 | -144 | -0.40% | 0.40% |
| licm.NumPromoted | 11178 | 11163 | -15 | -0.13% | 0.13% |
| licm.NumSunk | 13359 | 14321 | 962 | 7.20% | 7.20% |
| loop-delete.NumDeleted | 8547 | 8538 | -9 | -0.11% | 0.11% |
| loop-instsimplify.NumSimplified | 12876 | 12041 | -835 | -6.48% | 6.48% |
| loop-peel.NumPeeled | 1008 | 924 | -84 | -8.33% | 8.33% |
| loop-rotate.NumNotRotatedDueToHeaderSize | 368 | 365 | -3 | -0.82% | 0.82% |
| loop-rotate.NumRotated | 42015 | 42005 | -10 | -0.02% | 0.02% |
| loop-simplifycfg.NumLoopBlocksDeleted | 240 | 241 | 1 | 0.42% | 0.42% |
| loop-simplifycfg.NumTerminatorsFolded | 618 | 619 | 1 | 0.16% | 0.16% |
| loop-unroll.NumCompletelyUnrolled | 11028 | 11029 | 1 | 0.01% | 0.01% |
| loop-unroll.NumUnrolled | 12608 | 12525 | -83 | -0.66% | 0.66% |
| mem2reg.NumPHIInsert | 192110 | 192073 | -37 | -0.02% | 0.02% |
| mem2reg.NumSingleStore | 637650 | 637652 | 2 | 0.00% | 0.00% |
| scalar-evolution.NumTripCountsComputed | 283108 | 282998 | -110 | -0.04% | 0.04% |
| scalar-evolution.NumTripCountsNotComputed | 106712 | 106691 | -21 | -0.02% | 0.02% |
| simple-loop-unswitch.NumBranches | 5178 | 5185 | 7 | 0.14% | 0.14% |
| simple-loop-unswitch.NumCostMultiplierSkipped | 914 | 925 | 11 | 1.20% | 1.20% |
| simple-loop-unswitch.NumTrivial | 183 | 179 | -4 | -2.19% | 2.19% |
| simple-loop-unswitch.NumBranches | 5178 | 4752 | -426 | -8.23% | 8.23% |
| simple-loop-unswitch.NumCostMultiplierSkipped | 914 | 503 | -411 | -44.97% | 44.97% |
| simple-loop-unswitch.NumSwitches | 20 | 18 | -2 | -10.00% | 10.00% |
| simple-loop-unswitch.NumTrivial | 183 | 95 | -88 | -48.09% | 48.09% |

I.e. we end up with less instructions, less peeling, more LICM activity,
also note how none of those 4 regressions are here. Namely:

| statistic name | LICM-LoopRotate | LICM-LoopRotate-LICM | Δ | % | abs(%) |
| asm-printer.EmittedInsts | 9015799 | 9014474 | -1325 | -0.01% | 0.01% |
| indvars.NumElimCmp | 3544 | 3546 | 2 | 0.06% | 0.06% |
| indvars.NumElimExt | 36580 | 36681 | 101 | 0.28% | 0.28% |
| indvars.NumElimIV | 1187 | 1185 | -2 | -0.17% | 0.17% |
| indvars.NumElimIdentity | 136 | 146 | 10 | 7.35% | 7.35% |
| indvars.NumLFTR | 29890 | 29899 | 9 | 0.03% | 0.03% |
| indvars.NumReplaced | 2227 | 2299 | 72 | 3.23% | 3.23% |
| indvars.NumWidened | 26329 | 26404 | 75 | 0.28% | 0.28% |
| instcount.TotalBlocks | 1173840 | 1173652 | -188 | -0.02% | 0.02% |
| instcount.TotalInsts | 9896139 | 9895452 | -687 | -0.01% | 0.01% |
| lcssa.NumLCSSA | 423961 | 425373 | 1412 | 0.33% | 0.33% |
| licm.NumHoisted | 378753 | 383352 | 4599 | 1.21% | 1.21% |
| licm.NumMovedCalls | 2208 | 2204 | -4 | -0.18% | 0.18% |
| licm.NumMovedLoads | 31821 | 35755 | 3934 | 12.36% | 12.36% |
| licm.NumPromoted | 11154 | 11163 | 9 | 0.08% | 0.08% |
| licm.NumSunk | 13587 | 14321 | 734 | 5.40% | 5.40% |
| loop-delete.NumDeleted | 8402 | 8538 | 136 | 1.62% | 1.62% |
| loop-instsimplify.NumSimplified | 11890 | 12041 | 151 | 1.27% | 1.27% |
| loop-peel.NumPeeled | 925 | 924 | -1 | -0.11% | 0.11% |
| loop-rotate.NumRotated | 42003 | 42005 | 2 | 0.00% | 0.00% |
| loop-simplifycfg.NumLoopBlocksDeleted | 242 | 241 | -1 | -0.41% | 0.41% |
| loop-simplifycfg.NumLoopExitsDeleted | 20 | 497 | 477 | 2385.00% | 2385.00% |
| loop-simplifycfg.NumTerminatorsFolded | 336 | 619 | 283 | 84.23% | 84.23% |
| loop-unroll.NumCompletelyUnrolled | 11032 | 11029 | -3 | -0.03% | 0.03% |
| loop-unroll.NumUnrolled | 12529 | 12525 | -4 | -0.03% | 0.03% |
| mem2reg.NumDeadAlloca | 10221 | 10222 | 1 | 0.01% | 0.01% |
| mem2reg.NumPHIInsert | 192106 | 192073 | -33 | -0.02% | 0.02% |
| mem2reg.NumSingleStore | 637643 | 637652 | 9 | 0.00% | 0.00% |
| scalar-evolution.NumBruteForceTripCountsComputed | 812 | 814 | 2 | 0.25% | 0.25% |
| scalar-evolution.NumTripCountsComputed | 282934 | 282998 | 64 | 0.02% | 0.02% |
| scalar-evolution.NumTripCountsNotComputed | 106718 | 106691 | -27 | -0.03% | 0.03% |
| simple-loop-unswitch.NumBranches | 4752 | 5185 | 433 | 9.11% | 9.11% |
| simple-loop-unswitch.NumCostMultiplierSkipped | 503 | 925 | 422 | 83.90% | 83.90% |
| simple-loop-unswitch.NumSwitches | 18 | 20 | 2 | 11.11% | 11.11% |
| simple-loop-unswitch.NumTrivial | 95 | 179 | 84 | 88.42% | 88.42% |

{F15983613} {F15983615} {F15983616}
(this is vanilla llvm testsuite + rawspeed + darktable)

As an example of the code where early LICM only is bad, see:
https://godbolt.org/z/GzEbacs4K

This does have an observable compile-time regression of +~0.5% geomean
https://llvm-compile-time-tracker.com/compare.php?from=7c5222e4d1a3a14f029e5f614c9aefd0fa505f1e&to=5d81826c3411982ca26e46b9d0aff34c80577664&stat=instructions
but i think that's basically nothing, and there's potential that it might
be avoidable in the future by fixing clang to produce alignment information
on function arguments, thus making the second run unneeded.

Differential Revision: https://reviews.llvm.org/D99249

show more ...

[PhaseOrdering] Remove unused check-prefixes

Just use default CHECK in most cases.

[NPM] Fix some PhaseOrdering tests under NPM

These either already have corresponding NPM RUN lines,
or need to be fixed to not use -analyze.

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

This was reverted in 503deec2183d466dad64b763bab4e15fd8804239
because it caused

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

This was reverted in 503deec2183d466dad64b763bab4e15fd8804239
because it caused gigantic increase (3x) in branch mispredictions
in certain benchmarks on certain CPU's,
see https://reviews.llvm.org/D84108#2227365.

It has since been investigated and here are the results:
https://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20200907/827578.html
> It's an amazingly severe regression, but it's also all due to branch
> mispredicts (about 3x without this). The code layout looks ok so there's
> probably something else to deal with. I'm not sure there's anything we can
> reasonably do so we'll just have to take the hit for now and wait for
> another code reorganization to make the branch predictor a bit more happy :)
>
> Thanks for giving us some time to investigate and feel free to recommit
> whenever you'd like.
>
> -eric

So let's just reland this.
Original commit message:


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,
here 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}, this is mostly within the noise,
there are some small improvements, some small regressions.
One big regression i saw i fixed in 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=e40315d2b4ed1e38962a8f33ff151693ed4ada63&to=c8289c0ecbf235da9fb0e3bc052e3c0d6bff5cf9&stat=instructions
* this does regress compile-time by +0.5% geomean (unsurprizingly)
* size impact varies; for ThinLTO it's actually an improvement

The largest fallout appears to be in GVN's load partial redundancy
elimination, it spends *much* more time in
`MemoryDependenceResults::getNonLocalPointerDependency()`.
Non-local `MemoryDependenceResults` is widely-known to be, uh, costly.
There does not appear to be a proper solution to this issue,
other than silencing the compile-time performance regression
by tuning cut-off thresholds in `MemoryDependenceResults`,
at the cost of potentially regressing run-time performance.
D84609 attempts to move in that direction, but the path is unclear
and is going to take some time.

If we look at stats before/after diffs, some excerpts:
* RawSpeed (the target) {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}
* -36396 (-65.29%) common instructions hoisted
* +1676 (+0.02%) x86 asm instructions
* +662 (+0.06%) basic blocks
* +4395 (+0.04%) IR instructions

It is likely to be sub-optimal for when optimizing for code size,
so one might want to change tune pipeline by enabling sinking/hoisting
when optimizing for size.

Reviewed By: mkazantsev

Differential Revision: https://reviews.llvm.org/D84108

This reverts commit 503deec2183d466dad64b763bab4e15fd8804239.

show more ...

Temporairly revert "[SimplifyCFG][LoopRotate] SimplifyCFG: disable common instruction hoisting by default, enable late in pipeline"

As disscussed in post-commit review starting with
https://reviews

Temporairly revert "[SimplifyCFG][LoopRotate] SimplifyCFG: disable common instruction hoisting by default, enable late in pipeline"

As disscussed in post-commit review starting with
https://reviews.llvm.org/D84108#2227365
while this appears to be mostly a win overall, especially code-size-wise,
this appears to shake //certain// code pattens in a way that is extremely
unfavorable for performance (+30% runtime regression)
on certain CPU's (i personally can't reproduce).

So until the behaviour is better understood, and a path forward is mapped,
let's back this out for now.

This reverts commit 1d51dc38d89bd33fb8874e242ab87b265b4dec1c.

show more ...

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

I've been looking at missed vectorizations in one codebase.
One particular thing that s

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

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,
here 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}, this is mostly within the noise,
there are some small improvements, some small regressions.
One big regression i saw i fixed in 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=e40315d2b4ed1e38962a8f33ff151693ed4ada63&to=c8289c0ecbf235da9fb0e3bc052e3c0d6bff5cf9&stat=instructions
* this does regress compile-time by +0.5% geomean (unsurprizingly)
* size impact varies; for ThinLTO it's actually an improvement

The largest fallout appears to be in GVN's load partial redundancy
elimination, it spends *much* more time in
`MemoryDependenceResults::getNonLocalPointerDependency()`.
Non-local `MemoryDependenceResults` is widely-known to be, uh, costly.
There does not appear to be a proper solution to this issue,
other than silencing the compile-time performance regression
by tuning cut-off thresholds in `MemoryDependenceResults`,
at the cost of potentially regressing run-time performance.
D84609 attempts to move in that direction, but the path is unclear
and is going to take some time.

If we look at stats before/after diffs, some excerpts:
* RawSpeed (the target) {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}
* -36396 (-65.29%) common instructions hoisted
* +1676 (+0.02%) x86 asm instructions
* +662 (+0.06%) basic blocks
* +4395 (+0.04%) IR instructions

It is likely to be sub-optimal for when optimizing for code size,
so one might want to change tune pipeline by enabling sinking/hoisting
when optimizing for size.

Reviewed By: mkazantsev

Differential Revision: https://reviews.llvm.org/D84108

show more ...

[NFC][PhaseOrdering] Add a test demonstrating pitfails of common code hoisting on loop rotation

Depending on the -rotation-max-header-size=?,
hoisting common code early makes loop rotation impossibl

[NFC][PhaseOrdering] Add a test demonstrating pitfails of common code hoisting on loop rotation

Depending on the -rotation-max-header-size=?,
hoisting common code early makes loop rotation impossible.

show more ...