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, 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:
- 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)
- -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
- -36396 (-65.29%) common instructions hoisted
- +1676 (+0.02%) x86 asm instructions
- +662 (+0.06%) basic blocks
- +4395 (+0.04%) IR instructions