On 11/21/18 7:13 AM, Richard Biener wrote: > On Wed, Nov 21, 2018 at 1:12 AM Jeff Law <l...@redhat.com> wrote: >> >> On 11/20/18 6:42 AM, Michael Matz wrote: >>> Hi, >>> >>> this bug report is about cris generating worse code since tree-ssa. The >>> effect is also visible on x86-64. The symptom is that the work horse of >>> adler32.c (from zlib) needs spills in the inner loop, while gcc 3 did not, >>> and those spills go away with -fno-tree-reassoc. >>> >>> The underlying reason for the spills is register pressure, which could >>> either be rectified by the pressure aware scheduling (which cris doesn't >>> have), or by simply not generating high pressure code to begin with. In >>> this case it's TER which ultimately causes the register pressure to >>> increase, and there are many plans in people minds how to fix this (make >>> TER regpressure aware, do some regpressure scheduling on gimple, or even >>> more ambitious things), but this patch doesn't tackle this. Instead it >>> makes reassoc not generate the situation which causes TER to run wild. >>> >>> TER increasing register pressure is a long standing problem and so it has >>> some heuristics to avoid that. One wobbly heuristic is that it doesn't >>> TER expressions together that have the same base variable as their LHSs. >>> But reassoc generates only anonymous SSA names for its temporary >>> subexpressions, so that TER heuristic doesn't apply. In this testcase >>> it's even the case that reassoc doesn't really change much code (one >>> addition moves from the end to the beginning of the inner loop), so that >>> whole rewriting is even pointless. >>> >>> In any case, let's use copy_ssa_name instead of make_ssa_name, when we >>> have an obvious LHS; that leads to TER making the same decisions with and >>> without -fno-tree-reassoc, leading to the same register pressure and no >>> spills. > > I don't think this is OK. Take one example, in rewrite_expr_tree for the > final > recursion case we replace > > a_1 = _2 + _3; > > with sth else, like > > _4 = _5 + 1; > > so we compute a new value that may not have been computed before and > certainly not into the user variable a. If you change this to instead create > > a_4 = _5 + 1; > > then this leads to wrong debug info showing a value for 'a' that never > existed. > You can observe this with But isn't the point to use an underlying SSA_NAME_VAR when we have one that should be appropriate? Are we just being too aggressive with using copy_ssa_name?
jeff