Hi!
On Wed, Jul 08, 2020 at 12:59:29PM -0700, Carl Love wrote:
> [PATCH 5/6] rs6000, Add vector splat builtin support
> +(define_insn "xxspltiw_v4si"
> + [(set (match_operand:V4SI 0 "register_operand" "=wa")
> + (unspec:V4SI [(match_operand:SI 1 "s32bit_cint_operand" "n")]
> + UNSPEC_XXSPLTIW))]
> + "TARGET_POWER10"
> + "xxspltiw %x0,%1"
> + [(set_attr "type" "vecsimple")])
Hrm, from the instruction description (in the ISA) this should be an
unsigned integer, instead? (GNU as doesn't care, it takes the low 32
bits, of any integer, it doesn't have to be either a s32 or a u32
apparently).
> +(define_insn "xxspltiw_v4sf_inst"
> + [(set (match_operand:V4SF 0 "register_operand" "=wa")
> + (unspec:V4SF [(match_operand:SI 1 "c32bit_cint_operand" "n")]
> + UNSPEC_XXSPLTIW))]
> + "TARGET_POWER10"
> + "xxspltiw %x0,%c1"
> + [(set_attr "type" "vecsimple")])
This will do exactly the same as just "%1"? Or not? (I.e. call
output_addr_const for that arg). (We don't use %c anywhere else in the
port AFAICS, so let's not start that if there is no reason to).
> +(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]);
> + emit_insn (gen_xxspltidp_v2df_inst (operands[0], GEN_INT (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"
> +{
> + /* Note, the xxspltidp gives undefined results if the operand is a single
> + precision subnormal number. */
> + int value = INTVAL (operands[1]);
> +
> + if (((value & 0x7F800000) == 0) && ((value & 0x7FFFFF) != 0))
> + /* value is subnormal */
> + fprintf (stderr, "WARNING: Result for the xxspltidp instruction is
> undefined for subnormal input values.\n");
> +
> + return "xxspltidp %x0,%c1";
> +}
> + [(set_attr "type" "vecsimple")])
There are utility functions to print warnings. But, we shouldn't at all
here. Instead, the insn shouldn't match at all with bad inputs, or give
an actual error maybe (although it is nicer if the builtin handling code
does that).
> +(define_insn "xxsplti32dx_v4sf_inst"
> + [(set (match_operand:V4SF 0 "register_operand" "=wa")
> + (unspec:V4SF [(match_operand:V4SF 1 "register_operand" "0")
> + (match_operand:QI 2 "u1bit_cint_operand" "n")
> + (match_operand:SI 3 "s32bit_cint_operand" "n")]
> + UNSPEC_XXSPLTI32DX))]
> + "TARGET_POWER10"
> + "xxsplti32dx %x0,%2,%3"
> + [(set_attr "type" "vecsimple")])
(a space too much indent here)
> +;; Return 1 if op is a unsigned 1-bit constant integer.
> +(define_predicate "u1bit_cint_operand"
"an unsigned"
> +long long
> +rs6000_const_f32_to_i32 (rtx operand)
> +{
> + long long value;
> + const struct real_value *rv = CONST_DOUBLE_REAL_VALUE (operand);
> +
> + gcc_assert (GET_MODE (operand) == SFmode);
> + REAL_VALUE_TO_TARGET_SINGLE (*rv, value);
> + return value;
> +}
Can this just return "int"? (Or "unsigned int"?)
The rest of the patch looks good.
Segher
