This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH,SH] Add SH2A new instructions 5/6
- From: Kaz Kojima <kkojima at rr dot iij4u dot or dot jp>
- To: naveen dot hs at kpitcummins dot com
- Cc: gcc-patches at gcc dot gnu dot org, Anil dot Paranjape at kpitcummins dot com, Prafulla dot Thakare at kpitcummins dot com
- Date: Tue, 30 Oct 2007 08:48:02 +0900 (JST)
- Subject: Re: [PATCH,SH] Add SH2A new instructions 5/6
- References: <7B5265FF85A78C4BB9DAD0EE699C47600336E7B1@sohm.kpit.com>
"Naveen H.S." <naveen.hs@kpitcummins.com> wrote:
> Please find attached the modified patch "sh2a5.patch" as per the
> suggestions.
Looks fine, except for a few problems about coding standard.
Re-post the revised patch when the trunk returns to stage1.
Don't forget to describe how the patch is tested then.
> --- gcc-4.3-20070921/gcc/config/sh/constraints.md 2007-10-24 12:09:31.000000000 +0530
> +++ tars/gcc-4.3-20070921/gcc/config/sh/constraints.md 2007-10-24 12:18:12.000000000 +0530
[snip]
> @@ -204,6 +206,30 @@
> PIC_DIRECT_ADDR_P."
> (match_test "IS_NON_EXPLICIT_CONSTANT_P (op)"))
>
> +(define_constraint "Pso"
> + "Integer constant with a single bit set in its lower 8-bit."
> + (and (match_code "const_int")
> + (ior (match_test "ival == 1")
> + (match_test "ival == 2")
> + (match_test "ival == 4")
> + (match_test "ival == 8")
> + (match_test "ival == 16")
> + (match_test "ival == 32")
> + (match_test "ival == 64")
> + (match_test "ival == 128"))))
> +
> +(define_constraint "Psz"
> + "Integer constant with a single zero bit in the lower 8-bit."
> + (and (match_code "const_int")
> + (ior (match_test "~ival == 1")
> + (match_test "~ival == 2")
> + (match_test "~ival == 4")
> + (match_test "~ival == 8")
> + (match_test "~ival == 16")
> + (match_test "~ival == 32")
> + (match_test "~ival == 64")
> + (match_test "~ival == 128"))))
^^^^^^^^
Use tabs instead of 8 spaces.
> --- gcc-4.3-20070921/gcc/config/sh/sh.md 2007-10-24 12:18:00.000000000 +0530
> +++ tars/gcc-4.3-20070921/gcc/config/sh/sh.md 2007-10-24 12:18:12.000000000 +0530
> @@ -3164,6 +3164,14 @@ label:
> andi %1, %2, %0"
> [(set_attr "type" "arith_media")])
>
> +(define_insn "*andsi3_bclr"
> + [(set (match_operand:SI 0 "arith_reg_dest" "=r")
> + (and:SI (match_operand:SI 1 "arith_reg_operand" "%0")
> + (match_operand:SI 2 "const_int_operand" "Psz")))]
^^^^^^^^
Same here
> @@ -3246,6 +3254,14 @@ label:
> ori %1, %2, %0"
> [(set_attr "type" "arith_media")])
>
> +(define_insn "*iorsi3_bset"
> + [(set (match_operand:SI 0 "arith_reg_dest" "=r")
> + (ior:SI (match_operand:SI 1 "arith_reg_operand" "%0")
> + (match_operand:SI 2 "const_int_operand" "Pso")))]
^^^^^^^^
and here.
> --- gcc-4.3-20070921/gcc/testsuite/gcc.target/sh/sh2a-bclr.c 1970-01-01 05:30:00.000000000 +0530
> +++ tars/gcc-4.3-20070921/gcc/testsuite/gcc.target/sh/sh2a-bclr.c 2007-10-24 12:20:01.000000000 +0530
> @@ -0,0 +1,63 @@
> +/* Testcase to check generation of a SH2A specific instruction
> + "BCLR #imm3,Rn" */
Missing period.
> +/* { dg-skip-if "" { "sh*-*-*" } "*" "-m2a -m2a-nofpu -m2a-single
> + -m2a-single-only" } */
A single line would be safer.
> + volatile static struct a x = { 1, 66, ~1 }, y =
> + {
> + 1, 2, ~2};
Wrong indentation. Simply
volatile static struct a x = { 1, 66, ~1 }, y = { 1, 2, ~2};
would be Ok.
> --- gcc-4.3-20070921/gcc/testsuite/gcc.target/sh/sh2a-bset.c 1970-01-01 05:30:00.000000000 +0530
> +++ tars/gcc-4.3-20070921/gcc/testsuite/gcc.target/sh/sh2a-bset.c 2007-10-24 12:20:50.000000000 +0530
> @@ -0,0 +1,62 @@
> +/* Testcase to check generation of a SH2A specific instruction
> + "BSET #imm3,Rn" */
[snip]
> + volatile static struct a x = { 1, 66, ~1 }, y =
> + {
> + 1, 2, ~2};
Same here.
> 2007-10-25 Naveen.H.S naveen.hs@kpitcummins.com
>
> * config/sh/constraints.md : Pso, Psz : New constraints.
> * config/sh/sh.c (print_operand): "V" prints the position of a
> single
> bit set.
> "W" prints the position of a single bit cleared.
> * config/sh/sh.md : andsi3_bclr, iorsi3_bset : New.
The style of this entry is wrong. Should be
200z-xx-yy Naveen.H.S <naveen.hs@kpitcummins.com>
* config/sh/constraints.md (Pso, Psz): New constraints.
* config/sh/sh.c (print_operand): Add %V and %W operand codes.
* config/sh/sh.md (*andsi3_bclr, *iorsi3_bset): New insns.
Also new testcases require an entry in gcc/testsuite/ChangeLog:
200z-xx-yy Naveen.H.S <naveen.hs@kpitcummins.com>
* gcc.target/sh/sh2a-bclr.c: New test.
* gcc.target/sh/sh2a-bset.c: New test.
Regards,
kaz