RE: [x86 SSE PATCH v2] PR target/126619: Improve V4SF vector initialization.

"Liu, Hongtao" <[email protected]>
Newsgroups gmane.comp.gcc.patches
Message-ID <DS4PPF240F42FB7D889309BC3BC615B2F64E5A32@DS4PPF240F42FB7.namprd11.prod.outlook.com>
Hi Roger,

Thanks, this is much better than v1.  The SSE4.1 and above paths are a clear
win everywhere I measured, no regression at all.  A few things below, mostly
small.

> this revised patch has one test suite regression on IA32 caused by a
> difference in register allocation (unrelated to V4SF) that can be fixed
> by a small improvement to the STV pass.

I don't think STV is at fault, it's this hunk:

> -	  " Yr,*x,v,v,m,r ,m,x,v,?jrjm,?jrjm,?rm,!x,?re,!*fF"))
> +	  " Yr,*x,v,v,m,?r,m,x,v,?jrjm,?jrjm,?rm,!x,?re,!*fF"))

VI4F_128 covers V4SI too, and for V4SI the SImode source really does live in
a GPR: on !TARGET_64BIT ix86_expand_vector_init_one_nonzero builds V2DI out
of two force_reg'd SImode halves via gen_vec_setv4si_0.  Once movd is
disparaged LRA gives up on it and goes through the stack:

-	movd	%edx, %xmm0
-	movd	%eax, %xmm1
+	movl	$0, 12(%esp)
+	movd	%xmm0, 8(%esp)
+	movd	12(%esp), %xmm0
+	movd	8(%esp), %xmm1

which is where the %esp that sse2-stv-1.c scans for comes from (and it also
grows a frame).  So can we restrict the disparagement to V4SF, something like

(define_mode_attr vec_set_0_gpr [(V4SI "r ") (V4SF "?r")])
...
	  " Yr,*x,v,v,m,<vec_set_0_gpr>,m,x,v,?jrjm,?jrjm,?rm,!x,?re,!*fF"))

That fixes sse2-stv-1.c here and keeps all of the V4SF improvements, so no
STV change should be needed.

> +	    rtx pat = first_p
> +		      ? gen_sse4_1_insertps_v4sf_init (tmp, val,
> +						       CONST0_RTX (V4SFmode),
> +						       idx)
> +		      : gen_vec_setv4sf_sse4_1 (tmp, tmp, val, idx);

