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. 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. Segher
