On 01/12/2017 07:55 AM, Richard Biener wrote:
The following fixes PR77283, path splitting being overly aggressive
and causing loop unrolling not to happen (because how it distorts the
CFG).
It is a aim at creating a cost model (there's none apart from
not duplicating too much stmts) by means of the observation that
we'd have to have PHI nodes in the joiner to have any possibility
of CSE opportunities being exposed by duplicating it or threading
opportunities being exposed across the new latch. That includes
virtual PHIs for CSE (so any load/store) but not for the threading
(but IMHO threading should figure all this out on its own without
the requirement for somebody else duplicating the joiner).
Bootstrapped and tested on x86_64-unknown-linux-gnu, the s390x
libquantum regression is reportedly fixed by this. I had to adjust
gcc.dg/tree-ssa/split-path-7.c to not expect any path splitting because
I (and of course the cost model) can't see any reason to do it.
Ok for trunk?
Thanks,
Richard.
2017-01-12 Richard Biener <rguent...@suse.de>
PR tree-optimization/77283
* gimple-ssa-split-paths.c: Include gimple-ssa.h, tree-phinodes.h
and ssa-iterators.h.
(is_feasible_trace): Implement a cost model based on joiner
PHI node uses.
* gcc.dg/tree-ssa/split-path-7.c: Adjust.
* gcc.dg/tree-ssa/split-path-8.c: New testcase.
* gcc.dg/tree-ssa/split-path-9.c: Likewise.
So I think the only concern is split-path-7. My memory is hazy, but I
suspect split-path-7 shows the case where path splitting's CFG
manipulations can result in fewer jumps for diamond sub-graphs. You
might see assembly code improvements due to path splitting on this test
for the microblaze port.
Certainly the code in gimple-ssa-split-paths.c that you're adding is an
improvement and brings gimple path splitting closer to its intended
purpose. I don't think regressing split-path-7 should block this
improvement, but we would want a PR to track the code quality regression.
So I think it's OK for the trunk and if it shows a code quality
regression for split-path-7 on the microblaze port that we should have a
distinct PR to track that issue (which is probably best solved in
bb-reorder).
Thanks,
Jeff
