On Thu, Nov 24, 2016 at 3:53 PM, Segher Boessenkool <seg...@kernel.crashing.org> wrote: > On Thu, Nov 24, 2016 at 03:38:55PM +0100, Bernd Schmidt wrote: >> On 11/24/2016 03:36 PM, Segher Boessenkool wrote: >> >On Thu, Nov 24, 2016 at 03:26:45PM +0100, Bernd Schmidt wrote: >> >>On 11/24/2016 03:21 PM, Segher Boessenkool wrote: >> >> >> >>>Combine uses insn_rtx_cost extensively. I have tried to change it to use >> >>>the full rtx cost, not just the source cost, a few times before, and it >> >>>always only regressed. Part of it is that most ports' cost calculations >> >>>are, erm, not so great -- every target we care about needs fixes. >> >>> >> >>>Let's please not try this in stage 3. >> >> >> >>It got approved and committed. Do you want me to revert it now or wait >> >>for obvious signs of fallout? >> > >> >In my opinion it is an early stage 1 thing, not something suitable for >> >stage 3. I can do some simple tests on various targets if you want. >> >> Sure. >> >> I'll make the argument that stage 3 is when we fix stuff, including >> performance regressions, and this patch is very clearly a fix. When we >> have very obvious distortions like a case where costs from insn_rtx_cost >> and seq_cost aren't comparable, it's impossible to arrive at sane solutions. > > Your own 2/3 shows my point: you needed fixes to the i386 port for it > to behave sanely after this 1/3; what makes you think other ports are > not in the same boat? > > IMHO switching insn_rtx_cost to be based on not just set_src_cost is > a good idea, but will require re-tuning of all targets, so it is not > stage 3 material.
Agreed. > That we compare different kinds of costs (which really has no meaning at > all, it's a heuristic at best) in various places is a known problem, not > a regression. But technically stage 3 is for general bugfixing, not only regression fixing. I'd say be prepared to revert but wait to see who screams first. Thanks, Richard. > > Segher