Re: [0/3] Fix PR78120, in ifcvt/rtlanal/i386.

On 11/28/2016 07:50 PM, Jeff Law wrote: So how would you suggest this be fixed right now? I'd really like to get the regression addressed. The regression is still fixed. That wasn't the case at all stages while I was working on it, but the i386 patch seems to suffice now. I would claim

Re: [0/3] Fix PR78120, in ifcvt/rtlanal/i386.

On 11/24/2016 09:14 AM, Segher Boessenkool wrote: On Thu, Nov 24, 2016 at 08:48:04AM -0700, Jeff Law wrote: On 11/24/2016 07:53 AM, Segher Boessenkool wrote: 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

Re: [0/3] Fix PR78120, in ifcvt/rtlanal/i386.

On 11/25/2016 04:55 PM, Segher Boessenkool wrote: So IMNSHO this rtx costing change belongs in early stage 1, and should be reverted. Done. Bernd

Re: [0/3] Fix PR78120, in ifcvt/rtlanal/i386.

On 11/25/2016 04:55 PM, Segher Boessenkool wrote: So IMNSHO this rtx costing change belongs in early stage 1, and should be reverted. Done. Bernd

Re: [0/3] Fix PR78120, in ifcvt/rtlanal/i386.

On Sat, Nov 26, 2016 at 09:15:48AM -0700, Jeff Law wrote: > On 11/26/2016 04:11 AM, Eric Botcazou wrote: > >> From my investigations on the m68k, the effects on the IL are minimal > >>with a slight bias towards better code (by suppressing if-conversions of > >>some now more costly blocks). *But*

Re: [0/3] Fix PR78120, in ifcvt/rtlanal/i386.

On Sat, Nov 26, 2016 at 03:44:22AM -0700, Jeff Law wrote: > On 11/24/2016 03:32 PM, Segher Boessenkool wrote: > >On Thu, Nov 24, 2016 at 10:14:24AM -0600, Segher Boessenkool wrote: > >>On Thu, Nov 24, 2016 at 08:48:04AM -0700, Jeff Law wrote: > >>>On 11/24/2016 07:53 AM, Segher Boessenkool wrote:

Re: [0/3] Fix PR78120, in ifcvt/rtlanal/i386.

On 11/26/2016 04:11 AM, Eric Botcazou wrote: From my investigations on the m68k, the effects on the IL are minimal with a slight bias towards better code (by suppressing if-conversions of some now more costly blocks). *But* the size of the resulting code was all over the place -- sometimes it

Re: [0/3] Fix PR78120, in ifcvt/rtlanal/i386.

> From my investigations on the m68k, the effects on the IL are minimal > with a slight bias towards better code (by suppressing if-conversions of > some now more costly blocks). *But* the size of the resulting code was > all over the place -- sometimes it was better, others worse. From >

Re: [0/3] Fix PR78120, in ifcvt/rtlanal/i386.

