Re: [PATCH] vect: Recognise high-part multiply when the wide type has no vector form

> On 6 Aug 2026, at 10:36, Richard Biener <[email protected]> wrote:
> 
> On Thu, 6 Aug 2026, [email protected] wrote:
> 
>> From: Kyrylo Tkachov <[email protected]>
>> 
>> vect_recog_mulhs_pattern turns (a * b) >> N into IFN_MULH and casts the
>> result back to the type of the shift, which the over-widening machinery
>> then drops.  Building that cast needs a vector type for the wide type, and
>> the pattern gives up when there is none.
>> 
>> For a 64-bit high-part multiply the wide type is 128 bits, and no target
>> has a vector of 128-bit integers, so on AArch64 the pattern never fires
>> for scalable SVE and the loop stays scalar.  It does fire under
>> -msve-vector-bits=128, where the prevailing vector size is 128 bits and a
>> one-element TImode vector exists, which is what makes the failure specific
>> to variable-length vectors.
>> 
>> The pattern has already established that the users of the result need at
>> most min_output_precision bits and that NEW_TYPE holds at least that many,
>> so when the wide vector type is unavailable we can simply leave the result
>> at NEW_TYPE precision and skip the cast.
>> 
>> for (int i = 0; i < n; i++)
>>   d[i] = (uint64_t) (((unsigned __int128) a[i] * b[i]) >> 64);
>> 
>> on -march=armv8.2-a+sve2 now gives
>> 
>> ld1d z28.d, p7/z, [x1, x6, lsl 3]
>> ld1d z27.d, p7/z, [x2, x6, lsl 3]
>> umulh z28.d, z28.d, z27.d
>> st1d z28.d, p7, [x0, x6, lsl 3]
>> 
>> instead of a scalar loop.
>> 
>> Bootstrapped and regression-tested on aarch64-unknown-linux-gnu and
>> x86_64-linux.
>> 
>> Ok for trunk?
>> Thanks,
>> Kyrill
>> 
>> gcc/ChangeLog:
>> 
>> * tree-vect-patterns.cc (vect_recog_mulhs_pattern): Keep the
>> pattern result at NEW_TYPE precision when LHS_TYPE has no vector
>> type.
>> 
>> gcc/testsuite/ChangeLog:
>> 
>> * gcc.target/aarch64/sve/mul_highpart_scalable_1.c: New test.
>> * gcc.target/aarch64/sve/mul_highpart_scalable_run.c: New test.
>> 
>> Signed-off-by: Kyrylo Tkachov <[email protected]>
>> ---
>> .../aarch64/sve/mul_highpart_scalable_1.c     | 23 ++++++++++
>> .../aarch64/sve/mul_highpart_scalable_run.c   | 42 +++++++++++++++++++
>> gcc/tree-vect-patterns.cc                     | 17 ++++++--
>> 3 files changed, 78 insertions(+), 4 deletions(-)
>> create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/mul_highpart_scalable_1.c
>> create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/mul_highpart_scalable_run.c
>> 
>> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/mul_highpart_scalable_1.c b/gcc/testsuite/gcc.target/aarch64/sve/mul_highpart_scalable_1.c
>> new file mode 100644
>> index 00000000000..f61d235b7a8
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/aarch64/sve/mul_highpart_scalable_1.c
>> @@ -0,0 +1,23 @@
>> +/* A 64-bit high-part multiply has a 128-bit product type, and there is no
>> +   scalable vector of 128-bit integers.  Check that the vectoriser recognises
>> +   the high-part multiply anyway.  */
>> +/* { dg-do assemble { target aarch64_asm_sve2_ok } } */
>> +/* { dg-options "-O2 -ftree-vectorize -march=armv8.2-a+sve2 -mautovec-preference=sve-only -msve-vector-bits=scalable --save-temps" } */
>> +
>> +#include <stdint.h>
>> +
>> +#define DEF_LOOP(TYPE, WIDE) \
>> +  void __attribute__ ((noipa)) \
>> +  mulh_##TYPE (TYPE *restrict dst, TYPE *restrict a, \
>> +        TYPE *restrict b, int count) \
>> +  { \
>> +    for (int i = 0; i < count; ++i) \
>> +      dst[i] = (TYPE) (((WIDE) a[i] * b[i]) >> 64); \
>> +  }
>> +
>> +DEF_LOOP (int64_t, __int128)
>> +DEF_LOOP (uint64_t, unsigned __int128)
>> +
>> +/* { dg-final { scan-assembler-times {\tsmulh\tz[0-9]+\.d, z[0-9]+\.d, z[0-9]+\.d\n} 1 } } */
>> +/* { dg-final { scan-assembler-times {\tumulh\tz[0-9]+\.d, z[0-9]+\.d, z[0-9]+\.d\n} 1 } } */
>> +/* { dg-final { scan-assembler {\twhilelo\t} } } */
>> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/mul_highpart_scalable_run.c b/gcc/testsuite/gcc.target/aarch64/sve/mul_highpart_scalable_run.c
>> new file mode 100644
>> index 00000000000..d856302d7a1
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/aarch64/sve/mul_highpart_scalable_run.c
>> @@ -0,0 +1,42 @@
>> +/* { dg-do run { target aarch64_sve_hw } } */
>> +/* { dg-options "-O2 -ftree-vectorize -march=armv8.2-a+sve2 -mautovec-preference=sve-only -msve-vector-bits=scalable" } */
>> +
>> +#include "mul_highpart_scalable_1.c"
>> +
>> +#define N 77
>> +
>> +static int64_t sa[N], sb[N], sd[N];
>> +static uint64_t ua[N], ub[N], ud[N];
>> +
>> +int
>> +main (void)
>> +{
>> +  uint64_t s = 0x243f6a8885a308d3ULL;
>> +  for (int i = 0; i < N; ++i)
>> +    {
>> +      s ^= s << 13; s ^= s >> 7; s ^= s << 17;
>> +      sa[i] = (int64_t) s;
>> +      ua[i] = s;
>> +      s ^= s << 13; s ^= s >> 7; s ^= s << 17;
>> +      sb[i] = (int64_t) s;
>> +      ub[i] = s;
>> +    }
>> +  sa[0] = INT64_MIN; sb[0] = INT64_MIN;
>> +  sa[1] = INT64_MIN; sb[1] = -1;
>> +  sa[2] = -1; sb[2] = -1;
>> +  ua[0] = 0; ub[0] = ~0ULL;
>> +  ua[1] = ~0ULL; ub[1] = ~0ULL;
>> +  ua[2] = 1ULL << 63; ub[2] = 1ULL << 63;
>> +
>> +  mulh_int64_t (sd, sa, sb, N);
>> +  mulh_uint64_t (ud, ua, ub, N);
>> +
>> +  for (int i = 0; i < N; ++i)
>> +    {
>> +      if (sd[i] != (int64_t) (((__int128) sa[i] * sb[i]) >> 64))
>> + __builtin_abort ();
>> +      if (ud[i] != (uint64_t) (((unsigned __int128) ua[i] * ub[i]) >> 64))
>> + __builtin_abort ();
>> +    }
>> +  return 0;
>> +}
>> diff --git a/gcc/tree-vect-patterns.cc b/gcc/tree-vect-patterns.cc
>> index c57e215be57..9934ab8306a 100644
>> --- a/gcc/tree-vect-patterns.cc
>> +++ b/gcc/tree-vect-patterns.cc
>> @@ -3412,11 +3412,20 @@ vect_recog_mulhs_pattern (vec_info *vinfo,
>>     (ifn, new_vectype, OPTIMIZE_FOR_SPEED))
>>     return NULL;
>> 
>> -  /* The IR requires a valid vector type for the cast result, even though
>> -     it's likely to be discarded.  */
>> +  /* The IFN result is cast back to LHS_TYPE, a cast that the over-widening
>> +     machinery then drops.  Some targets have no vector form of LHS_TYPE at
>> +     all: a 64-bit high-part multiply has a 128-bit LHS_TYPE, and there is no
>> +     vector of 128-bit integers.  Keep the pattern result at NEW_TYPE
>> +     precision in that case.  This is safe because the check at the top of the
>> +     function proved that the users of the result need at most
>> +     TARGET_PRECISION bits, and NEW_TYPE holds at least that many.  */
> 
> Huh, but this is not the type of the IFN, neither for scalar nor
> for the vector case?
> 
> I'll note that anticipating removal of a cast is OK, just make sure
> to not specify a vector type for a pattern stmt that doesn't have one
> (it's technically not necessary, we'll re-recognize one or refuse to
> vectorize if there is none later).

