Re: Recent combine change causing regressions

2019-05-13 Thread Segher Boessenkool
On Mon, May 13, 2019 at 09:38:00AM -0500, Segher Boessenkool wrote:
> I tested on 30-something targets (all *-linux), and only mips64 regressed
> a little, everything else improved.

I meant alpha, btw.  0.08% code size growth.  And the biggest winner is
i386, with a similar shrink.  Most targets shrink a tiny bit, only a few
stay the same size, and even fewer grow.


Segher


Re: Recent combine change causing regressions

2019-05-13 Thread Segher Boessenkool
On Mon, May 13, 2019 at 11:41:58PM +0900, Oleg Endo wrote:
> On Mon, 2019-05-13 at 09:38 -0500, Segher Boessenkool wrote:
> > On Mon, May 13, 2019 at 08:27:15AM -0600, Jeff Law wrote:
> > > Tests that now fail, but worked before (3 tests):
> > > 
> > > gcc.target/sh/pr51244-11.c scan-assembler-not subc|and|bra
> > > gcc.target/sh/pr51244-11.c scan-assembler-times bf\t0f 1
> > > gcc.target/sh/pr51244-11.c scan-assembler-times bt\t0f 1

> Hmmm .. on SH3 TARGET_ZDCBRANCH should be off, afair.

The test explicitly uses O1 -mzdcbranch .


Segher


Re: Recent combine change causing regressions

2019-05-13 Thread Segher Boessenkool
On Mon, May 13, 2019 at 08:27:15AM -0600, Jeff Law wrote:
> 
> sh3-linux-gnu and sh3eb-linux-gnu:
> 
> 
> Tests that now fail, but worked before (3 tests):
> 
> gcc.target/sh/pr51244-11.c scan-assembler-not subc|and|bra
> gcc.target/sh/pr51244-11.c scan-assembler-times bf\t0f 1
> gcc.target/sh/pr51244-11.c scan-assembler-times bt\t0f 1


--- pr51244-11.s.OK 2019-05-13 15:25:02.283142995 +
+++ pr51244-11.s.BAD2019-05-13 15:23:25.875758477 +
@@ -12,9 +12,9 @@
mov r4,r0
mov.l   @(12,r4),r1
tst r1,r1
-   bf  0f
-   mov #0,r0
-0:
+   subcr1,r1
+   not r1,r1
+   and r1,r0
 .L2:
rts 
nop
@@ -29,9 +29,8 @@
mov r4,r0
mov.l   @(12,r4),r1
tst r1,r1
-   bt  0f
-   mov #0,r0
-0:
+   subcr1,r1
+   and r1,r0
 .L4:
rts 
nop


is the whole diff, fwiw.


Segher


Re: Recent combine change causing regressions

