On Mon Oct 5, 2026 at 2:26 AM PDT, Roger Sayle 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.
>
> 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?
Ok.
>
>
> 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.
>>
