Re: Recent combine change causing regressions
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
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
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
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
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
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
Recent combine change causing regressions
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 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? Jeff
