[MIPS,committed] Backport bswap patches to 4.8

I got an off-list request to backport the bswap patterns to 4.8.
They've been in trunk for a while without any problems being reported
and they should be relatively safe.

Here's what I applied after testing on mips64-linux-gnu (with
--with-arch=mips64r2).  The only difference is the use of:

  [(set_attr "length" "8")]

rather than trunk's:

  [(set_attr "insn_count" "2")]

Thanks,
Richard


gcc/
	* config/mips/mips.h (ISA_HAS_WSBH): Define.
	* config/mips/mips.md (UNSPEC_WSBH, UNSPEC_DSBH, UNSPEC_DSHD): New
	constants.
	(bswaphi2, bswapsi2, bswapdi2, wsbh, dsbh, dshd): New patterns.

gcc/testsuite/
	* gcc.target/mips/bswap-1.c, gcc.target/mips/bswap-2.c,
	gcc.target/mips/bswap-3.c, gcc.target/mips/bswap-4.c,
	gcc.target/mips/bswap-5.c, gcc.target/mips/bswap-6.c: New tests.

On Thu, Jan 9, 2014 at 9:06 PM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> I got an off-list request to backport the bswap patterns to 4.8.
> They've been in trunk for a while without any problems being reported
> and they should be relatively safe.
>
> Here's what I applied after testing on mips64-linux-gnu (with
> --with-arch=mips64r2).  The only difference is the use of:
>
>   [(set_attr "length" "8")]
>
> rather than trunk's:
>
>   [(set_attr "insn_count" "2")]

FYI, just a clarification from a RM perspective - it's ok for target maintainers
to backport patches like this to the newest maintained release given they
only touch target specific code (even if they are not regressions).  We've
always had the policy to allow HW enablement on release branches and
the target maintainers decide whether it's safe and non-intrusive enough.

Thanks,
Richard.