I’ve sent an updated patch (I hard reworked it earlier to add a generic test case as per Andrea’s feedback) with a reworked comment.
Thanks,
Kyrill

> 
>> +  tree out_type = lhs_type;
>>   *type_out = get_vectype_for_scalar_type (vinfo, lhs_type);
>>   if (!*type_out)
>> -    return NULL;
>> +    {
>> +      out_type = new_type;
>> +      *type_out = new_vectype;
>> +    }
>> 
>>   /* Generate the IFN_MULHRS call.  */
>>   tree new_var = vect_recog_temp_ssa_var (new_type, NULL);
>> @@ -3432,7 +3441,7 @@ vect_recog_mulhs_pattern (vec_info *vinfo,
>>     dump_printf_loc (MSG_NOTE, vect_location,
>>      "created pattern stmt: %G", (gimple *) mulhrs_stmt);
>> 
>> -  return vect_convert_output (vinfo, last_stmt_info, lhs_type,
>> +  return vect_convert_output (vinfo, last_stmt_info, out_type,
>>       mulhrs_stmt, new_vectype);
>> }
>> 
>> 
> 
> -- 
> Richard Biener <[email protected]>
> SUSE Software Solutions Germany GmbH,
> Frankenstrasse 146, 90461 Nuernberg, Germany;
> GF: Jochen Jaser, Andrew McDonald, Abhinav Puri; (HRB 36809, AG Nuernberg)