Re: [PATCHv2 0/7] ARC: Add support for nps400 variant

[Claudiu Zissulescu <claziss@gmail.com>]

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