> Thanks,
> Richard
>
>
> gcc/
>         * config/mips/mips.h (ISA_HAS_WSBH): Define.
>         * config/mips/mips.md (UNSPEC_WSBH, UNSPEC_DSBH, UNSPEC_DSHD): New
>         constants.
>         (bswaphi2, bswapsi2, bswapdi2, wsbh, dsbh, dshd): New patterns.
>
> gcc/testsuite/
>         * gcc.target/mips/bswap-1.c, gcc.target/mips/bswap-2.c,
>         gcc.target/mips/bswap-3.c, gcc.target/mips/bswap-4.c,
>         gcc.target/mips/bswap-5.c, gcc.target/mips/bswap-6.c: New tests.
>
> Index: gcc/config/mips/mips.h
> ===================================================================
> --- gcc/config/mips/mips.h      2014-01-09 14:59:58.086893612 +0000
> +++ gcc/config/mips/mips.h      2014-01-09 15:01:20.065606374 +0000
> @@ -949,6 +949,11 @@ #define ISA_HAS_ROR                ((ISA_MIPS32R2                          \
>                                   || TARGET_SMARTMIPS)                  \
>                                  && !TARGET_MIPS16)
>
> +/* ISA has the WSBH (word swap bytes within halfwords) instruction.
> +   64-bit targets also provide DSBH and DSHD.  */
> +#define ISA_HAS_WSBH           ((ISA_MIPS32R2 || ISA_MIPS64R2)         \
> +                                && !TARGET_MIPS16)
> +
>  /* ISA has data prefetch instructions.  This controls use of 'pref'.  */
>  #define ISA_HAS_PREFETCH       ((ISA_MIPS4                             \
>                                   || TARGET_LOONGSON_2EF                \
> Index: gcc/config/mips/mips.md
> ===================================================================
> --- gcc/config/mips/mips.md     2014-01-09 15:01:08.387504683 +0000
> +++ gcc/config/mips/mips.md     2014-01-09 15:02:06.590011975 +0000
> @@ -73,6 +73,11 @@ (define_c_enum "unspec" [
>    UNSPEC_STORE_LEFT
>    UNSPEC_STORE_RIGHT
>
> +  ;; Integer operations that are too cumbersome to describe directly.
> +  UNSPEC_WSBH
> +  UNSPEC_DSBH
> +  UNSPEC_DSHD
> +
>    ;; Floating-point moves.
>    UNSPEC_LOAD_LOW
>    UNSPEC_LOAD_HIGH
> @@ -5379,6 +5384,56 @@ (define_insn "rotr<mode>3"
>  }
>    [(set_attr "type" "shift")
>     (set_attr "mode" "<MODE>")])
> +
> +(define_insn "bswaphi2"
> +  [(set (match_operand:HI 0 "register_operand" "=d")
> +       (bswap:HI (match_operand:HI 1 "register_operand" "d")))]
> +  "ISA_HAS_WSBH"
> +  "wsbh\t%0,%1"
> +  [(set_attr "type" "shift")])
> +
> +(define_insn_and_split "bswapsi2"
> +  [(set (match_operand:SI 0 "register_operand" "=d")
> +       (bswap:SI (match_operand:SI 1 "register_operand" "d")))]
> +  "ISA_HAS_WSBH && ISA_HAS_ROR"
> +  "#"
> +  ""
> +  [(set (match_dup 0) (unspec:SI [(match_dup 1)] UNSPEC_WSBH))
> +   (set (match_dup 0) (rotatert:SI (match_dup 0) (const_int 16)))]
> +  ""
> +  [(set_attr "length" "8")])
> +
> +(define_insn_and_split "bswapdi2"
> +  [(set (match_operand:DI 0 "register_operand" "=d")
> +       (bswap:DI (match_operand:DI 1 "register_operand" "d")))]
> +  "TARGET_64BIT && ISA_HAS_WSBH"
> +  "#"
> +  ""
> +  [(set (match_dup 0) (unspec:DI [(match_dup 1)] UNSPEC_DSBH))
> +   (set (match_dup 0) (unspec:DI [(match_dup 0)] UNSPEC_DSHD))]
> +  ""
> +  [(set_attr "length" "8")])
> +
> +(define_insn "wsbh"
> +  [(set (match_operand:SI 0 "register_operand" "=d")
> +       (unspec:SI [(match_operand:SI 1 "register_operand" "d")] UNSPEC_WSBH))]
> +  "ISA_HAS_WSBH"
> +  "wsbh\t%0,%1"
> +  [(set_attr "type" "shift")])
> +
> +(define_insn "dsbh"
> +  [(set (match_operand:DI 0 "register_operand" "=d")
> +       (unspec:DI [(match_operand:DI 1 "register_operand" "d")] UNSPEC_DSBH))]
> +  "TARGET_64BIT && ISA_HAS_WSBH"
> +  "dsbh\t%0,%1"
> +  [(set_attr "type" "shift")])
> +
> +(define_insn "dshd"
> +  [(set (match_operand:DI 0 "register_operand" "=d")
> +       (unspec:DI [(match_operand:DI 1 "register_operand" "d")] UNSPEC_DSHD))]
> +  "TARGET_64BIT && ISA_HAS_WSBH"
> +  "dshd\t%0,%1"
> +  [(set_attr "type" "shift")])
>
>  ;;
>  ;;  ....................
> Index: gcc/testsuite/gcc.target/mips/bswap-1.c
> ===================================================================
> --- /dev/null   2013-12-26 20:29:50.272541227 +0000
> +++ gcc/testsuite/gcc.target/mips/bswap-1.c     2014-01-09 15:01:20.067606391 +0000
> @@ -0,0 +1,10 @@
> +/* { dg-options "isa_rev>=2" } */
> +/* { dg-skip-if "bswap recognition needs expensive optimizations" { *-*-* } { "-O0" "-O1" } { "" } } */
> +
> +NOMIPS16 unsigned short
> +foo (unsigned short x)
> +{
> +  return ((x << 8) & 0xff00) | ((x >> 8) & 0xff);
> +}
> +
> +/* { dg-final { scan-assembler "\twsbh\t" } } */
> Index: gcc/testsuite/gcc.target/mips/bswap-2.c
> ===================================================================
> --- /dev/null   2013-12-26 20:29:50.272541227 +0000
> +++ gcc/testsuite/gcc.target/mips/bswap-2.c     2014-01-09 15:01:20.067606391 +0000
> @@ -0,0 +1,9 @@
> +/* { dg-options "isa_rev>=2" } */
> +
> +NOMIPS16 unsigned short
> +foo (unsigned short x)
> +{
> +  return __builtin_bswap16 (x);
> +}
> +
> +/* { dg-final { scan-assembler "\twsbh\t" } } */
> Index: gcc/testsuite/gcc.target/mips/bswap-3.c
> ===================================================================
> --- /dev/null   2013-12-26 20:29:50.272541227 +0000
> +++ gcc/testsuite/gcc.target/mips/bswap-3.c     2014-01-09 15:01:20.067606391 +0000
> @@ -0,0 +1,14 @@
> +/* { dg-options "isa_rev>=2" } */
> +/* { dg-skip-if "bswap recognition needs expensive optimizations" { *-*-* } { "-O0" "-O1" } { "" } } */
> +
> +NOMIPS16 unsigned int
> +foo (unsigned int x)
> +{
> +  return (((x << 24) & 0xff000000)
> +         | ((x << 8) & 0xff0000)
> +         | ((x >> 8) & 0xff00)
> +         | ((x >> 24) & 0xff));
> +}
> +
> +/* { dg-final { scan-assembler "\twsbh\t" } } */
> +/* { dg-final { scan-assembler "\tror\t" } } */
> Index: gcc/testsuite/gcc.target/mips/bswap-4.c
> ===================================================================
> --- /dev/null   2013-12-26 20:29:50.272541227 +0000
> +++ gcc/testsuite/gcc.target/mips/bswap-4.c     2014-01-09 15:01:20.068606400 +0000
> @@ -0,0 +1,10 @@
> +/* { dg-options "isa_rev>=2" } */
> +
> +NOMIPS16 unsigned int
> +foo (unsigned int x)
> +{
> +  return __builtin_bswap32 (x);
> +}
> +
> +/* { dg-final { scan-assembler "\twsbh\t" } } */
> +/* { dg-final { scan-assembler "\tror\t" } } */
> Index: gcc/testsuite/gcc.target/mips/bswap-5.c
> ===================================================================
> --- /dev/null   2013-12-26 20:29:50.272541227 +0000
> +++ gcc/testsuite/gcc.target/mips/bswap-5.c     2014-01-09 15:01:20.068606400 +0000
> @@ -0,0 +1,20 @@
> +/* { dg-options "isa_rev>=2 -mgp64" } */
> +/* { dg-skip-if "bswap recognition needs expensive optimizations" { *-*-* } { "-O0" "-O1" } { "" } } */
> +
> +typedef unsigned long long uint64_t;
> +
> +NOMIPS16 uint64_t
> +foo (uint64_t x)
> +{
> +  return (((x << 56) & 0xff00000000000000ull)
> +         | ((x << 40) & 0xff000000000000ull)
> +         | ((x << 24) & 0xff0000000000ull)
> +         | ((x << 8) & 0xff00000000ull)
> +         | ((x >> 8) & 0xff000000)
> +         | ((x >> 24) & 0xff0000)
> +         | ((x >> 40) & 0xff00)
> +         | ((x >> 56) & 0xff));
> +}
> +
> +/* { dg-final { scan-assembler "\tdsbh\t" } } */
> +/* { dg-final { scan-assembler "\tdshd\t" } } */
> Index: gcc/testsuite/gcc.target/mips/bswap-6.c
> ===================================================================
> --- /dev/null   2013-12-26 20:29:50.272541227 +0000
> +++ gcc/testsuite/gcc.target/mips/bswap-6.c     2014-01-09 15:01:20.068606400 +0000
> @@ -0,0 +1,12 @@
> +/* { dg-options "isa_rev>=2 -mgp64" } */
> +
> +typedef unsigned long long uint64_t;
> +
> +NOMIPS16 uint64_t
> +foo (uint64_t x)
> +{
> +  return __builtin_bswap64 (x);
> +}
> +
> +/* { dg-final { scan-assembler "\tdsbh\t" } } */
> +/* { dg-final { scan-assembler "\tdshd\t" } } */

Richard Biener <richard.guenther@gmail.com> writes:
> On Thu, Jan 9, 2014 at 9:06 PM, Richard Sandiford
> <rdsandiford@googlemail.com> wrote:
>> I got an off-list request to backport the bswap patterns to 4.8.
>> They've been in trunk for a while without any problems being reported
>> and they should be relatively safe.
>>
>> Here's what I applied after testing on mips64-linux-gnu (with
>> --with-arch=mips64r2).  The only difference is the use of:
>>
>>   [(set_attr "length" "8")]
>>
>> rather than trunk's:
>>
>>   [(set_attr "insn_count" "2")]
>
> FYI, just a clarification from a RM perspective - it's ok for target maintainers
> to backport patches like this to the newest maintained release given they
> only touch target specific code (even if they are not regressions).  We've
> always had the policy to allow HW enablement on release branches and
> the target maintainers decide whether it's safe and non-intrusive enough.

OK, thanks.  That's how I thought it worked and was why I didn't ask
explicitly.  Hope I didn't rock the boat here.  (Sounds like there might
have been an IRC discussion or something. :-))

I assumed the patch I just applied to add extra cases to the
/proc/cpuinfo detection for Loongson came under the same category.
I put that one in 4.7 too because it should be ultra safe and because
it's a host thing rather than a target thing (someone upgrading their
kernel might otherwise find that gcc suddenly changes behaviour).

Thanks,
Richard