Hi Karl,
> On 21 Aug 2026, at 15:18, Karl Meakin <[email protected]> wrote:
>
> Fix the behaviour of the `vadd` family of intrinsics on signed integer
> overflow. Previously, they followed the standard C semantics where
> signed overflow was undefined behaviour; but the ACLE requires signed
> overflow to have wraparound semantics. Fix by casting to unsigned before
> performing the addition.
>
> Also add tests for some floating point addition intrinsics that were
> missed.
>
> gcc/ChangeLog:
>
> * config/aarch64/aarch64-neon-builtins-base.cc (gimple_arith):
> New struct.
> (vaddd, vadd, vaddq): Use `gimple_arith` rather than
> `gimple_expr`.
> * config/aarch64/aarch64-neon-builtins-base.def: Reorder
> definitions and add comments.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/aarch64/neon/vadd.c: Add tests for `vadd_f16`,
> `vadd_f32`, `vadd_f64`.
> ---
> .../aarch64/aarch64-neon-builtins-base.cc | 62 ++++++++++++++++++-
> .../aarch64/aarch64-neon-builtins-base.def | 6 +-
> gcc/testsuite/gcc.target/aarch64/neon/vadd.c | 23 ++++++-
> 3 files changed, 86 insertions(+), 5 deletions(-)
>
> diff --git a/gcc/config/aarch64/aarch64-neon-builtins-base.cc
> b/gcc/config/aarch64/aarch64-neon-builtins-base.cc
> index d8fae81388e..81511911f0c 100644
> --- a/gcc/config/aarch64/aarch64-neon-builtins-base.cc
> +++ b/gcc/config/aarch64/aarch64-neon-builtins-base.cc
> @@ -204,6 +204,61 @@ public:
> }
> };
>
> +/* Like gimple_expr above, but converts signed inputs to unsigned first, to
> + avoid undefined behaviour on overflow. */
> +class gimple_arith : public gimple_function_base
> +{
> + tree_code m_int_code;
> + tree_code m_float_code;
> + tree_code m_poly_code;
> +
> +public:
> + constexpr gimple_arith (tree_code code)
> + : m_int_code (code), m_float_code (code), m_poly_code (code)
> + {}
> +
> + constexpr gimple_arith (tree_code int_code, tree_code float_code,
> + tree_code poly_code)
> + : m_int_code (int_code), m_float_code (float_code), m_poly_code
> (poly_code)
> + {}
> +
> + gimple *fold (gimple_folder &f) const override
> + {
> + auto nargs = gimple_call_num_args (f.call);
> + auto arg0 = nargs >= 1 ? gimple_call_arg (f.call, 0) : nullptr;
> + auto arg1 = nargs >= 2 ? gimple_call_arg (f.call, 1) : nullptr;
> + auto arg2 = nargs >= 3 ? gimple_call_arg (f.call, 2) : nullptr;
> +
> + auto type_class = f.type_suffix (0).tclass;
> + switch (type_class)
> + {
> + case TYPE_signed:
> + break;
> + case TYPE_unsigned:
> + return gimple_build_assign (f.lhs, m_int_code, arg0, arg1, arg2);
> + case TYPE_float:
> + return gimple_build_assign (f.lhs, m_float_code, arg0, arg1, arg2);
> + case TYPE_poly:
> + return gimple_build_assign (f.lhs, m_poly_code, arg0, arg1, arg2);
> + default:
> + gcc_unreachable ();
> + }
> +
> + auto signed_type = TREE_TYPE (arg0);
> + auto unsigned_type = unsigned_type_for (signed_type);
> + arg0 = arg0 ? f.force_val (build_cast (unsigned_type, arg0)) : nullptr;
> + arg1 = arg1 ? f.force_val (build_cast (unsigned_type, arg1)) : nullptr;
> + arg2 = arg2 ? f.force_val (build_cast (unsigned_type, arg2)) : nullptr;
> + auto ret = nargs == 1 ? fold_build1 (m_int_code, unsigned_type, arg0)
> + : nargs == 2 ? fold_build2 (m_int_code, unsigned_type, arg0, arg1)
> + : fold_build3 (m_int_code, unsigned_type, arg0, arg1, arg2);
> +
> + ret = fold_build1 (VIEW_CONVERT_EXPR, signed_type, f.force_val (ret));
> +
> + return gimple_build_assign (f.lhs, ret);
> + }
> +};
Btw, I noticed that a pre-existing bug with this infrastructure is that
intrinsics with unused result can fail to fold when we expect them to and end
up calling the unreachable expand. With this patch it’d be:
void f(float32x4_t a, float32x4_t b)
{
vaddq_f32 (a, b);
}
For FP operations they may have CP_READ_FPCR | CP_RAISE_FP_EXCEPTIONS
properties so we need to preserve them when trapping math is in effect.
So it’d be good to fix that up in a pre-req patch before extending the
intrinsics that use this class.
Thanks,
Kyrill
> +
> struct gimple_create : public gimple_function_base
> {
> gimple *fold (gimple_folder &f) const override
> @@ -749,9 +804,10 @@ NEON_FUNCTION (vdupd_lane, gimple_get_lane,)
> NEON_FUNCTION (vdupd_laneq, gimple_get_lane,)
>
> // Lanewise arithmetic
> -NEON_FUNCTION (vaddd, gimple_expr, (PLUS_EXPR))
> -NEON_FUNCTION (vadd, gimple_expr, (PLUS_EXPR, PLUS_EXPR, BIT_XOR_EXPR))
> -NEON_FUNCTION (vaddq, gimple_expr, (PLUS_EXPR, PLUS_EXPR, BIT_XOR_EXPR))
> +// Addition
> +NEON_FUNCTION (vaddd, gimple_arith, (PLUS_EXPR))
> +NEON_FUNCTION (vadd, gimple_arith, (PLUS_EXPR, PLUS_EXPR, BIT_XOR_EXPR))
> +NEON_FUNCTION (vaddq, gimple_arith, (PLUS_EXPR, PLUS_EXPR, BIT_XOR_EXPR))
>
> // Bitwise operations
> NEON_FUNCTION (vand, gimple_expr, (BIT_AND_EXPR))
> diff --git a/gcc/config/aarch64/aarch64-neon-builtins-base.def
> b/gcc/config/aarch64/aarch64-neon-builtins-base.def
> index 7257f59bbc5..e63b98b283b 100644
> --- a/gcc/config/aarch64/aarch64-neon-builtins-base.def
> +++ b/gcc/config/aarch64/aarch64-neon-builtins-base.def
> @@ -63,15 +63,19 @@ DEF_NEON_FUNCTION (vcopyq_laneq, neon_copy_lane,
>
> // Lanewise arithmetic
> #define REQUIRED_EXTENSIONS nonstreaming_only (AARCH64_FL_SIMD)
> +// Addition
> DEF_NEON_FUNCTION (vaddd, d_integer, ("s0,s0,s0"))
> DEF_NEON_FUNCTION (vadd, all_arith_no_fp16, ("D0,D0,D0"))
> -DEF_NEON_FUNCTION (vadd, bhd_poly, ("D0,D0,D0"))
> DEF_NEON_FUNCTION (vaddq, all_arith_no_fp16, ("Q0,Q0,Q0"))
> +
> +// Polynomial "addition" (really xor)
> +DEF_NEON_FUNCTION (vadd, bhd_poly, ("D0,D0,D0"))
> DEF_NEON_FUNCTION (vaddq, bhdq_poly, ("Q0,Q0,Q0"))
> #undef REQUIRED_EXTENSIONS
>
> // Lanewise arithmetic (FP16)
> #define REQUIRED_EXTENSIONS nonstreaming_only (AARCH64_FL_SIMD |
> AARCH64_FL_F16)
> +// Addition
> DEF_NEON_FUNCTION (vadd, h_float, ("D0,D0,D0"))
> DEF_NEON_FUNCTION (vaddq, h_float, ("Q0,Q0,Q0"))
> #undef REQUIRED_EXTENSIONS
> diff --git a/gcc/testsuite/gcc.target/aarch64/neon/vadd.c
> b/gcc/testsuite/gcc.target/aarch64/neon/vadd.c
> index e622718685d..e1bfec03ba0 100644
> --- a/gcc/testsuite/gcc.target/aarch64/neon/vadd.c
> +++ b/gcc/testsuite/gcc.target/aarch64/neon/vadd.c
> @@ -38,6 +38,13 @@ TEST_UNIFORM_BINARY (vadd_u16, uint16x4_t)
> */
> TEST_UNIFORM_BINARY (vadd_s16, int16x4_t)
>
> +/*
> +** test_vadd_f16:
> +** fadd v0\.4h, (v0\.4h, v1\.4h|v1\.4h, v0\.4h)
> +** ret
> +*/
> +TEST_UNIFORM_BINARY (vadd_f16, float16x4_t)
> +
> /*
> ** test_vadd_p16:
> ** eor v0\.8b, (v0\.8b, v1\.8b|v1\.8b, v0\.8b)
> @@ -59,6 +66,13 @@ TEST_UNIFORM_BINARY (vadd_u32, uint32x2_t)
> */
> TEST_UNIFORM_BINARY (vadd_s32, int32x2_t)
>
> +/*
> +** test_vadd_f32:
> +** fadd v0\.2s, (v0\.2s, v1\.2s|v1\.2s, v0\.2s)
> +** ret
> +*/
> +TEST_UNIFORM_BINARY (vadd_f32, float32x2_t)
> +
> /*
> ** test_vadd_u64:
> ** add d0, (d0, d1|d1, d0)
> @@ -73,6 +87,13 @@ TEST_UNIFORM_BINARY (vadd_u64, uint64x1_t)
> */
> TEST_UNIFORM_BINARY (vadd_s64, int64x1_t)
>
> +/*
> +** test_vadd_f64:
> +** fadd d0, (d0, d1|d1, d0)
> +** ret
> +*/
> +TEST_UNIFORM_BINARY (vadd_f64, float64x1_t)
> +
> /*
> ** test_vadd_p64:
> ** eor v0\.8b, (v0\.8b, v1\.8b|v1\.8b, v0\.8b)
> @@ -179,7 +200,7 @@ TEST_UNIFORM_BINARY (vaddq_f64, float64x2_t)
> TEST_UNIFORM_BINARY (vaddq_p64, poly64x2_t)
>
> /* `poly128_t` is a scalar type, like `__uint128_t`, so it is passed in two
> GPR
> - registers. *
> + registers. */
> /*
> ** test_vaddq_p128:
> ** eor x[0-9], x[0-9]+, x[0-9]+
> --
> 2.51.0
>