When ops[0] is non-zero, first_p is still true at i == 0, so idx is 1, which
const248_operand rejects.  It works today only because recog matches the
result as vec_set<mode>_0 instead of the pattern you called gen_ for -- the
asm is "insertps $0xe" (vec_set<mode>_0's literal template) rather than the
$14 the new pattern would print.  Since 1 is excluded from const248_operand
on purpose, please just use vec_set<mode>_0 for i == 0, e.g.

	    rtx pat;
	    if (!first_p)
	      pat = gen_vec_setv4sf_sse4_1 (tmp, tmp, val, idx);
	    else if (i == 0)
	      pat = gen_vec_setv4sf_0 (tmp, CONST0_RTX (V4SFmode), val);
	    else
	      pat = gen_sse4_1_insertps_v4sf_init (tmp, val,
						  CONST0_RTX (V4SFmode), idx);

> +(define_insn "sse4_1_insertps_<mode>_init"
> +  [(set (match_operand:VI4F_128 0 "register_operand" "=Yr,*x,v,v")

Nothing ever generates the V4SI form (init_v4si uses pinsrd), so the only
consumer of the V4SI instantiation is combine, and there it's a pessimization:

  v4si f (int a) { v4si z = {0,0,0,0}; z[2] = a; return z; }

  -O2 -msse4.1 -mno-avx
-	pxor	%xmm0, %xmm0
-	pinsrd	$2, %edi, %xmm0
+	movd	%edi, %xmm1
+	insertps	$43, %xmm1, %xmm0

pinsrd reads the GPR directly and the pxor is a zero idiom that breaks the
dependency, while the new form costs exactly the inter-unit move this patch
is trying to get rid of, crosses to the FP domain, and leaves a false
dependency on %xmm0.  Can you make it V4SF only, like vec_setv4sf_sse4_1?
That also makes (set_attr "mode" "V4SF") correct.


So ok for trunk with the mode_attr, the idx == 1 case, the V4SF-only
iterator and vars[0] fixed.

BR,
Hongtao

________________________________________
From: Roger Sayle <[email protected]>
Sent: Thursday, 20 August 2026 13:15:54
To: 'Hongtao Liu'
Cc: 'Patches GCC'; Liu, Hongtao; 'Uros Bizjak'
Subject: [x86 SSE PATCH v2] PR target/126619: Improve V4SF vector initialization.

Hi Hongtao,
Here's a revised version of my previous patch for PR target/126619,
incorporating your feedback and suggestions.  Although x86 SSE vector
initialization is now much improved, it's not yet perfect.  I'd like
to propose performing V16QI initialization improvements in a follow-up
patch.  Likewise, this revised patch has one test suite regression on
IA32 caused by a difference in register allocation (unrelated to V4SF)
that can be fixed by a small improvement to the STV pass.

One thing I'd like to point out at this stage is that use of insertps
and/or pinsr chains is not just an optimization for size (fewer insns),
but these chains also use significantly fewer registers, and theoretically
on modern CPUs do not (need to) have dependency chains.  A sequence of
insertps/pinsr may be executed/scheduled out-of-order [given dependency
tracking by vector element rather than by entire register].  If Intel
and AMD don't already do this, it would be a good use for a few extra
transistors.  I suspect GCC's own schedulers could also be tweaked.

This patch has been tested on x86_64-pc-linux-gnu with make bootstrap
and make -k check, both with and without --target_board=unix{-m32}
with a single new failure (with -m32) of gcc.target/i386/sse2-stv-1.c,
that I'll fix shortly.  Hongtao's suggestion to tweak register
preferencing (to use ? instead of !) isn't at fault/wrong, it just
exposes a latent inefficiency elsewhere.  Ok for mainline?


2026-08-20  Roger Sayle  <[email protected]>
            Hongtao Liu  <[email protected]>

gcc/ChangeLog
        PR target/126619
        * config/i386/i386-expand.cc
        (ix86_expand_vector_init_one_nonzero) <case E_V4SFmode>:
        Improved initialization of one non-zero element V4SF vectors.
        (ix86_expand_vector_init_v4sf): Reuse the above function
        ix86_expand_vector_init_one_nonzero where possible.  Various
        improvements.  Fall back to using (the original)
        ix86_expand_vector_init_concat in the general case, when
        SSE 4.1 instructions aren't available.

        * config/i386/sse.md (vec_set<mode>_0): Prefer to avoid
        inter-unit moves to general purpose registers in reload.
        (vec_setv4sf_sse4_1): Remove asterisk to expose to i386-expand.
        (sse4_1_insertps_<mode>_init): Variant of insertps that clears
        all the other elements of the destination to zero.
        (sse2_insertps_v4sf_3): Implementation of the above instruction
        available on SSE2 by using the pslldq instruction.

gcc/testsuite/ChangeLog
        PR target/126619
        * gcc.target/i386/avx-init-v4sf-1.c: Update test case.
        * gcc.target/i386/avx-init-v4sf-2.c: Likewise.
        * gcc.target/i386/avx2-init-v4sf-1.c: Likewise.
        * gcc.target/i386/sse-init-v4sf-2.c: Likewise.
        * gcc.target/i386/sse-init-v4sf-3.c: Likewise.
        * gcc.target/i386/sse2-init-v4sf-1.c: Likewise.
        * gcc.target/i386/sse2-init-v4sf-2.c: Likewise.
        * gcc.target/i386/sse4_1-init-v4sf-2.c: Likewise.
        * gcc.target/i386/sse4_1-init-v4sf-3.c: Likewise.

Thanks again,
Roger

> -----Original Message-----
> From: Hongtao Liu <[email protected]>
> Sent: 11 August 2026 08:10
> To: Roger Sayle <[email protected]>
> Cc: Patches GCC <[email protected]>; Liu, Hongtao
> <[email protected]>; Uros Bizjak <[email protected]>
> Subject: Re: [x86 SSE PATCH] PR target/126619: Improve V4SF vector
> initialization.
>
> On Sun, Aug 9, 2026 at 6:42 PM Roger Sayle <[email protected]>
> wrote:
> >
> >
> > This patch addresses PR target/126619, a performance regression caused
> > by my recent update of SSE vector initialization on x86_64.  The new
> > idiom attempts to benefit from the implicit zero extension provided by
> > movss and movd, but this causes problems for (V4SF) cases were the
> > (SFmode) value is already in a register.  Without newer extensions,
> > GCC's approach to zero extension (i.e. vec_init of (V4SF){ x, 0, 0, 0
> > }) is to perform an interunit transfer to a general purpose integer
> > register, and then transfer the value back again.  Inter-unit moves
> > are expensive, especially on older microarchitectures.
> >
> > The problem is fixed in several ways.  The first is to tweak register
> > preferencing in vec_set<mode>_0's define_insn, so that general
> > registers are only used if the source/destination is already an integer GPR.
> > This changes reload from generating (two instructions):
> >
> >         movd %xmm0, %eax
> >         movd %eax, %xmm0
> >
> > to instead using:
> >
> >         pxor %xmm1, %xmm1
> >         movss %xmm0, %xmm1
> >         movaps %xmm1, %xmm0
> >
> > which requires 3 instructions, and 1 extra register, but requires no
> > inter-unit moves.  This matches what clang/llvm does.
> >
> > However, it's possible to do better, borrowing an idiom from the
> > middle-end's expansion of integer zero-extensions.
> >
> >         pslldq  $12, %xmm0
> >         psrldq  $12, %xmm0
> >
> > uses two instructions, and doesn't require an extra register.
> > Indeed, initializing the vector (V4SF){ 0, 0, 0, x } can be done in a
> > single instruction, as it doesn't require a "right"
> > shift.
> >
> > Additionally, for cases such as (V4SF){ a, b, c, d }, where there is
> > no benefit from zero extension, we should continue using GCC's
> > original CONCAT of V2SF idiom, avoiding any overhead of zero extension
> > (Hongtao's suggestion in the Bugzilla PR).
> >
> > Additionally, there are some additional V4SF initialization tweaks.
> > When loading from memory, where zero extension is free "onevar_perm"s
> > should construct { x, 0, 0, 0 } then perform a shuffle using shufps,
> > but when the source is a register, it should construct { 0, 0, 0, x }
> > (using the single shift instruction described above), and perform a
> > modified shuffle using shufps from there.
> >
> > With TARGET_SSE4_1, the first insertps can be used to clear
> > (initialize) all the other elements to zero, and the remaining
> > non-zero elements can be inserted with regular insertps.
> >
> I think even with sse4.1 and above we should still use the original
> ix86_expand_vector_concat solution since insertps have sequential dependence
> with depth of 4, but vec_concat only with depth 2.
> so the vector_concat solution should still be faster(including the 4-memory case),
> maybe for optimization for size, insertps is prefered.
>
> >-  else
> >+  else if (TARGET_SSE4_1)
>
> So may just
>  else if (TARGET_SSE4_1 && optimize_insn_for_size_p ())
>   {
>     ......
>   }
> else
>   ix86_expand_vector_init_concat (V4SFmode, target, ops, 4);
>
> >+    {
> >+      int i;
> >+      rtx tmp = gen_reg_rtx (V4SFmode);
> >+      bool first_p = true;
> >...
> >+  else
> >+    ix86_expand_vector_init_concat (V4SFmode, target, ops, 4);
> >}
>
> > ;; see comment above inline_secondary_memory_needed function in
> >i386.cc  (define_insn "vec_set<mode>_0"
> >   [(set (match_operand:VI4F_128 0 "nonimmediate_operand"
> >-          "=Yr,*x,v,v,v,v,x,x,v,Yr ,?x ,x  ,m ,m   ,m")
> >+          "=Yr,*x,v,v,v,!v,x,x,v,Yr ,?x ,x  ,m ,m   ,m")
>          (vec_merge:VI4F_128
>
> Add *?* to the corresponding alternative *r* of operands[2] instead of add *!* to
> *v* of operands[0]?
> my experience is ?r is better to help LRA for register allocation choice.
>
> >+;; Use sse4_1_insertps_v4s[if] to vector_init one non-zero value.
> >+(define_insn "sse4_1_insertps_<mode>_init"
> >+  [(set (match_operand:VI4F_128 0 "register_operand" "=Yr,*x,v")
> >+        (vec_merge:VI4F_128
> >+          (vec_duplicate:VI4F_128
> >+            (match_operand:<ssescalarmode> 1 "nonimmediate_operand"
> "Yrjm,*xjm,vm")
> >+          (match_operand:VI4F_128 2 "const0_operand")
> >+          (match_operand:SI 3 "const248_operand")))]
> >+  "TARGET_SSE4_1"
> >+{
> >+  int op3 = INTVAL (operands[3]);
> >+  operands[3] = GEN_INT ((exact_log2 (op3) << 4) + (op3 ^ 15));
> >+  switch (which_alternative)
> >+    {
> >+    case 0:
> >+    case 1:
> >+      return "insertps\t{%3, %1, %0|%0, %1, %3}";
> >+    case 2:
> >+      return "vinsertps\t{%3, %1, %0, %0|%0, %0, %1, %3}";
>
> Split alternative 2 into v,m and use {%3, %1, %1, %0|%0, %1, %1, %3} for
> alternative *v*, it can avoid false dependence for operands[0].
> Also for the new *m* alternative, can we only enable it when not
> preferred_for_speed since there's false dependence.
>    (set (attr "preferred_for_speed")
>     (cond [(eq_attr "alternative" "3")
>              (symbol_ref "false")]
>           (symbol_ref "true")))
>
> >+    default:
> >+      gcc_unreachable ();
> >+    }
>
> >+      if (TARGET_SSE4_1)
> >+        {
> >+          if (!REG_P (var) && !MEM_P (var))
> >+            var = force_reg (SFmode, var);
>
> -      rtx tmp1 = gen_reg_rtx (V4SFmode);
> -      vars[0] = ops[0];
> -      vars[1] = CONST0_RTX (SFmode);
> -      vars[2] = CONST0_RTX (SFmode);
> -      vars[3] = CONST0_RTX (SFmode);
> -      ix86_expand_vector_init_v4sf (tmp1, vars);
> +      if (TARGET_SSE4_1)
> +        {
> +          rtx tmp = gen_reg_rtx (V4SFmode);
> +          rtx val = ops[0];
> +          if (!REG_P (val) && !MEM_P (val))
> +            val = force_reg (SFmode, val);
> +          emit_insn (gen_vec_setv4sf_0 (tmp, CONST0_RTX (V4SFmode), val));
> +          val = ops[1];
> +          if (!REG_P (val) && !MEM_P (val))
> +            val = force_reg (SFmode, val);
> +          emit_insn (gen_vec_setv4sf_sse4_1 (target, tmp, val, GEN_INT (2)));
> +        }
>
> I think we can generate 1 insertps for {a,b,0,0} {a, 0, b,0}, {a, 0, 0, b}, here still
> generates 2 insertps,  maybe we can directy gen_sse4_1_insertps_v4sf, use
> subreg to convert SFmode to V4SFmode?
>
>
> BR,
> Hongtao
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.