On Thu, 12 Jan 2017, Jeff Law wrote:
> 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).
Committed. With the wrong variant of the -9 test, fixed as below.
Richard.
2017-01-13 Richard Biener <rguent...@suse.de>
PR tree-optimization/77283
* gcc.dg/tree-ssa/split-path-9.c: Fix.
Index: gcc/testsuite/gcc.dg/tree-ssa/split-path-9.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/split-path-9.c (revision 244393)
+++ gcc/testsuite/gcc.dg/tree-ssa/split-path-9.c (working copy)
@@ -9,8 +9,8 @@ foo(unsigned int size, unsigned int *sta
for(i = 0; i < size; i++)
{
- if(*state & 1)
- *state ^= 1;
+ if(state[i] & 1)
+ state[i] ^= 1;
}
}
