Hi Saurabh,
> On 22 Jul 2024, at 13:39, saurabh....@arm.com wrote:
>
> External email: Use caution opening links or attachments
>
>
> The AArch64 FEAT_FAMINMAX extension is optional in Armv9.2 and mandatory
> in Armv9.5. It introduces instructions for computing the floating point
> absolute maximum and minimum of the two vectors element-wise.
>
> This patch introduces intrinsics for AdvSIMD faminmax extension in the
> form of the following builtin-functions:
> * vamax_f16
> * vamaxq_f16
> * vamax_f32
> * vamaxq_f32
> * vamaxq_f64
> * vamin_f16
> * vaminq_f16
> * vamin_f32
> * vaminq_f32
> * vaminq_f64
>
> gcc/ChangeLog:
>
> * config/aarch64/aarch64-builtins.cc
> (enum aarch64_builtins): New enum values for faminmax builtins.
> (aarch64_init_faminmax_builtins): New function to declare new
> builtins.
> (handle_arm_neon_h): Modified to call
> aarch64_init_faminmax_builtins.
> (aarch64_general_check_builtin_call): Modified to check whether
> +faminmax flag is being used and printing error message if not used.
> (aarch64_expand_builtin_faminmax): New function to emit
> instructions of this extension.
> (aarch64_general_expand_builtin): Modified to call
> aarch64_expand_builtin_faminmax.
> * config/aarch64/aarch64-option-extensions.def
> (AARCH64_OPT_EXTENSION): Introduce new flag for this extension.
> * config/aarch64/aarch64-simd.md
> (aarch64_<faminmax><mode>): Introduce instruction pattern for this
> extension.
> * config/aarch64/aarch64.h
> (TARGET_FAMINMAX): Introduce new flag for this extension.
> * config/aarch64/iterators.md: Introduce new iterators for this
> extension.
> * config/arm/types.md: Introduce neon_fp_aminmax<q> attributes.
> * doc/invoke.texi: Document extension in AArch64 Options.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/aarch64/simd/faminmax.c: New tests for this
> extension.
> ---
> Hi,
>
> Regression tested for aarch64-none-linux-gnu and found no regressions.
>
> This patch is dependent on the patch series "Extend
> aarch64_feature_flags to 128 bits" which is under review. This patch
> should be commited only after that patch series is commited. I am
> raising this patch now for early feedback.
>
> Ok for master? I don't have commit access so can someone please commit
> on my behalf?
>
> Regards,
> Saurabh
> ---
> gcc/config/aarch64/aarch64-builtins.cc | 150 ++++++++++++++++--
> .../aarch64/aarch64-option-extensions.def | 2 +
> gcc/config/aarch64/aarch64-simd.md | 11 ++
> gcc/config/aarch64/aarch64.h | 4 +
> gcc/config/aarch64/iterators.md | 10 ++
> gcc/config/arm/types.md | 6 +
> gcc/doc/invoke.texi | 2 +
> .../gcc.target/aarch64/simd/faminmax.c | 40 +++++
> 8 files changed, 216 insertions(+), 9 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/aarch64/simd/faminmax.c
>
> <0001-aarch64-Add-ACLE-intrinsics-for-AdvSIMD-faminmax.patch>
@@ -2197,15 +2269,34 @@ aarch64_general_check_builtin_call (location_t
location, vec<location_t>,
case AARCH64_WSR64:
case AARCH64_WSRF:
case AARCH64_WSRF64:
- tree addr = STRIP_NOPS (args[0]);
- if (TREE_CODE (TREE_TYPE (addr)) != POINTER_TYPE
- || TREE_CODE (addr) != ADDR_EXPR
- || TREE_CODE (TREE_OPERAND (addr, 0)) != STRING_CST)
- {
- error_at (location, "first argument to %qD must be a string literal",
- fndecl);
- return false;
- }
+ {
+ tree addr = STRIP_NOPS (args[0]);
+ if (TREE_CODE (TREE_TYPE (addr)) != POINTER_TYPE
+ || TREE_CODE (addr) != ADDR_EXPR
+ || TREE_CODE (TREE_OPERAND (addr, 0)) != STRING_CST)
+ {
+ error_at (location,
+ "first argument to %qD must be a string literal",
+ fndecl);
+ return false;
+ }
+ }
+ case AARCH64_FAMINMAX_BUILTIN_FAMAX8H:
+ case AARCH64_FAMINMAX_BUILTIN_FAMAX2S:
+ case AARCH64_FAMINMAX_BUILTIN_FAMAX4S:
+ case AARCH64_FAMINMAX_BUILTIN_FAMAX2D:
+ case AARCH64_FAMINMAX_BUILTIN_FAMIN4H:
+ case AARCH64_FAMINMAX_BUILTIN_FAMIN8H:
+ case AARCH64_FAMINMAX_BUILTIN_FAMIN2S:
+ case AARCH64_FAMINMAX_BUILTIN_FAMIN4S:
+ case AARCH64_FAMINMAX_BUILTIN_FAMIN2D:
+ {
+ if (!TARGET_FAMINMAX)
+ {
+ error_at (location, "need +faminmax flag to call %qD", fndecl);
I’d rather keep these error messages consistent around the backend.
Can we reuse report_missing_extension from aarch64-sve-builtins.cc
<http://aarch64-sve-builtins.cc/> somehow to have the “intrinsic requires +foo
extension” reporting in one place?
diff --git a/gcc/config/aarch64/aarch64-simd.md
b/gcc/config/aarch64/aarch64-simd.md
index bbeee221f37..c769a482312 100644
--- a/gcc/config/aarch64/aarch64-simd.md
+++ b/gcc/config/aarch64/aarch64-simd.md
@@ -9881,3 +9881,14 @@
"shl\\t%d0, %d1, #16"
[(set_attr "type" "neon_shift_imm")]
)
+
+;; faminmax instruction patterns
+(define_insn "aarch64_<faminmax><mode>"
+ [(set (match_operand:VHSDF 0 "register_operand" "=w")
+ (unspec:VHSDF [(match_operand:VHSDF 1 "register_operand" "w")
+ (match_operand:VHSDF 2 "register_operand" "w")]
+ FAMINMAX_UNS))]
+ "TARGET_FAMINMAX"
+ "<faminmax>\t%0.<Vtype>, %1.<Vtype>, %2.<Vtype>"
+ [(set_attr "type" "neon_fp_aminmax<q>")]
+)
Any reason why we can’t represent these using organic RTL codes for smin/smax
and abs?
It would allow the RTL optimizers to more easily see through the semantics.
Or is the NaN/Inf handling of these instructions inconsistent with the RTL
semantics?
Thanks,
Kyrill
