AMD General Hi Richard,
> -----Original Message----- > From: Kumar, Venkataramanan > Sent: Thursday, October 8, 2026 11:47 AM > To: 'Richard Biener' <[email protected]>; [email protected] > Cc: [email protected] > Subject: RE: [PATCH] [x86][v2] umul<mode>3_highpart for VnDI > > Hi Richard, > > > -----Original Message----- > > From: Richard Biener <[email protected]> > > Sent: Monday, October 5, 2026 7:25 PM > > To: [email protected] > > Cc: [email protected] > > Subject: [PATCH] [x86][v2] umul<mode>3_highpart for VnDI > > > > Caution: This message originated from an External Source. Use proper > > caution when opening attachments, clicking links, or responding. > > > > > > This implements umul<mode>3_highpart for V2DI, V4DI and V8DI using > > SSE2 instructions, specifically exercising pmuludq. I wasn't able to > > produce a scalar vectorizer pattern open-coding a sequence that would > > vectorize to this, specifically with the same VF and the setup of the > > input to the pmuludq, so I opted for implementing this in the target. > > > > I added a testcase for a V2DI loop that shouldn't be vectorized and > > one for a V4DI loop that should. > > > > This speeds up 750.sealcrypto_r by ~20% on Zen4 and should recover > > what was lost with r17-3437. > > > > v2: > > - merge the two function costing parts, use SImode multiplication cost, > > VnDImode multiplication cost is not accurate for the SImode widening > > multiplication we want to use > > - add similar costing to the MULT_HIGHPART_EXPR case > > - alternate multiplication sequence, squeezing out a few ops > > > > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress. > > > > I have re-measured this with 750.sealcrypto_r and the alternate > > multiplication sequence is about 1.5% faster, overall it's still a ~20% win. > > > > OK for trunk if testing passes? > > > > Thanks, > > Richard. > > > > PR tree-optimization/127452 > > * config/i386/i386-protos.h (ix86_expand_umulvndi_highpart): > > Declare. > > * config/i386/i386-expand.cc (ix86_expand_umulvndi_highpart): > > New function. > > * config/i386/sse.md (umul<mode>3_highpart): Add for > > VI8_AVX2_AVX512F. > > * config/i386/i386.cc (ix86_vector_costs::add_stmt_cost): Cost > > IFN_MULH as we implement it. > > > > * gcc.target/i386/vect-umulv2di_high.c: New testcase. > > * gcc.target/i386/vect-umulv4di_high.c: Likewise. > > --- > > gcc/config/i386/i386-expand.cc | 67 +++++++++++++++ > > gcc/config/i386/i386-protos.h | 1 + > > gcc/config/i386/i386.cc | 82 +++++++++++-------- > > gcc/config/i386/sse.md | 11 +++ > > .../gcc.target/i386/vect-umulv2di_high.c | 14 ++++ > > .../gcc.target/i386/vect-umulv4di_high.c | 12 +++ > > 6 files changed, 154 insertions(+), 33 deletions(-) create mode > > 100644 gcc/testsuite/gcc.target/i386/vect-umulv2di_high.c > > create mode 100644 gcc/testsuite/gcc.target/i386/vect-umulv4di_high.c > > > > diff --git a/gcc/config/i386/i386-expand.cc > > b/gcc/config/i386/i386-expand.cc index 9e112d01734..a81993c29c5 100644 > > --- a/gcc/config/i386/i386-expand.cc > > +++ b/gcc/config/i386/i386-expand.cc > > @@ -28467,6 +28467,73 @@ ix86_expand_sse2_mulvxdi3 (rtx op0, rtx op1, > > rtx > > op2) > > gen_rtx_MULT (mode, op1, op2)); } > > > > +void > > +ix86_expand_umulvndi_highpart (rtx op0, rtx op1, rtx op2) { > > + /* The following sequence can be used with SSE2. */ > > + machine_mode mode = GET_MODE (op0); > > + rtx (*umul) (rtx, rtx, rtx); > > + machine_mode nmode; > > + if (mode == V2DImode) > > + { > > + umul = gen_vec_widen_umult_even_v4si; > > + nmode = V4SImode; > > + } > > + else if (mode == V4DImode) > > + { > > + umul = gen_vec_widen_umult_even_v8si; > > + nmode = V8SImode; > > + } > > + else if (mode == V8DImode) > > + { > > + umul = gen_vec_widen_umult_even_v16si; > > + nmode = V16SImode; > > + } > > + else > > + gcc_unreachable (); > > + > > + rtx t32 = GEN_INT (32); > > + > > + /* Shift input vectors right 32 bits so we can get high parts of > > + operands. */ rtx op1_hi = expand_binop (mode, lshr_optab, op1, > > + t32, NULL, 1, OPTAB_DIRECT); rtx op2_hi = expand_binop (mode, > > + lshr_optab, op2, t32, NULL, 1, OPTAB_DIRECT); > > + > > + /* Multiply low parts. */ > > + rtx p0 = gen_reg_rtx (mode); > > + emit_insn (umul (p0, gen_lowpart (nmode, op1), gen_lowpart (nmode, > > + op2))); > > + > > + /* Multiply high parts by low parts. */ rtx p1 = gen_reg_rtx > > + (mode); emit_insn (umul (p1, gen_lowpart (nmode, op1_hi), > > + gen_lowpart (nmode, op2))); rtx p2 = gen_reg_rtx (mode); emit_insn > > + (umul (p2, gen_lowpart (nmode, op1), gen_lowpart (nmode, op2_hi))); > > + > > + /* Multiply high parts. */ > > + rtx p3 = gen_reg_rtx (mode); > > + emit_insn (umul (p3, gen_lowpart (nmode, op1_hi), > > + gen_lowpart (nmode, op2_hi))); > > + > > + /* Fold the carry in once cross product at a time. > > + t = (p0 >> 32) + p1 */ > > + rtx p0_hi = expand_binop (mode, lshr_optab, p0, t32, NULL, 1, > > + OPTAB_DIRECT); rtx t = expand_binop (mode, add_optab, p0_hi, p1, > > + NULL, 1, OPTAB_DIRECT); > > + > > + /* Extract low 32 bits of products by masking with 0xffffffff. */ > > + rtx mask = gen_reg_rtx (mode); emit_insn (gen_rtx_SET (mask, > > + CONSTM1_RTX (mode))); mask = expand_binop (mode, lshr_optab, mask, > > + t32, NULL, 1, OPTAB_DIRECT); > > + > > + /* u = (t & MASK) + p2 */ > > + rtx u = expand_binop (mode, and_optab, t, mask, NULL, 1, > > + OPTAB_DIRECT); u = expand_binop (mode, add_optab, u, p2, NULL, 1, > > + OPTAB_DIRECT); > > + > > + /* high = p3 + (t >> 32) + (u >> 32) */ > > + rtx t_hi = expand_binop (mode, lshr_optab, t, t32, NULL, 1, > > +OPTAB_DIRECT); > > + rtx u_hi = expand_binop (mode, lshr_optab, u, t32, NULL, 1, > > +OPTAB_DIRECT); > > + rtx high = expand_binop (mode, add_optab, p3, t_hi, NULL, 1, > > +OPTAB_DIRECT); > > + force_expand_binop (mode, add_optab, high, u_hi, op0, 1, > > +OPTAB_DIRECT); } > > + > > /* Return 1 if control transfer instruction INSN > > should be encoded with notrack prefix. */ > > > > diff --git a/gcc/config/i386/i386-protos.h > > b/gcc/config/i386/i386-protos.h index 4e56bf70c8d..23f82b6239e 100644 > > --- a/gcc/config/i386/i386-protos.h > > +++ b/gcc/config/i386/i386-protos.h > > @@ -310,6 +310,7 @@ extern void ix86_expand_mul_widen_evenodd (rtx, > > rtx, rtx, bool, bool); extern void ix86_expand_mul_widen_hilo (rtx, > > rtx, rtx, bool, bool); extern void ix86_expand_sse2_mulv4si3 (rtx, > > rtx, rtx); extern void > > ix86_expand_sse2_mulvxdi3 (rtx, rtx, rtx); > > +extern void ix86_expand_umulvndi_highpart (rtx, rtx, rtx); > > extern void ix86_expand_sse2_abs (rtx, rtx); extern bool > > ix86_expand_vector_init_duplicate (bool, machine_mode, rtx, > > rtx); diff --git > > a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc index > > e9015d2427e..afe799b87dc 100644 > > --- a/gcc/config/i386/i386.cc > > +++ b/gcc/config/i386/i386.cc > > @@ -26696,7 +26696,16 @@ ix86_vector_costs::add_stmt_cost (int count, > > vect_cost_for_stmt kind, > > /* For MULT_HIGHPART_EXPR, x86 only supports pmulhw, > > take it as MULT_EXPR. */ > > case MULT_HIGHPART_EXPR: > > - stmt_cost = ix86_multiplication_cost (ix86_cost, mode); > > + if (kind == vector_stmt && GET_MODE_INNER (mode) == DImode) > > + { > > + /* ix86_expand_umulvndi_highpart, 4 SImode -> DImode > > + widening multiplies. */ > > + stmt_cost = ix86_multiplication_cost (ix86_cost, SImode) * 4; > > + /* 5 shifts by 32bit, 1 and, 4 adds. */ > > + stmt_cost += ix86_vec_cost (mode, ix86_cost->sse_op) * 10; > > + } > > Is this above cost changes also needed for case MULT_EXPR ?? https://gcc.gnu.org/bugzilla/show_bug.cgi?id=127769 Filed a ticket for missing SLP due to costing MULT_EXPR more. > > > + else > > + stmt_cost = ix86_multiplication_cost (ix86_cost, mode); > > break; > > /* There's no direct instruction for WIDEN_MULT_EXPR, > > take emulation into account. */ @@ -26927,6 +26936,45 @@ > > ix86_vector_costs::add_stmt_cost (int count, vect_cost_for_stmt kind, > > break; > > } > > } > > + else if ((kind == vector_stmt || kind == scalar_stmt) > > + && stmt_info > > + && stmt_info->stmt > > + && is_gimple_call (stmt_info->stmt)) > > + { > > + tree fndecl = gimple_call_fndecl (stmt_info->stmt); > > + cgraph_node *node; > > + combined_fn cfn; > > + if ((fndecl > > + && (node = cgraph_node::get (fndecl)) > > + && node->simd_clones) > > + || gimple_call_internal_p (stmt_info->stmt, IFN_MASK_CALL)) > > + stmt_cost = 10 * ix86_vec_cost (mode, > > + mode == SFmode ? ix86_cost->fmass > > + : ix86_cost->fmasd); > > + else if ((cfn = gimple_call_combined_fn (stmt_info->stmt)) != > > CFN_LAST) > > + switch (cfn) > > + { > > + case CFN_FMA: > > + stmt_cost = ix86_vec_cost (mode, > > + mode == SFmode ? ix86_cost->fmass > > + : ix86_cost->fmasd); > > + break; > > + case CFN_MULH: > > + if (kind == vector_stmt && GET_MODE_INNER (mode) == DImode) > > + { > > + /* ix86_expand_umulvndi_highpart, 4 SImode -> DImode > > + widening multiplies. */ > > + stmt_cost = ix86_multiplication_cost (ix86_cost, SImode) * > > 4; > > + /* 5 shifts by 32bit, 1 and, 4 adds. */ > > + stmt_cost += ix86_vec_cost (mode, ix86_cost->sse_op) * 10; > > + } > > + else > > + stmt_cost = ix86_multiplication_cost (ix86_cost, mode); > > + break; > > + default: > > + break; > > + } > > + } > > > > /* Record number of load/store/gather/scatter in vectorized body. */ > > if (where == vect_body && !m_costing_for_scalar) @@ -27054,38 > > +27102,6 @@ ix86_vector_costs::add_stmt_cost (int count, > vect_cost_for_stmt kind, > > } > > } > > > > - > > - combined_fn cfn; > > - if ((kind == vector_stmt || kind == scalar_stmt) > > - && stmt_info > > - && stmt_info->stmt > > - && is_gimple_call (stmt_info->stmt)) > > - { > > - tree fndecl = gimple_call_fndecl (stmt_info->stmt); > > - cgraph_node *node; > > - if ((fndecl > > - && (node = cgraph_node::get (fndecl)) > > - && node->simd_clones) > > - || gimple_call_internal_p (stmt_info->stmt, IFN_MASK_CALL)) > > - stmt_cost = 10 * ix86_vec_cost (mode, > > - mode == SFmode ? ix86_cost->fmass > > - : ix86_cost->fmasd); > > - else if ((cfn = gimple_call_combined_fn (stmt_info->stmt)) != > > CFN_LAST) > > - switch (cfn) > > - { > > - case CFN_FMA: > > - stmt_cost = ix86_vec_cost (mode, > > - mode == SFmode ? ix86_cost->fmass > > - : ix86_cost->fmasd); > > - break; > > - case CFN_MULH: > > - stmt_cost = ix86_multiplication_cost (ix86_cost, mode); > > - break; > > - default: > > - break; > > - } > > - } > > - > > if (kind == vec_promote_demote) > > { > > int outer_size > > diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md index > > 2b6d7de1f7f..c1e8b256295 100644 > > --- a/gcc/config/i386/sse.md > > +++ b/gcc/config/i386/sse.md > > @@ -17404,6 +17404,17 @@ > > (set_attr "prefix" "orig,vex") > > (set_attr "mode" "<sseinsnmode>")]) > > > > +(define_expand "umul<mode>3_highpart" > > + [(set (match_operand:VI8_AVX2_AVX512F 0 "register_operand") > > + (umul_highpart:VI8_AVX2_AVX512F > > + (match_operand:VI8_AVX2_AVX512F 1 "vector_operand") > > + (match_operand:VI8_AVX2_AVX512F 2 "vector_operand")))] > > + "TARGET_SSE2" > > +{ > > + ix86_expand_umulvndi_highpart (operands[0], operands[1], > > +operands[2]); > > + DONE; > > +}) > > + > > (define_expand "vec_widen_umult_even_v16si<mask_name>" > > [(set (match_operand:V8DI 0 "register_operand") > > (mult:V8DI > > diff --git a/gcc/testsuite/gcc.target/i386/vect-umulv2di_high.c > > b/gcc/testsuite/gcc.target/i386/vect-umulv2di_high.c > > new file mode 100644 > > index 00000000000..ca839983dc9 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/i386/vect-umulv2di_high.c > > @@ -0,0 +1,14 @@ > > +/* { dg-do compile { target { ! ia32 } } } */ > > +/* { dg-options "-O2 -msse2 -mno-avx" } */ > > + > > +#include <stdint.h> > > + > > +void mulu64_high (uint64_t * __restrict r, uint64_t *a, uint64_t *b) > > +{ > > + for (int i = 0; i < 128; ++i) > > + r[i] = ((unsigned __int128)a[i] * (unsigned __int128)b[i]) >> 64; > > +} > > + > > +/* Vectorizing two 64bit unsigned highpart multiplies with 4 pmuludq plus > > + more is slower than two iterations with a mulq each. */ > > +/* { dg-final { scan-assembler-times "pmuludq" 0 } } */ > > diff --git a/gcc/testsuite/gcc.target/i386/vect-umulv4di_high.c > > b/gcc/testsuite/gcc.target/i386/vect-umulv4di_high.c > > new file mode 100644 > > index 00000000000..ef966b4ec07 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/i386/vect-umulv4di_high.c > > @@ -0,0 +1,12 @@ > > +/* { dg-do compile { target { ! ia32 } } } */ > > +/* { dg-options "-O2 -mavx2 -mprefer-vector-width=256" } */ > > + > > +#include <stdint.h> > > + > > +void mulu64_high (uint64_t * __restrict r, uint64_t *a, uint64_t *b) > > +{ > > + for (int i = 0; i < 128; ++i) > > + r[i] = ((unsigned __int128)a[i] * (unsigned __int128)b[i]) >> 64; > > +} > > + > > +/* { dg-final { scan-assembler-times "pmuludq" 4 } } */ > > -- > > 2.51.0 > > Regards, > Venkat. Regards, Venkat.
