| Message ID | 20160503105601.GB9646@embecosm.com |
|---|---|
| State | New |
| Headers | show |
On 03/05/16 12:56, Andrew Burgess wrote: > * Claudiu Zissulescu <Claudiu.Zissulescu@synopsys.com> [2016-05-02 09:02:16 +0000]: > >> Please also consider to address also the following warnings introduced: >> >> mainline/gcc/gcc/config/arc/arc.md:888: warning: source missing a mode? >> mainline/gcc/gcc/config/arc/arc.md:906: warning: source missing a mode? >> mainline/gcc/gcc/config/arc/arc.md:921: warning: source missing a mode? >> mainline/gcc/gcc/config/arc/arc.md:6146: warning: source missing a mode? >> > > Here's a revised fixup patch that includes addressing these 4 > warnings. > > Thanks, > Andrew > > --- > > gcc/arc: New peephole2 and little endian arc test fixes > > Resolve some test failures introduced for little endian arc as a result > of the recent arc/nps400 additions. > > There's a new peephole2 optimisation to merge together two zero_extracts > in order that the movb instruction can be used. > > Source operand modes filled in for 3 instruction patterns and a > peephole2 optimisation, to silence build warnings. > > One of the test cases is extended so that the test does something > meaningful in both big and little endian arc mode. > > Other tests have their expected results updated to reflect improvements > in other areas of GCC. > > gcc/ChangeLog: > > * config/arc/arc.md (movb peephole2): New peephole2 to merge two > zero_extract operations to allow a movb to occur. > (*tst_bitfield_tst): Add mode to source operand. > (*tst_bitfield_asr): Likewise. > (*tst_bitfield): Likewise. > (bitops peephole2): Likewise. > > gcc/testsuite/ChangeLog: > > * gcc.target/arc/movb-1.c: Update little endian arc results. > * gcc.target/arc/movb-2.c: Likewise. > * gcc.target/arc/movb-5.c: Likewise. > * gcc.target/arc/movh_cl-1.c: Extend test to cover little endian > arc. > --- > gcc/ChangeLog | 9 +++++++++ > gcc/config/arc/arc.md | 22 ++++++++++++++++++---- > gcc/testsuite/ChangeLog | 8 ++++++++ > gcc/testsuite/gcc.target/arc/movb-1.c | 2 +- > gcc/testsuite/gcc.target/arc/movb-2.c | 2 +- > gcc/testsuite/gcc.target/arc/movb-5.c | 2 +- > gcc/testsuite/gcc.target/arc/movh_cl-1.c | 11 +++++++++++ > 7 files changed, 49 insertions(+), 7 deletions(-) > > diff --git a/gcc/config/arc/arc.md b/gcc/config/arc/arc.md > index c61107f..96c1e77 100644 > --- a/gcc/config/arc/arc.md > +++ b/gcc/config/arc/arc.md > @@ -879,7 +879,7 @@ > ; since this is about constants, reload shouldn't care. > (define_insn "*tst_bitfield_tst" > [(set (match_operand:CC_ZN 0 "cc_set_register" "") > - (match_operator 4 "zn_compare_operator" > + (match_operator:CC_ZN 4 "zn_compare_operator" > [(zero_extract:SI > (match_operand:SI 1 "register_operand" "c") > (match_operand:SI 2 "const_int_operand" "n") > @@ -897,7 +897,7 @@ > ; Likewise for asr.f. > (define_insn "*tst_bitfield_asr" > [(set (match_operand:CC_ZN 0 "cc_set_register" "") > - (match_operator 4 "zn_compare_operator" > + (match_operator:CC_ZN 4 "zn_compare_operator" > [(zero_extract:SI > (match_operand:SI 1 "register_operand" "c") > (match_operand:SI 2 "const_int_operand" "n") > @@ -912,7 +912,7 @@ > > (define_insn "*tst_bitfield" > [(set (match_operand:CC_ZN 0 "cc_set_register" "") > - (match_operator 5 "zn_compare_operator" > + (match_operator:CC_ZN 5 "zn_compare_operator" > [(zero_extract:SI > (match_operand:SI 1 "register_operand" "%Rcqq,c, c,Rrq,c") > (match_operand:SI 2 "const_int_operand" "N,N, n,Cbn,n") > @@ -6128,7 +6128,7 @@ > (zero_extract:SI (match_dup 1) > (match_dup 2) > (match_operand:SI 4 "const_int_operand" ""))) > - (set (match_dup 1) (match_operand 8)) > + (set (match_dup 1) (match_operand:SI 8)) > (set (zero_extract:SI (match_dup 0) > (match_operand:SI 5 "const_int_operand" "") > (match_operand:SI 6 "const_int_operand" "")) > @@ -6144,6 +6144,20 @@ > (zero_extract:SI (match_dup 1) (match_dup 5) (match_dup 7)))]) > (match_dup 1)]) > > +(define_peephole2 > + [(set (match_operand:SI 0 "register_operand" "") > + (zero_extract:SI (match_dup 0) > + (match_operand:SI 1 "const_int_operand" "") > + (match_operand:SI 2 "const_int_operand" ""))) > + (set (zero_extract:SI (match_operand:SI 3 "register_operand" "") > + (match_dup 1) > + (match_dup 2)) > + (match_dup 0))] > + "TARGET_NPS_BITOPS > + && !reg_overlap_mentioned_p (operands[0], operands[3])" > + [(set (zero_extract:SI (match_dup 3) (match_dup 1) (match_dup 2)) > + (zero_extract:SI (match_dup 0) (match_dup 1) (match_dup 2)))]) > + > ;; include the arc-FPX instructions > (include "fpx.md") > > diff --git a/gcc/testsuite/gcc.target/arc/movb-1.c b/gcc/testsuite/gcc.target/arc/movb-1.c > index 65d4ba4..94d9f5f 100644 > --- a/gcc/testsuite/gcc.target/arc/movb-1.c > +++ b/gcc/testsuite/gcc.target/arc/movb-1.c > @@ -10,4 +10,4 @@ f (void) > bar.b = foo.b; > } > /* { dg-final { scan-assembler "movb\[ \t\]+r\[0-5\]+, *r\[0-5\]+, *r\[0-5\]+, *5, *3, *8" { target arceb-*-* } } } */ > -/* { dg-final { scan-assembler "movb\[ \t\]+r\[0-5\]+, *r\[0-5\]+, *r\[0-5\]+, *19, *21, *8" { target arc-*-* } } } */ > +/* { dg-final { scan-assembler "movb\[ \t\]+r\[0-5\]+, *r\[0-5\]+, *r\[0-5\]+, *3, *5, *8" { target arc-*-* } } } */ > diff --git a/gcc/testsuite/gcc.target/arc/movb-2.c b/gcc/testsuite/gcc.target/arc/movb-2.c > index 1ba9976..708f393 100644 > --- a/gcc/testsuite/gcc.target/arc/movb-2.c > +++ b/gcc/testsuite/gcc.target/arc/movb-2.c > @@ -9,5 +9,5 @@ f (void) > { > bar.b = foo.b; > } > -/* { dg-final { scan-assembler "movb\[ \t\]+r\[0-5\]+, *r\[0-5\]+, *r\[0-5\]+, *23, *23, *9" { target arc-*-* } } } */ > +/* { dg-final { scan-assembler "movb\[ \t\]+r\[0-5\]+, *r\[0-5\]+, *r\[0-5\]+, *7, *7, *9" { target arc-*-* } } } */ > /* { dg-final { scan-assembler "movb\[ \t\]+r\[0-5\]+, *r\[0-5\]+, *r\[0-5\]+, *0, *0, *9" { target arceb-*-* } } } */ > diff --git a/gcc/testsuite/gcc.target/arc/movb-5.c b/gcc/testsuite/gcc.target/arc/movb-5.c > index 9dbe8a1..d285888 100644 > --- a/gcc/testsuite/gcc.target/arc/movb-5.c > +++ b/gcc/testsuite/gcc.target/arc/movb-5.c > @@ -9,5 +9,5 @@ f (void) > { > bar.b = foo.b; > } > -/* { dg-final { scan-assembler "movb\[ \t\]+r\[0-5\]+, *r\[0-5\]+, *r\[0-5\]+, *23, *(23|7), *9" { target arc-*-* } } } */ > +/* { dg-final { scan-assembler "movb\[ \t\]+r\[0-5\]+, *r\[0-5\]+, *r\[0-5\]+, *7, *7, *9" { target arc-*-* } } } */ > /* { dg-final { scan-assembler "movb\[ \t\]+r\[0-5\]+, *r\[0-5\]+, *r\[0-5\]+, *0, *0, *9" { target arceb-*-* } } } */ > diff --git a/gcc/testsuite/gcc.target/arc/movh_cl-1.c b/gcc/testsuite/gcc.target/arc/movh_cl-1.c > index 220cd9d..c643481 100644 > --- a/gcc/testsuite/gcc.target/arc/movh_cl-1.c > +++ b/gcc/testsuite/gcc.target/arc/movh_cl-1.c > @@ -10,6 +10,9 @@ struct thing > { > unsigned a : 1; > unsigned b : 1; > + unsigned c : 28; > + unsigned d : 1; > + unsigned e : 1; > }; > }; > }; > @@ -24,4 +27,12 @@ blah () > func (xx.raw); > } > > +void > +woof () > +{ > + struct thing xx; > + xx.d = xx.e = 1; > + func (xx.raw); > +} > + > /* { dg-final { scan-assembler "movh\.cl r\[0-9\]+,0xc0000000>>16" } } */ > It seems alright to me, but you need to get Joern approval on this, Claudiu
On 03/05/16 11:56, Andrew Burgess wrote: > * Claudiu Zissulescu <Claudiu.Zissulescu@synopsys.com> [2016-05-02 09:02:16 +0000]: > >> Please also consider to address also the following warnings introduced: >> >> mainline/gcc/gcc/config/arc/arc.md:888: warning: source missing a mode? >> mainline/gcc/gcc/config/arc/arc.md:906: warning: source missing a mode? >> mainline/gcc/gcc/config/arc/arc.md:921: warning: source missing a mode? >> mainline/gcc/gcc/config/arc/arc.md:6146: warning: source missing a mode? >> > Here's a revised fixup patch that includes addressing these 4 > warnings. This the the wrong approach. zn_compare_operator is a special predicate that accepts CC_ZNmode as well as CC_Zmode. The point is that some comparison uses need Z and N set, while other are OK with just Z. Not accepting CC_Zmode would cause spurious recognition rejections. A generator program complaining about a missing mode for an operand with a special predicate is wrong. from md.texi: : Predicates written with @code{define_special_predicate} do not get any : automatic mode checks, and are treated as having special mode handling : by @command{genrecog}.
Joern, Thanks for taking the time to review the previous patch. I've revised the original fix-up patch to remove the addition of the MODE in the zn_compare_operator case as you suggest, this is patch #1. Patch #2 is an attempt to address you comment: A generator program complaining about a missing mode for an operand with a special predicate is wrong. I'm hoping you'll review (and if you like it) merge patch #1. I'd appreciate you feedback on patch #2. I can always repost patch #2 as a top-level post to the mailing list if you think this is the right fix, but should get a wider audience. Thanks, Andrew --- Andrew Burgess (2): gcc/arc: New peephole2 and little endian arc test fixes gcc/genrecog: Don't warn for missing mode on special predicates gcc/ChangeLog | 11 +++++++++++ gcc/config/arc/arc.md | 16 +++++++++++++++- gcc/genrecog.c | 22 +++++++++++++++++++--- gcc/testsuite/ChangeLog | 8 ++++++++ gcc/testsuite/gcc.target/arc/movb-1.c | 2 +- gcc/testsuite/gcc.target/arc/movb-2.c | 2 +- gcc/testsuite/gcc.target/arc/movb-5.c | 2 +- gcc/testsuite/gcc.target/arc/movh_cl-1.c | 11 +++++++++++ 8 files changed, 67 insertions(+), 7 deletions(-)
diff --git a/gcc/config/arc/arc.md b/gcc/config/arc/arc.md index c61107f..96c1e77 100644 --- a/gcc/config/arc/arc.md +++ b/gcc/config/arc/arc.md @@ -879,7 +879,7 @@ ; since this is about constants, reload shouldn't care. (define_insn "*tst_bitfield_tst" [(set (match_operand:CC_ZN 0 "cc_set_register" "") - (match_operator 4 "zn_compare_operator" + (match_operator:CC_ZN 4 "zn_compare_operator" [(zero_extract:SI (match_operand:SI 1 "register_operand" "c") (match_operand:SI 2 "const_int_operand" "n") @@ -897,7 +897,7 @@ ; Likewise for asr.f. (define_insn "*tst_bitfield_asr" [(set (match_operand:CC_ZN 0 "cc_set_register" "") - (match_operator 4 "zn_compare_operator" + (match_operator:CC_ZN 4 "zn_compare_operator" [(zero_extract:SI (match_operand:SI 1 "register_operand" "c") (match_operand:SI 2 "const_int_operand" "n") @@ -912,7 +912,7 @@ (define_insn "*tst_bitfield" [(set (match_operand:CC_ZN 0 "cc_set_register" "") - (match_operator 5 "zn_compare_operator" + (match_operator:CC_ZN 5 "zn_compare_operator" [(zero_extract:SI (match_operand:SI 1 "register_operand" "%Rcqq,c, c,Rrq,c") (match_operand:SI 2 "const_int_operand" "N,N, n,Cbn,n") @@ -6128,7 +6128,7 @@ (zero_extract:SI (match_dup 1) (match_dup 2) (match_operand:SI 4 "const_int_operand" ""))) - (set (match_dup 1) (match_operand 8)) + (set (match_dup 1) (match_operand:SI 8)) (set (zero_extract:SI (match_dup 0) (match_operand:SI 5 "const_int_operand" "") (match_operand:SI 6 "const_int_operand" "")) @@ -6144,6 +6144,20 @@ (zero_extract:SI (match_dup 1) (match_dup 5) (match_dup 7)))]) (match_dup 1)]) +(define_peephole2 + [(set (match_operand:SI 0 "register_operand" "") + (zero_extract:SI (match_dup 0) + (match_operand:SI 1 "const_int_operand" "") + (match_operand:SI 2 "const_int_operand" ""))) + (set (zero_extract:SI (match_operand:SI 3 "register_operand" "") + (match_dup 1) + (match_dup 2)) + (match_dup 0))] + "TARGET_NPS_BITOPS + && !reg_overlap_mentioned_p (operands[0], operands[3])" + [(set (zero_extract:SI (match_dup 3) (match_dup 1) (match_dup 2)) + (zero_extract:SI (match_dup 0) (match_dup 1) (match_dup 2)))]) + ;; include the arc-FPX instructions (include "fpx.md") diff --git a/gcc/testsuite/gcc.target/arc/movb-1.c b/gcc/testsuite/gcc.target/arc/movb-1.c index 65d4ba4..94d9f5f 100644 --- a/gcc/testsuite/gcc.target/arc/movb-1.c +++ b/gcc/testsuite/gcc.target/arc/movb-1.c @@ -10,4 +10,4 @@ f (void) bar.b = foo.b; } /* { dg-final { scan-assembler "movb\[ \t\]+r\[0-5\]+, *r\[0-5\]+, *r\[0-5\]+, *5, *3, *8" { target arceb-*-* } } } */ -/* { dg-final { scan-assembler "movb\[ \t\]+r\[0-5\]+, *r\[0-5\]+, *r\[0-5\]+, *19, *21, *8" { target arc-*-* } } } */ +/* { dg-final { scan-assembler "movb\[ \t\]+r\[0-5\]+, *r\[0-5\]+, *r\[0-5\]+, *3, *5, *8" { target arc-*-* } } } */ diff --git a/gcc/testsuite/gcc.target/arc/movb-2.c b/gcc/testsuite/gcc.target/arc/movb-2.c index 1ba9976..708f393 100644 --- a/gcc/testsuite/gcc.target/arc/movb-2.c +++ b/gcc/testsuite/gcc.target/arc/movb-2.c @@ -9,5 +9,5 @@ f (void) { bar.b = foo.b; } -/* { dg-final { scan-assembler "movb\[ \t\]+r\[0-5\]+, *r\[0-5\]+, *r\[0-5\]+, *23, *23, *9" { target arc-*-* } } } */ +/* { dg-final { scan-assembler "movb\[ \t\]+r\[0-5\]+, *r\[0-5\]+, *r\[0-5\]+, *7, *7, *9" { target arc-*-* } } } */ /* { dg-final { scan-assembler "movb\[ \t\]+r\[0-5\]+, *r\[0-5\]+, *r\[0-5\]+, *0, *0, *9" { target arceb-*-* } } } */ diff --git a/gcc/testsuite/gcc.target/arc/movb-5.c b/gcc/testsuite/gcc.target/arc/movb-5.c index 9dbe8a1..d285888 100644 --- a/gcc/testsuite/gcc.target/arc/movb-5.c +++ b/gcc/testsuite/gcc.target/arc/movb-5.c @@ -9,5 +9,5 @@ f (void) { bar.b = foo.b; } -/* { dg-final { scan-assembler "movb\[ \t\]+r\[0-5\]+, *r\[0-5\]+, *r\[0-5\]+, *23, *(23|7), *9" { target arc-*-* } } } */ +/* { dg-final { scan-assembler "movb\[ \t\]+r\[0-5\]+, *r\[0-5\]+, *r\[0-5\]+, *7, *7, *9" { target arc-*-* } } } */ /* { dg-final { scan-assembler "movb\[ \t\]+r\[0-5\]+, *r\[0-5\]+, *r\[0-5\]+, *0, *0, *9" { target arceb-*-* } } } */ diff --git a/gcc/testsuite/gcc.target/arc/movh_cl-1.c b/gcc/testsuite/gcc.target/arc/movh_cl-1.c index 220cd9d..c643481 100644 --- a/gcc/testsuite/gcc.target/arc/movh_cl-1.c +++ b/gcc/testsuite/gcc.target/arc/movh_cl-1.c @@ -10,6 +10,9 @@ struct thing { unsigned a : 1; unsigned b : 1; + unsigned c : 28; + unsigned d : 1; + unsigned e : 1; }; }; }; @@ -24,4 +27,12 @@ blah () func (xx.raw); } +void +woof () +{ + struct thing xx; + xx.d = xx.e = 1; + func (xx.raw); +} + /* { dg-final { scan-assembler "movh\.cl r\[0-9\]+,0xc0000000>>16" } } */