Andrew Pinski <andrew.pin...@caviumnetworks.com> writes:
> Index: testsuite/gcc.target/mips/octeon2-lx-1.c
> ===================================================================
> --- testsuite/gcc.target/mips/octeon2-lx-1.c (revision 0)
> +++ testsuite/gcc.target/mips/octeon2-lx-1.c (revision 0)
> @@ -0,0 +1,18 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=octeon2 -O -mgp64" } */
> +
> +#define TEST(N, R, T) \
> + T f##N (T j, R *b, long long i) { return j + b[i]; } \
> + T g##N (T j, unsigned R *b, long long i) { return j + b[i]; }
> +
> +TEST (1, char, int)
> +TEST (2, char, long long)
> +/* { dg-final { scan-assembler-times "\tlbx\t" 2 } } */
> +/* { dg-final { scan-assembler-times "\tlbux\t" 2 } } */
> +TEST (3, short, int)
> +TEST (4, short, long long)
> +/* { dg-final { scan-assembler-times "\tlhx\t" 2 } } */
> +/* { dg-final { scan-assembler-times "\tlhux\t" 2 } } */
> +TEST (5, int, long long)
> +/* { dg-final { scan-assembler-times "\tlwx\t" 1 } } */
> +/* { dg-final { scan-assembler-times "\tlwux\t" 1 } } */
There's obviously nothing wrong with testing long long indices,
but it doesn't seem very typical. I'd prefer the attached, which
tests both "long long" and "int". (I checked that it works for
-mabi=n32 and -mabi=64 on mips64-linux-gnu.)
> -(define_insn "mips_lwx_<mode>"
> - [(set (match_operand:SI 0 "register_operand" "=d")
> - (mem:SI (plus:P (match_operand:P 1 "register_operand" "d")
> - (match_operand:P 2 "register_operand" "d"))))]
> - "ISA_HAS_DSP"
> - "lwx\t%0,%2(%1)"
(Reviewing the patch made me realise that this ought to be using IMOVE32,
just like lwxs does. But that's something for another time.)
> +(define_expand "mips_ldx"
> + [(match_operand:DI 0 "register_operand")
> + (match_operand 1 "pmode_register_operand")
> + (match_operand:SI 2 "register_operand")]
> + "ISA_HAS_DSP && TARGET_64BIT"
> +{
> + operands[2] = convert_to_mode (Pmode, operands[2], false);
> + emit_insn (PMODE_INSN (gen_mips_ldx,
> + (operands[0], operands[1], operands[2])));
> + DONE;
> +})
Seems like this and lwx could be combined using :GPR.
> -;; This attribute gives the length suffix for a sign- or zero-extension
> -;; instruction.
> -(define_mode_attr size [(QI "b") (HI "h")])
> +;; This attribute gives the length suffix for a sign-, zero-extension
> +;; load and store instruction.
> +(define_mode_attr size [(QI "b") (HI "h") (SI "w") (DI "d")])
> +(define_mode_attr SIZE [(QI "B") (HI "H") (SI "W") (DI "D")])
;; This attribute gives the length suffix for a load or store instruction.
;; The same suffixes work for zero and sign extensions.
> Index: config/mips/mips.c
> ===================================================================
> --- config/mips/mips.c (revision 182342)
> +++ config/mips/mips.c (working copy)
> @@ -2159,6 +2159,29 @@ mips_lwxs_address_p (rtx addr)
> }
> return false;
> }
> +
> +/* Return true if ADDR matches the pattern for the L{B,H,W,D}{,U}X load
> + indexed address instruction. Note that such addresses are
> + not considered legitimate in the TARGET_LEGITIMATE_ADDRESS_P
> + sense, because their use is so restricted. */
> +
> +static bool
> +mips_loadindexed_address_p (rtx addr, enum machine_mode mode)
Nitlet, but I'd prefer mips_lx_address_p or mips_load_indexed_address_p.
> @@ -3552,6 +3575,11 @@ mips_rtx_costs (rtx x, int code, int out
> *total = COSTS_N_INSNS (2);
> return true;
> }
> + if (mips_loadindexed_address_p (addr, mode))
> + {
> + *total = COSTS_N_INSNS (2);
> + return true;
> + }
Please combine this with the lwxs condition.
> @@ -12959,6 +12988,9 @@ static const struct mips_builtin_descrip
> DIRECT_BUILTIN (mult, MIPS_DI_FTYPE_SI_SI, dsp_32),
> DIRECT_BUILTIN (multu, MIPS_DI_FTYPE_USI_USI, dsp_32),
>
> + /* Built-in functions for the DSP ASE (64-bit only). */
> + DIRECT_BUILTIN (ldx, MIPS_DI_FTYPE_POINTER_SI, dsp_64),
> +
> /* The following are for the MIPS DSP ASE REV 2 (32-bit only). */
> DIRECT_BUILTIN (dpa_w_ph, MIPS_DI_FTYPE_DI_V2HI_V2HI, dspr2_32),
> DIRECT_BUILTIN (dps_w_ph, MIPS_DI_FTYPE_DI_V2HI_V2HI, dspr2_32),
You need to add this to the list in extend.texi too.
> +#define ISA_HAS_LDX ((ISA_HAS_DSP || TARGET_OCTEON2) &&
> TARGET_64BIT)
Long line:
#define ISA_HAS_LDX ((ISA_HAS_DSP || TARGET_OCTEON2) \
&& TARGET_64BIT)
OK otherwise, thanks.
Richard
/* { dg-do compile } */
/* { dg-options "-march=octeon2 -O -mgp64" } */
#define TEST(N, R, T) \
T fll##N (T j, R *b, long long i) { return j + b[i]; } \
T gll##N (T j, unsigned R *b, long long i) { return j + b[i]; } \
T fi##N (T j, R *b, int i) { return j + b[i]; } \
T gi##N (T j, unsigned R *b, int i) { return j + b[i]; } \
TEST (1, char, int)
TEST (2, char, long long)
/* { dg-final { scan-assembler-times "\tlbx\t" 4 } } */
/* { dg-final { scan-assembler-times "\tlbux\t" 4 } } */
TEST (3, short, int)
TEST (4, short, long long)
/* { dg-final { scan-assembler-times "\tlhx\t" 4 } } */
/* { dg-final { scan-assembler-times "\tlhux\t" 4 } } */
TEST (5, int, long long)
/* { dg-final { scan-assembler-times "\tlwx\t" 2 } } */
/* { dg-final { scan-assembler-times "\tlwux\t" 2 } } */
