On Mon, Oct 5, 2026 at 11:27 AM Roger Sayle <[email protected]> wrote:
>
> Hi Hongtao,
> Sorry for the delay in getting back to you with your suggested
> corrections.  Please find a revised version of the previous patch
> implementing the bug-fix (typo) you spotted, and various whitespace
> issue corrections.
>
> This revision retains the all_same_p terminating case for the
> recursion, that you proposed could perhaps be removed, because
> although this path is not currently triggered (as explained not
> all of the possible optimizations are implemented yet), it's
> better practice to keep the terminating conditions explicit in
> a recursive function, than require an (undocumented) invariant
> of all the function's callers.
>
> Your insightful comment:
> > We can stop here until we find real opportunity in real workload.
> led me to investigate the opportunities for improvement.
>
> Using the following hack (not a proposed patch):
>
> diff --git a/gcc/config/i386/i386-expand.cc b/gcc/config/i386/i386-expand.cc
> index 9e112d01734..8e6942458d9 100644
> --- a/gcc/config/i386/i386-expand.cc
> +++ b/gcc/config/i386/i386-expand.cc
> @@ -20550,6 +20550,49 @@ ix86_expand_vector_init (bool mmx_ok, rtx target, 
> rtx vals)
>        gcc_unreachable ();
>      }
>
> +  unsigned int ni = 0;
> +  unsigned int ai = 0;
> +  unsigned char buffer[128];
> +  for (i = 0; i < n_elts; i++)
> +    {
> +      x = XVECEXP (vals, 0, i);
> +      if (x == CONST0_RTX (inner_mode))
> +       buffer[i] = '0';
> +      else
> +       {
> +         int j = 0;
> +         while (j < i)
> +           {
> +             if (rtx_equal_p (x, XVECEXP (vals, 0, j)))
> +               {
> +                 buffer[i] = buffer[j];
> +                 break;
> +               }
> +             j++;
> +           }
> +
> +         if (j == i)
> +           {
> +             if (CONST_SCALAR_INT_P (x)
> +                 || CONST_DOUBLE_P (x)
> +                 || CONST_FIXED_P (x))
> +               {
> +                 ni++;
> +                 buffer[i] = (ni < 10) ? ni + '0' : '_';
> +               }
> +             else
> +               {
> +                 buffer[i] = (ai < 26) ? ai + 'a' : '_';
> +                 ai++;
> +               }
> +           }
> +       }
> +    }
> +  buffer[n_elts] = '\0';
> +  FILE *fp = fopen("/tmp/gcc.log","a");
> +  fprintf(fp,"%s: %s\n",mode_name[mode],buffer);
> +  fclose(fp);
> +
>    for (i = 0; i < n_elts; ++i)
>      {
>        x = XVECEXP (vals, 0, i);
>
> I was able analyze all the vector initializations in stage 2 and 3 of
> a gcc bootstrap, which produced the following histogram:
>
>    9621 V2DI: ab
>    1570 V2SI: ab
>    1164 V2SF: ab
>     950 V2DF: ab
>     796 V2HI: ab
>     722 V2DI: a0
>     619 V4SI: abcd
>     463 V2DI: 0a
>     301 V4SI: a000
>     294 V4SF: abcd
>     279 V2SI: aa
>     146 V2SF: aa
>     144 V4DF: abcd
>     123 V4SI: ab00
>     100 V2DF: aa
>      98 V4SF: aaaa
>      56 V8SF: abaaaaaa
>      56 V8SF: aabcdefg
>      50 V4SF: abaa
>      50 V4SF: aabc
>      48 V2DI: 1a
>      40 V4DF: abaa
>      40 V4DF: aabc
>      40 V4DF: aaaa
>      33 V2SI: a0
>      24 V8SF: abcdefgh
>      22 V4SI: aabc
>      21 V2SI: 0a
>      20 V4HI: abcd
>      18 V2TI: ab
>      18 V2DI: a1
>      16 V4SI: 000a
>      12 V4SI: abbb
>      12 V4SI: a111
>      12 V4SI: 100a
>      12 V4HI: abbb
>      12 V2HI: aa
>      12 V16QI: abcdefghijklmnop
>       9 V4SI: aabb
>       6 V4SI: aaab
>       6 V4SI: a120
>       4 V4TI: abcd
>       4 V4QI: 00ab
>       4 V4DI: abcd
>       4 V2SI: a1
>       4 V2SI: 1a
>       3 V4SI: abbc
>       2 V4SI: a123
>       2 V4SI: 1a00
>       2 V4QI: aabb
>       2 V4QI: 1ab2
>
> Confirming that you're right, and my ideas for optimizations such as
> implementing (v16qi){a,a,a,a,a,a,0,0,0,0,0,0,0,0,0,0} in two instructions
> using a vpbroadcastb followed by psrldq would probably have no significant
> impact in the real world.  There are some interesting opportunities, and
> one surprising insight is that GCC appears to make heavy use of "short"
> vectors such as "V2SI" and "V4QI" (not the 128, 256 or 512-bit vectors
> supported by the hardware), which was a surpise to me.

That's BB vectorization where of cours two-lane opportunities happen
most often.  In some places the vectorizer is also doing vector
construction from vectors, aka V4SI from two V2SI parts (some targets
have such vec_init expanders, I think x86 is one of those), falling
back to intiializing V4SI as V2DI initialized from two DImode regs.

That might hint at more advanced "patterns" being compositions of
smaller vector patterns we implement.  Not sure if it makes sense
to divide-and-conquer as "fallback" for Vn<mode> with n > 2,
looking whether we can initialize Vn/2<mode> in a special way and
then combine both halves?  (or maybe we do that already)

> If anyone would like to post/send me the results of running the above hack
> on SPEC, the linux kernel or a code base of their choice (note it isn't
> threadsafe, so make -j1), I'd be happy to prioritize any interesting/
> frequent vector patterns.
>
>
> 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 no new failures.  Ok for mainline?
>
>
> 2026-10-05  Roger Sayle  <[email protected]>
>             Hongtao Liu  <[email protected]>
>
> gcc/ChangeLog
>         * config/i386/i386-expand.cc (ix86_expand_vector_init_insert)
>         <case E_V16QImode>: We don't need to zero extend var, if the
>         high bytes will be overwritten.
>         <case E_V8HImode>: Likewise for V8HImode.  Whitespace fix.
>         (ix86_expand_vector_init_qi4): New helper function to effectively
>         load a V4QImode vector into an SImode register or constant.
>         (ix86_expand_vector_init_qi8): Likewise, to effectively load
>         a V8QImode vector into a DImode register or constant.
>         (ix86_expand_vector_init_v16qi): Change return type to void,
>         and never fail.  Fallback to V2DI on TARGET64 and V4SI otherwise
>         using the above helper functions.
>         (ix86_expand_vector_init_general): Update call to
>         ix86_expand_vector_init_v16qi now that it can't fail.
>
> gcc/testsuite/ChangeLog
>         * gcc.target/i386/avx2-init-v16qi-1.c: New test case.
>         * gcc.target/i386/avx-init-v16qi-1.c: Add more test vectors.
>         * gcc.target/i386/sse-init-v16qi-1.c: Likewise.
>         * gcc.target/i386/sse2-init-v16qi-2.c: Likewise.
>         * gcc.target/i386/sse2-init-v16qi-3.c: Likewise.
>         * gcc.target/i386/sse4_1-init-v16qi-1.c: Likewise.
>
>
> Thanks again,
> Roger
> --
>
> > -----Original Message-----
> > From: Hongtao Liu <[email protected]>
> > Sent: 08 September 2026 04:45
> > Subject: Re: [PATCH] Improve V16QI vector initialization on x86 SSE.
> >
> > On Wed, Sep 2, 2026 at 11:01 PM Roger Sayle <[email protected]>
> > wrote:
> > >         vpinsrb $1, %esi, %xmm0, %xmm0
> > >         vpbroadcastw    %xmm0, %xmm0
> > >         ret
> > >
> > > bar:    vmovdqa .LC0(%rip), %xmm0
> > >         vpinsrb $0, %edi, %xmm0, %xmm0
> > >         vpinsrb $8, %edi, %xmm0, %xmm0
> > >         ret
> > >
> > > There's still more that can be done/improved, but I stopped here
> > > (1) because the patch is already quite large and (2) I'm not sure if
> > > the final bar above is optimal (yes in number of instructions), or if
> > > (v2di){0x12345678abcdefLL,0} is better written as movabs+movq?
> > We can stop here until we find real opportunity in real workload.
> >
> > >+  for (i = 0; i < 4; i++)
> > >+    if (CONST_INT_P (ops[i]) && ops[i] != const0_rtx)
> > >+      {
> > >+ val += (UINTVAL (ops[0]) & 0xff) << (i*8);
> >
> > typo, should be ops[i], not ops[0]?
> > >+ vars[i] = const0_rtx;
> > >+      }
> >
> > >+  bool all_same_p = true;
> > >+  for (i = 1; i < 16; i++)
> > >+    if (!rtx_equal_p (ops[i], ops[0]))
> > >+      {
> > >+        all_same_p = false;
> > >+        break;
> > >+      }
> > >+  if (all_same_p)
> > We already handle all_same_p in ix86_expand_vector_init before it reaches
> > _general, and I didn't see any place with recursively call which needs to 
> > handle
> > all_same_p, so perhaps just remove the code for all_same_p.
> >
>

Reply via email to