<saurabh....@arm.com> writes:
> Refactor AdvSIMD intrinsics defined using the new pragma-based approach
> so that it is more extensible.
>
> Introduce a new struct, simd_type, which defines types using a mode and
> qualifiers, and use objects of this struct in the declaration of intrinsics
> in the aarch64-simd-pragma-builtins.def file.
>
> Change aarch64_pragma_builtins_data struct to support return type and
> argument types.
>
> Refactor aarch64_fntype and aarch64_expand_pragma_builtin so that it
> initialises corresponding vectors in a loop. As we add intrinsics with
> more arguments, these functions won't need to change to support those.
>
> gcc/ChangeLog:
>
> * config/aarch64/aarch64-builtins.cc
> (ENTRY): Modify to add support of return and argument types.
> (struct simd_type): New struct to declare types using mode and
> qualifiers.
> (struct aarch64_pragma_builtins_data): Replace mode with
> the array of types to support return and argument types.
> (aarch64_get_number_of_args): New utility to get number of
> arguments given an aarch64_builtin_signatures variant.
> (aarch64_fntype): Modify to handle different signatures.
> (aarch64_expand_pragma_builtin): Modify to handle different
> signatures.
> * config/aarch64/aarch64-simd-pragma-builtins.def
> (ENTRY_VHSDF): Rename to ENTRY_BINARY_VHSDF.
> (ENTRY_BINARY): New macro to declare binary intrinsics.
> (ENTRY_BINARY_VHSDF): Remove signature argument and use
> ENTRY_BINARY.
> ---
> gcc/config/aarch64/aarch64-builtins.cc | 106 ++++++++++++++----
> .../aarch64/aarch64-simd-pragma-builtins.def | 22 ++--
> 2 files changed, 97 insertions(+), 31 deletions(-)
Thanks for picking this up. As we discussed off-line, I'll do the
review in the form of a patch rather than as English text, in the
interests of parallelising the remaining work.
The main things were:
- I'd suggested in one of the replies to Vladimir that we add:
tree type () const { return aarch64_simd_builtin_type (mode, qualifiers); }
to simd_type, to reduce the cut-&-paste. This patch does that.
- I hadn't realised that my suggestion to use build_function_type_vec
would require a GCed vector. Given that it does, I suppose we should
use build_function_type_array instead. Sorry for the run-around.
- When setting up the operands during expand, it's probably more robust
to use the mode of the associated type, since that is tied directly
to the operand value. This removes the need to describe every argument
(including non-vector ones) using simd_type, which will be useful for
later patches.
- The:
expand_insn (icode, ops.length (), ops.address ());
return ops[0].value;
sequence is likely to be used by most intrinsics, so I think we should
do it after the main switch, rather than in each case statement.
Here's what I'd like to commit. I'll wait until tomorrow in case there
are any comments.
Thanks,
Richard
>From c9bcc73f3b09f072dbc97c8e8c3f46ed0d7c1bfc Mon Sep 17 00:00:00 2001
From: Saurabh Jha <saurabh....@arm.com>
Date: Tue, 26 Nov 2024 12:18:24 +0000
Subject: [PATCH] aarch64: Refactor AdvSIMD intrinsics
To: gcc-patches@gcc.gnu.org
Refactor AdvSIMD intrinsics defined using the new pragma-based approach
so that it is more extensible.
Introduce a new struct, simd_type, which defines types using a mode and
qualifiers, and use objects of this struct in the declaration of intrinsics
in the aarch64-simd-pragma-builtins.def file.
Change aarch64_pragma_builtins_data struct to support return type and
argument types.
Refactor aarch64_fntype and aarch64_expand_pragma_builtin so that it
initialises corresponding vectors in a loop. As we add intrinsics with
more arguments, these functions won't need to change to support those.
gcc/ChangeLog:
* config/aarch64/aarch64-builtins.cc
(ENTRY): Modify to add support of return and argument types.
(struct simd_type): New struct to declare types using mode and
qualifiers.
(struct aarch64_pragma_builtins_data): Replace mode with
the array of types to support return and argument types.
(aarch64_fntype): Modify to handle different signatures.
(aarch64_expand_pragma_builtin): Modify to handle different
signatures.
* config/aarch64/aarch64-simd-pragma-builtins.def
(ENTRY_VHSDF): Rename to ENTRY_BINARY_VHSDF.
(ENTRY_BINARY): New macro to declare binary intrinsics.
(ENTRY_BINARY_VHSDF): Remove signature argument and use
ENTRY_BINARY.
Co-authored-by: Vladimir Miloserdov <vladimir.miloser...@arm.com>
Co-authored-by: Richard Sandiford <richard.sandif...@arm.com>
---
gcc/config/aarch64/aarch64-builtins.cc | 108 ++++++++++++++----
gcc/config/aarch64/aarch64-builtins.h | 2 +-
.../aarch64/aarch64-simd-pragma-builtins.def | 22 ++--
3 files changed, 99 insertions(+), 33 deletions(-)
diff --git a/gcc/config/aarch64/aarch64-builtins.cc
b/gcc/config/aarch64/aarch64-builtins.cc
index b860e22f01f..3b170f29d3c 100644
--- a/gcc/config/aarch64/aarch64-builtins.cc
+++ b/gcc/config/aarch64/aarch64-builtins.cc
@@ -780,7 +780,7 @@ typedef struct
AARCH64_SIMD_BUILTIN_##T##_##N##A,
#undef ENTRY
-#define ENTRY(N, S, M, U) \
+#define ENTRY(N, S, T0, T1, T2, U) \
AARCH64_##N,
enum aarch64_builtins
@@ -1598,10 +1598,50 @@ enum class aarch64_builtin_signatures
binary,
};
+namespace {
+
+/* Pairs a machine mode with the information needed to turn it into a
+ function argument type or return type. */
+struct simd_type {
+ tree type () const { return aarch64_simd_builtin_type (mode, qualifiers); }
+
+ machine_mode mode;
+ aarch64_type_qualifiers qualifiers;
+};
+
+namespace simd_types {
+ constexpr simd_type p8 { V8QImode, qualifier_poly };
+ constexpr simd_type p8q { V16QImode, qualifier_poly };
+ constexpr simd_type s8 { V8QImode, qualifier_none };
+ constexpr simd_type s8q { V16QImode, qualifier_none };
+ constexpr simd_type u8 { V8QImode, qualifier_unsigned };
+ constexpr simd_type u8q { V16QImode, qualifier_unsigned };
+
+ constexpr simd_type f16 { V4HFmode, qualifier_none };
+ constexpr simd_type f16q { V8HFmode, qualifier_none };
+ constexpr simd_type p16 { V4HImode, qualifier_poly };
+ constexpr simd_type p16q { V8HImode, qualifier_poly };
+ constexpr simd_type s16 { V4HImode, qualifier_none };
+ constexpr simd_type s16q { V8HImode, qualifier_none };
+ constexpr simd_type u16 { V4HImode, qualifier_unsigned };
+ constexpr simd_type u16q { V8HImode, qualifier_unsigned };
+
+ constexpr simd_type bf16 { V4BFmode, qualifier_none };
+ constexpr simd_type bf16q { V8BFmode, qualifier_none };
+
+ constexpr simd_type f32 { V2SFmode, qualifier_none };
+ constexpr simd_type f32q { V4SFmode, qualifier_none };
+ constexpr simd_type f64q { V2DFmode, qualifier_none };
+
+ constexpr simd_type none { VOIDmode, qualifier_none };
+}
+
+}
+
#undef ENTRY
-#define ENTRY(N, S, M, U) \
- {#N, aarch64_builtin_signatures::S, E_##M##mode, U, \
- aarch64_required_extensions::REQUIRED_EXTENSIONS},
+#define ENTRY(N, S, T0, T1, T2, U) \
+ {#N, aarch64_builtin_signatures::S, simd_types::T0, simd_types::T1, \
+ simd_types::T2, U, aarch64_required_extensions::REQUIRED_EXTENSIONS},
/* Initialize pragma builtins. */
@@ -1609,7 +1649,7 @@ struct aarch64_pragma_builtins_data
{
const char *name;
aarch64_builtin_signatures signature;
- machine_mode mode;
+ simd_type types[3];
int unspec;
aarch64_required_extensions required_extensions;
};
@@ -1618,19 +1658,26 @@ static aarch64_pragma_builtins_data
aarch64_pragma_builtins[] = {
#include "aarch64-simd-pragma-builtins.def"
};
+/* Return the function type for BUILTIN_DATA. */
static tree
aarch64_fntype (const aarch64_pragma_builtins_data &builtin_data)
{
- auto type = aarch64_simd_builtin_type (builtin_data.mode, qualifier_none);
+ tree return_type = NULL_TREE;
+ auto_vec<tree, 8> arg_types;
switch (builtin_data.signature)
{
case aarch64_builtin_signatures::binary:
- return build_function_type_list (type, type, type, NULL_TREE);
- default:
- gcc_unreachable ();
+ return_type = builtin_data.types[0].type ();
+ for (int i = 1; i <= 2; ++i)
+ arg_types.quick_push (builtin_data.types[i].type ());
+ break;
}
+ return build_function_type_array (return_type, arg_types.length (),
+ arg_types.address ());
}
+/* Simulate function definitions for all of the builtins in
+ aarch64_pragma_builtins. */
static void
aarch64_init_pragma_builtins ()
{
@@ -3368,23 +3415,39 @@ aarch64_expand_builtin_data_intrinsic (unsigned int
fcode, tree exp, rtx target)
return ops[0].value;
}
+/* Expand CALL_EXPR EXP, given that it is a call to the function described
+ by BUILTIN_DATA, and return the function's return value. Put the result
+ in TARGET if convenient. */
static rtx
aarch64_expand_pragma_builtin (tree exp, rtx target,
- const aarch64_pragma_builtins_data *builtin_data)
+ const aarch64_pragma_builtins_data &builtin_data)
{
- expand_operand ops[3];
- auto mode = builtin_data->mode;
- auto op1 = expand_normal (CALL_EXPR_ARG (exp, 0));
- auto op2 = expand_normal (CALL_EXPR_ARG (exp, 1));
- create_output_operand (&ops[0], target, mode);
- create_input_operand (&ops[1], op1, mode);
- create_input_operand (&ops[2], op2, mode);
+ unsigned int nargs = call_expr_nargs (exp);
- auto unspec = builtin_data->unspec;
- auto icode = code_for_aarch64 (unspec, mode);
- expand_insn (icode, 3, ops);
+ auto_vec<expand_operand, 8> ops;
+ ops.safe_grow (nargs + 1);
+ create_output_operand (&ops[0], target, TYPE_MODE (TREE_TYPE (exp)));
+ for (unsigned int i = 1; i <= nargs; ++i)
+ {
+ tree arg = CALL_EXPR_ARG (exp, i - 1);
+ create_input_operand (&ops[i], expand_normal (arg),
+ TYPE_MODE (TREE_TYPE (arg)));
+ }
- return target;
+ insn_code icode;
+ switch (builtin_data.unspec)
+ {
+ case UNSPEC_FAMAX:
+ case UNSPEC_FAMIN:
+ icode = code_for_aarch64 (builtin_data.unspec,
+ builtin_data.types[0].mode);
+ break;
+ default:
+ gcc_unreachable ();
+ }
+
+ expand_insn (icode, ops.length (), ops.address ());
+ return ops[0].value;
}
/* Expand an expression EXP as fpsr or fpcr setter (depending on
@@ -3624,7 +3687,7 @@ aarch64_general_expand_builtin (unsigned int fcode, tree
exp, rtx target,
return aarch64_expand_builtin_data_intrinsic (fcode, exp, target);
if (auto builtin_data = aarch64_get_pragma_builtin (fcode))
- return aarch64_expand_pragma_builtin (exp, target, builtin_data);
+ return aarch64_expand_pragma_builtin (exp, target, *builtin_data);
gcc_unreachable ();
}
@@ -4278,7 +4341,6 @@ aarch64_resolve_overloaded_builtin_general (location_t
loc, tree function,
#undef CF3
#undef CF4
#undef CF10
-#undef ENTRY_VHSDF
#undef VAR1
#undef VAR2
#undef VAR3
diff --git a/gcc/config/aarch64/aarch64-builtins.h
b/gcc/config/aarch64/aarch64-builtins.h
index 00db7a74885..7282c44d7f1 100644
--- a/gcc/config/aarch64/aarch64-builtins.h
+++ b/gcc/config/aarch64/aarch64-builtins.h
@@ -98,4 +98,4 @@ struct GTY(()) aarch64_simd_type_info
extern aarch64_simd_type_info aarch64_simd_types[];
-#endif
\ No newline at end of file
+#endif
diff --git a/gcc/config/aarch64/aarch64-simd-pragma-builtins.def
b/gcc/config/aarch64/aarch64-simd-pragma-builtins.def
index d66642eaa0a..e49db23cbd1 100644
--- a/gcc/config/aarch64/aarch64-simd-pragma-builtins.def
+++ b/gcc/config/aarch64/aarch64-simd-pragma-builtins.def
@@ -18,16 +18,20 @@
along with GCC; see the file COPYING3. If not see
<http://www.gnu.org/licenses/>. */
-#undef ENTRY_VHSDF
-#define ENTRY_VHSDF(NAME, SIGNATURE, UNSPEC) \
- ENTRY (NAME##_f16, SIGNATURE, V4HF, UNSPEC) \
- ENTRY (NAME##q_f16, SIGNATURE, V8HF, UNSPEC) \
- ENTRY (NAME##_f32, SIGNATURE, V2SF, UNSPEC) \
- ENTRY (NAME##q_f32, SIGNATURE, V4SF, UNSPEC) \
- ENTRY (NAME##q_f64, SIGNATURE, V2DF, UNSPEC)
+#undef ENTRY_BINARY
+#define ENTRY_BINARY(N, T0, T1, T2, U) \
+ ENTRY (N, binary, T0, T1, T2, U)
+
+#undef ENTRY_BINARY_VHSDF
+#define ENTRY_BINARY_VHSDF(NAME, UNSPEC) \
+ ENTRY_BINARY (NAME##_f16, f16, f16, f16, UNSPEC) \
+ ENTRY_BINARY (NAME##q_f16, f16q, f16q, f16q, UNSPEC) \
+ ENTRY_BINARY (NAME##_f32, f32, f32, f32, UNSPEC) \
+ ENTRY_BINARY (NAME##q_f32, f32q, f32q, f32q, UNSPEC) \
+ ENTRY_BINARY (NAME##q_f64, f64q, f64q, f64q, UNSPEC)
// faminmax
#define REQUIRED_EXTENSIONS nonstreaming_only (AARCH64_FL_FAMINMAX)
-ENTRY_VHSDF (vamax, binary, UNSPEC_FAMAX)
-ENTRY_VHSDF (vamin, binary, UNSPEC_FAMIN)
+ENTRY_BINARY_VHSDF (vamax, UNSPEC_FAMAX)
+ENTRY_BINARY_VHSDF (vamin, UNSPEC_FAMIN)
#undef REQUIRED_EXTENSIONS
--
2.25.1
