Hi Carl,

On Tue, Jul 21, 2020 at 10:32:53AM -0700, Carl Love wrote:
>    Looked at comment about changing the return type of 
> rs6000_const_f32_to_i32 (rtx operand)
>       from long to int or unsigned int.  If I make that change I get an ICE
>       on insn not recognized.

Let's leave that for a future improvement, then (or never to be done :-) )

> +(define_expand "xxspltidp_v2df"
> +  [(set (match_operand:V2DF 0 "register_operand" )
> +     (unspec:V2DF [(match_operand:SF 1 "const_double_operand")]
> +                  UNSPEC_XXSPLTID))]
> + "TARGET_POWER10"
> +{
> +  long value = rs6000_const_f32_to_i32 (operands[1]);
> +  rs6000_emit_xxspltidp_v2df (operands[0], value);
> +  DONE;
> +})
> +
> +(define_insn "xxspltidp_v2df_inst"
> +  [(set (match_operand:V2DF 0 "register_operand" "=wa")
> +     (unspec:V2DF [(match_operand:SI 1 "c32bit_cint_operand" "n")]
> +                  UNSPEC_XXSPLTID))]
> +  "TARGET_POWER10"
> +  "xxspltidp %x0,%1"
> +  [(set_attr "type" "vecsimple")])

I guess the only current way to generate this rtl insn is via the
builtin, and it starts off as an immediate argument to an unspec, so
nothing in GCC can change it.  So we could just pass it through to the
assembler, and *it* should complain (or let it through as well, but that
is not very friendly to the user, this is rather hard to debug).

> +void
> +rs6000_emit_xxspltidp_v2df (rtx dst, long value)
> +{
> +  printf("rs6000_emit_xxspltidp_v2df called %ld\n", value);
> +  printf("rs6000_emit_xxspltidp_v2df called 0x%lx\n", value);

Old debugging code?  Please remove these two lines.

> +  if (((value & 0x7F800000) == 0) && ((value & 0x7FFFFF) != 0))
> +    inform (input_location,
> +         "the result for the xxspltidp instruction is undefined for 
> subnormal input values.\n");

This as well, given the above.

> +  emit_insn( gen_xxspltidp_v2df_inst (dst, GEN_INT (value)));

s/( / (/

Given the above, only this line is left here, so the callers can so that
GEN_INT themselves and elide this routine, simplifying the code?

A short source code comment near the define_insn (saying subnormals do
not work with this insn) might be helpful still.

> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/vec-splati-runnable.c

> +/* { dg-final { scan-assembler-times {\msplati\M} 6 } } */
> +/* { dg-final { scan-assembler-times {\msrdbi\M} 6 } } */

Those aren't existing instructions?  How did this pass testing?  I guess
the testcase was skipped?


Segher

Reply via email to