2019-05-13 Thread Jeff Law
On 5/13/19 8:38 AM, Segher Boessenkool wrote:
> On Mon, May 13, 2019 at 08:27:15AM -0600, Jeff Law wrote:
>> sh3-linux-gnu and sh3eb-linux-gnu:
> 
> I test sh2 and sh4, but not sh3 :-)
> 
>> Tests that now fail, but worked before (3 tests):
>>
>> gcc.target/sh/pr51244-11.c scan-assembler-not subc|and|bra
>> gcc.target/sh/pr51244-11.c scan-assembler-times bf\t0f 1
>> gcc.target/sh/pr51244-11.c scan-assembler-times bt\t0f 1
>>
>> Previously we'd match this pattern:
>>
>> (define_insn "*cset_zero"
>>   [(set (match_operand:SI 0 "arith_reg_dest" "=r")
>> (if_then_else:SI (match_operand:SI 1 "cbranch_treg_value")
>>  (match_operand:SI 2 "arith_reg_operand" "0")
>>  (const_int 0)))]
>>   "TARGET_SH1 && TARGET_ZDCBRANCH"
>>
>> After your change we no longer try to do so.
>>
>> I really don't care about the SH port.  But isn't this really a symptom
>> of a larger problem.  Namely that by not generating if-then-else you've
>> hosed every target that implements conditional moves via if-then-else
>> constructs?
> 
> I tested on 30-something targets (all *-linux), and only mips64 regressed
> a little, everything else improved.  So the current tuning is better than
> what it was before.  No doubt it can be improved though!
> 
> This is only if-then-else for a single bit, fwiw.
So are other targets still generating conditional moves?  If so the fix
may ultimately be to rewrite that pattern in the SH backend.

> 
> I'll build some sh3-linux if I find a cycle or two.
Thanks.  It does reproduce with a cross. In fact, you just need the
compiler -- you don't need an assembler, binutils, headers, etc :-)

jeff


Re: Recent combine change causing regressions

2019-05-13 Thread Oleg Endo
On Mon, 2019-05-13 at 09:38 -0500, Segher Boessenkool wrote:
> On Mon, May 13, 2019 at 08:27:15AM -0600, Jeff Law wrote:
> > sh3-linux-gnu and sh3eb-linux-gnu:
> 
> I test sh2 and sh4, but not sh3 :-)
> 
> > Tests that now fail, but worked before (3 tests):
> > 
> > gcc.target/sh/pr51244-11.c scan-assembler-not subc|and|bra
> > gcc.target/sh/pr51244-11.c scan-assembler-times bf\t0f 1
> > gcc.target/sh/pr51244-11.c scan-assembler-times bt\t0f 1
> > 
> > Previously we'd match this pattern:
> > 
> > (define_insn "*cset_zero"
> >   [(set (match_operand:SI 0 "arith_reg_dest" "=r")
> > (if_then_else:SI (match_operand:SI 1 "cbranch_treg_value")
> >  (match_operand:SI 2 "arith_reg_operand"
> > "0")
> >  (const_int 0)))]
> >   "TARGET_SH1 && TARGET_ZDCBRANCH"
> > 
> > After your change we no longer try to do so.
> > 
> > I really don't care about the SH port.  But isn't this really a
> > symptom
> > of a larger problem.  Namely that by not generating if-then-else
> > you've
> > hosed every target that implements conditional moves via if-then-
> > else
> > constructs?
> 
> I tested on 30-something targets (all *-linux), and only mips64
> regressed
> a little, everything else improved.  So the current tuning is better
> than
> what it was before.  No doubt it can be improved though!
> 
> This is only if-then-else for a single bit, fwiw.
> 
> I'll build some sh3-linux if I find a cycle or two.
> 

Hmmm .. on SH3 TARGET_ZDCBRANCH should be off, afair.
What would be the alternative now for the if-then-else?

Cheers,
Oleg



Re: Recent combine change causing regressions

2019-05-13 Thread Segher Boessenkool
On Mon, May 13, 2019 at 08:27:15AM -0600, Jeff Law wrote:
> sh3-linux-gnu and sh3eb-linux-gnu:

I test sh2 and sh4, but not sh3 :-)

> Tests that now fail, but worked before (3 tests):
> 
> gcc.target/sh/pr51244-11.c scan-assembler-not subc|and|bra
> gcc.target/sh/pr51244-11.c scan-assembler-times bf\t0f 1
> gcc.target/sh/pr51244-11.c scan-assembler-times bt\t0f 1
> 
> Previously we'd match this pattern:
> 
> (define_insn "*cset_zero"
>   [(set (match_operand:SI 0 "arith_reg_dest" "=r")
> (if_then_else:SI (match_operand:SI 1 "cbranch_treg_value")
>  (match_operand:SI 2 "arith_reg_operand" "0")
>  (const_int 0)))]
>   "TARGET_SH1 && TARGET_ZDCBRANCH"
> 
> After your change we no longer try to do so.
> 
> I really don't care about the SH port.  But isn't this really a symptom
> of a larger problem.  Namely that by not generating if-then-else you've
> hosed every target that implements conditional moves via if-then-else
> constructs?

I tested on 30-something targets (all *-linux), and only mips64 regressed
a little, everything else improved.  So the current tuning is better than
what it was before.  No doubt it can be improved though!

This is only if-then-else for a single bit, fwiw.

I'll build some sh3-linux if I find a cycle or two.


Segher