This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [0/3] Fix PR78120, in ifcvt/rtlanal/i386.
On Thu, Nov 24, 2016 at 3:53 PM, Segher Boessenkool
<segher@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