On 11/24/2016 03:32 PM, Segher Boessenkool wrote: On Thu, Nov 24, 2016 at 10:14:24AM -0600, Segher Boessenkool wrote: On Thu, Nov 24, 2016 at 08:48:04AM -0700, Jeff Law wrote: On 11/24/2016 07:53 AM, Segher Boessenkool wrote: That we compare different kinds of costs (which really has no

Re: [0/3] Fix PR78120, in ifcvt/rtlanal/i386.

On Fri, Nov 25, 2016 at 10:15:25AM +0100, Richard Biener wrote: > On Thu, Nov 24, 2016 at 5:14 PM, Segher Boessenkool > wrote: > > On Thu, Nov 24, 2016 at 08:48:04AM -0700, Jeff Law wrote: > >> On 11/24/2016 07:53 AM, Segher Boessenkool wrote: > >> > > >> >That we

Re: [0/3] Fix PR78120, in ifcvt/rtlanal/i386.

On 11/25/2016 02:15 AM, Richard Biener wrote: That's a good question ;) The stage 3 definition has a loophole via "go file a bug about feature X, then it's a bugfix!". Right. That loophole has existed since we've moved to the current model -- we extend a level of trust to our developers not

Re: [0/3] Fix PR78120, in ifcvt/rtlanal/i386.

On Thu, Nov 24, 2016 at 5:14 PM, Segher Boessenkool wrote: > On Thu, Nov 24, 2016 at 08:48:04AM -0700, Jeff Law wrote: >> On 11/24/2016 07:53 AM, Segher Boessenkool wrote: >> > >> >That we compare different kinds of costs (which really has no meaning at >> >all, it's a

Re: [0/3] Fix PR78120, in ifcvt/rtlanal/i386.

On Thu, Nov 24, 2016 at 10:14:24AM -0600, Segher Boessenkool wrote: > On Thu, Nov 24, 2016 at 08:48:04AM -0700, Jeff Law wrote: > > On 11/24/2016 07:53 AM, Segher Boessenkool wrote: > > > > > >That we compare different kinds of costs (which really has no meaning at > > >all, it's a heuristic at

Re: [0/3] Fix PR78120, in ifcvt/rtlanal/i386.

On Thu, Nov 24, 2016 at 08:48:04AM -0700, Jeff Law wrote: > On 11/24/2016 07:53 AM, Segher Boessenkool wrote: > > > >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 the

Re: [0/3] Fix PR78120, in ifcvt/rtlanal/i386.

On 11/24/2016 07:53 AM, Segher Boessenkool wrote: 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 the problems with the costing system exhibit themselves as a code quality

Re: [0/3] Fix PR78120, in ifcvt/rtlanal/i386.

On 11/24/2016 08:16 AM, Richard Biener wrote: 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,

Re: [0/3] Fix PR78120, in ifcvt/rtlanal/i386.

On 11/24/2016 03:53 PM, Segher Boessenkool wrote: 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? When I realized i386 was broken I had a look at aarch64 and it looked sane, and

Re: [0/3] Fix PR78120, in ifcvt/rtlanal/i386.

On Thu, Nov 24, 2016 at 3:53 PM, Segher Boessenkool 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

Re: [0/3] Fix PR78120, in ifcvt/rtlanal/i386.

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

Re: [0/3] Fix PR78120, in ifcvt/rtlanal/i386.

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

Re: [0/3] Fix PR78120, in ifcvt/rtlanal/i386.

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

Re: [0/3] Fix PR78120, in ifcvt/rtlanal/i386.

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.

Re: [0/3] Fix PR78120, in ifcvt/rtlanal/i386.

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

Re: [0/3] Fix PR78120, in ifcvt/rtlanal/i386.

[ Only your 0/3 and 3/3 messages arrived -- or is this 1/3? ] On Wed, Nov 23, 2016 at 08:00:30PM +0100, Bernd Schmidt wrote: > Note that I misspelled the PR number in the 0/3 message :-/ > > On 11/23/2016 07:57 PM, Bernd Schmidt wrote: > >1. I noticed comparisons between set_src_cost and

Re: [0/3] Fix PR78120, in ifcvt/rtlanal/i386.

On Wed, Nov 23, 2016 at 8:01 PM, Bernd Schmidt wrote: > On 11/23/2016 07:57 PM, Bernd Schmidt wrote: >> >> 2. The i386 backend mishandles SET rtxs. If you have a fairly plain >> single-insn SET, you tend to get COSTS_N_INSNS (2) out of set_rtx_cost, >> because rtx_costs has a

Re: [0/3] Fix PR78120, in ifcvt/rtlanal/i386.

On 11/23/2016 08:30 PM, Jeff Law wrote: On 11/23/2016 12:00 PM, Bernd Schmidt wrote: Note that I misspelled the PR number in the 0/3 message :-/ On 11/23/2016 07:57 PM, Bernd Schmidt wrote: 1. I noticed comparisons between set_src_cost and set_rtx_cost seemed to be invalid. There seems to be

Re: [0/3] Fix PR78120, in ifcvt/rtlanal/i386.

On 11/23/2016 12:00 PM, Bernd Schmidt wrote: Note that I misspelled the PR number in the 0/3 message :-/ On 11/23/2016 07:57 PM, Bernd Schmidt wrote: 1. I noticed comparisons between set_src_cost and set_rtx_cost seemed to be invalid. There seems to be no good reason that insn_rtx_cost

Re: [0/3] Fix PR78120, in ifcvt/rtlanal/i386.

On 11/23/2016 07:57 PM, Bernd Schmidt wrote: 2. The i386 backend mishandles SET rtxs. If you have a fairly plain single-insn SET, you tend to get COSTS_N_INSNS (2) out of set_rtx_cost, because rtx_costs has a default of COSTS_N_INSNS (1) for a SET, and you get the cost of the src in addition to

Re: [0/3] Fix PR78120, in ifcvt/rtlanal/i386.

Note that I misspelled the PR number in the 0/3 message :-/ On 11/23/2016 07:57 PM, Bernd Schmidt wrote: 1. I noticed comparisons between set_src_cost and set_rtx_cost seemed to be invalid. There seems to be no good reason that insn_rtx_cost shouldn't use the latter. It also makes the numbers