On 10/06/16 13:29, James Greenhalgh wrote:
>
> Hi,
>
> My autotester picked up some issues with the vcvt{ds}_n_* intrinsics
> added in r237200.
>
> The iterators in this pattern do not resolve, as they have not been
> explicitly tied to the mode iterator (rather than the code iterator)
> used by the pattern.
>
> This fixup adds the attribute tags, allowing the patterns to work
> correctly.
>
> Additionally, the types assigned to these instructions were wrong, and
> would permit the immediate operand to be in a register. This will then
> develop in to an ICE as the patterns require an immediate operand, and so
> won't match. The ICE can be exposed by writing a wrapping function around
> the vcvtd_n_* intrinsics, which forces the immediate operand to a register.
> We have the infrastructure to error to the user rather than ICEing, but it
> needs some different types, which this patch adds.
>
> I've checked this with an aarch64-none-elf test run, and run it through
> several rounds of my autotester for aarch64-none-elf and
> aarch64_be-none-elf.
>
> OK?
>
OK.
R.
> Thanks,
> James
>
> ---
> 2016-06-10 James Greenhalgh <james.greenha...@arm.com>
>
> * config/aarch64/aarch64.md
> (<FCVT_F2FIXED:fcvt_fixed_insn><GPF:mode>3): Add attributes to
> iterators.
> (<FCVT_FIXED2F:fcvt_fixed_insn><GPI:mode>3): Likewise. Correct
> attributes.
> * config/aarch64/aarch64-builtins.c
> (aarch64_types_binop_uss_qualifiers): Delete.
> (TYPES_BINOP_USS): Likewise.
> (aarch64_types_binop_sus_qualifiers): Likewise.
> (TYPES_BINOP_SUS): Likewise.
> (aarch64_types_fcvt_from_unsigned_qualifiers): New.
> (TYPES_FCVTIMM_SUS): Likewise.
> * config/aarch64/aarch64-simd-builtins.def (scvtf): Use SHIFTIMM
> rather than BINOP.
> (ucvtf): Use FCVTIMM_SUS rather than BINOP_SUS.
> (fcvtzs): Use SHIFTIMM rather than BINOP.
> (fcvtzu): Use SHIFTIMM_USS rather than BINOP_USS.
>
>
> 0001-Patch-AArch64-Fixup-to-fcvt-patterns-added-in-r23720.patch
>
>
> diff --git a/gcc/config/aarch64/aarch64-builtins.c
> b/gcc/config/aarch64/aarch64-builtins.c
> index 262ea1c..6b90b2a 100644
> --- a/gcc/config/aarch64/aarch64-builtins.c
> +++ b/gcc/config/aarch64/aarch64-builtins.c
> @@ -139,14 +139,6 @@ aarch64_types_binop_ssu_qualifiers[SIMD_MAX_BUILTIN_ARGS]
> = { qualifier_none, qualifier_none, qualifier_unsigned };
> #define TYPES_BINOP_SSU (aarch64_types_binop_ssu_qualifiers)
> static enum aarch64_type_qualifiers
> -aarch64_types_binop_uss_qualifiers[SIMD_MAX_BUILTIN_ARGS]
> - = { qualifier_unsigned, qualifier_none, qualifier_none };
> -#define TYPES_BINOP_USS (aarch64_types_binop_uss_qualifiers)
> -static enum aarch64_type_qualifiers
> -aarch64_types_binop_sus_qualifiers[SIMD_MAX_BUILTIN_ARGS]
> - = { qualifier_none, qualifier_unsigned, qualifier_none };
> -#define TYPES_BINOP_SUS (aarch64_types_binop_sus_qualifiers)
> -static enum aarch64_type_qualifiers
> aarch64_types_binopp_qualifiers[SIMD_MAX_BUILTIN_ARGS]
> = { qualifier_poly, qualifier_poly, qualifier_poly };
> #define TYPES_BINOPP (aarch64_types_binopp_qualifiers)
> @@ -181,6 +173,10 @@
> aarch64_types_shift_to_unsigned_qualifiers[SIMD_MAX_BUILTIN_ARGS]
> = { qualifier_unsigned, qualifier_none, qualifier_immediate };
> #define TYPES_SHIFTIMM_USS (aarch64_types_shift_to_unsigned_qualifiers)
> static enum aarch64_type_qualifiers
> +aarch64_types_fcvt_from_unsigned_qualifiers[SIMD_MAX_BUILTIN_ARGS]
> + = { qualifier_none, qualifier_unsigned, qualifier_immediate };
> +#define TYPES_FCVTIMM_SUS (aarch64_types_fcvt_from_unsigned_qualifiers)
> +static enum aarch64_type_qualifiers
> aarch64_types_unsigned_shift_qualifiers[SIMD_MAX_BUILTIN_ARGS]
> = { qualifier_unsigned, qualifier_unsigned, qualifier_immediate };
> #define TYPES_USHIFTIMM (aarch64_types_unsigned_shift_qualifiers)
> diff --git a/gcc/config/aarch64/aarch64-simd-builtins.def
> b/gcc/config/aarch64/aarch64-simd-builtins.def
> index 1332734..02d465b 100644
> --- a/gcc/config/aarch64/aarch64-simd-builtins.def
> +++ b/gcc/config/aarch64/aarch64-simd-builtins.def
> @@ -447,10 +447,10 @@
> BUILTIN_VSDQ_HSI (QUADOP_LANE, sqrdmlsh_laneq, 0)
>
> /* Implemented by <FCVT_F2FIXED/FIXED2F:fcvt_fixed_insn><*><*>3. */
> - BUILTIN_VSDQ_SDI (BINOP, scvtf, 3)
> - BUILTIN_VSDQ_SDI (BINOP_SUS, ucvtf, 3)
> - BUILTIN_VALLF (BINOP, fcvtzs, 3)
> - BUILTIN_VALLF (BINOP_USS, fcvtzu, 3)
> + BUILTIN_VSDQ_SDI (SHIFTIMM, scvtf, 3)
> + BUILTIN_VSDQ_SDI (FCVTIMM_SUS, ucvtf, 3)
> + BUILTIN_VALLF (SHIFTIMM, fcvtzs, 3)
> + BUILTIN_VALLF (SHIFTIMM_USS, fcvtzu, 3)
>
> /* Implemented by aarch64_rsqrte<mode>. */
> BUILTIN_VALLF (UNOP, rsqrte, 0)
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 926f2da..b3ae42b 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -4640,8 +4640,8 @@
> FCVT_F2FIXED))]
> ""
> "@
> - <FCVT_F2FIXED:fcvt_fixed_insn>\t%<w1>0, %<s>1, #%2
> - <FCVT_F2FIXED:fcvt_fixed_insn>\t%<s>0, %<s>1, #%2"
> + <FCVT_F2FIXED:fcvt_fixed_insn>\t%<GPF:w1>0, %<GPF:s>1, #%2
> + <FCVT_F2FIXED:fcvt_fixed_insn>\t%<GPF:s>0, %<GPF:s>1, #%2"
> [(set_attr "type" "f_cvtf2i, neon_fp_to_int_<GPF:Vetype>")
> (set_attr "fp" "yes, *")
> (set_attr "simd" "*, yes")]
> @@ -4654,8 +4654,8 @@
> FCVT_FIXED2F))]
> ""
> "@
> - <FCVT_FIXED2F:fcvt_fixed_insn>\t%<s>0, %<w1>1, #%2
> - <FCVT_FIXED2F:fcvt_fixed_insn>\t%<s>0, %<s>1, #%2"
> + <FCVT_FIXED2F:fcvt_fixed_insn>\t%<GPI:v>0, %<GPI:w>1, #%2
> + <FCVT_FIXED2F:fcvt_fixed_insn>\t%<GPI:v>0, %<GPI:v>1, #%2"
> [(set_attr "type" "f_cvti2f, neon_int_to_fp_<GPI:Vetype>")
> (set_attr "fp" "yes, *")
> (set_attr "simd" "*, yes")]
>
