On Sun, Sep 20, 2026 at 6:27 AM H.J. Lu <[email protected]> wrote:
>
> On Wed, Sep 16, 2026 at 4:19 PM Hongtao Liu <[email protected]> wrote:
> >
> > On Wed, Sep 16, 2026 at 1:29 PM H.J. Lu <[email protected]> wrote:
> > >
> > > On Mon, Sep 14, 2026 at 7:59 PM Richard Biener
> > > <[email protected]> wrote:
> > > >
> > > > On Mon, Sep 14, 2026 at 11:52 AM Liu, Hongtao <[email protected]>
> > > > wrote:
> > > > >
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Liu, Hongtao <[email protected]>
> > > > > > Sent: Monday, September 14, 2026 5:45 PM
> > > > > > To: H.J. Lu <[email protected]>; GCC Patches
> > > > > > <[email protected]>;
> > > > > > Uros Bizjak <[email protected]>
> > > > > > Subject: RE: [PATCH v3] x86: Adjust cost for vector integer
> > > > > > conversion without
> > > > > > SSE4.1
> > > > > >
> > > > > > > + int scale = 0;
> > > > > > > + if (scalar_p)
> > > > > > > + {
> > > > > > > + if (stmt_info
> > > > > > > + && stmt_info->stmt
> > > > > > > + && gimple_code (stmt_info->stmt) == GIMPLE_ASSIGN)
> > > > > > > + {
> > > > > > > + tree lhs = gimple_assign_lhs (stmt_info->stmt);
> > > > > > > + if (TREE_CODE (lhs) == ARRAY_REF)
> > > > > > > + {
> > > > > > > + tree index = TREE_OPERAND (lhs, 1);
> > > > > > > + /* Two more instructions are needed to access
> > > > > > > non-zero
> > > > > > > + array element. */
> > > > > > > + if (TREE_CODE (index) != INTEGER_CST
> > > > > > > + || compare_tree_int (index, 0) != 0)
> > > > > > > + scale = 2;
> > > > > > > + }
> > > > > > > + }
> > > > > > > + }
> > > > > >
> > > > > > Not fully understand this part, why access non-zero array elements
> > > > > > should be
> > > > > > penalized with additional cost, is it testcase specific or it's
> > > > > > general?
> > >
> > > Without it, I got
> > >
> > > FAIL: gcc.target/i386/pr111023-2.c scan-assembler (?:pcmpgtd|psrad)
> > > FAIL: gcc.target/i386/pr111023-2.c scan-assembler punpckldq
> > > FAIL: gcc.target/i386/pr111023-4b.c scan-assembler punpcklbw
> > > FAIL: gcc.target/i386/pr111023-8b.c scan-assembler punpcklwd
> > > FAIL: gcc.target/i386/pr111023.c scan-assembler punpckldq
> > >
> > > It is
> > >
> > > pxor %xmm1, %xmm1
> > > punpckldq %xmm1, %xmm0
> > > movaps %xmm0, (%rdi)
> > >
> > > vs
> > >
> > > movd %xmm0, %eax
> > > pshufd $85, %xmm0, %xmm0
> > > movd %xmm0, %edx
> > > movq %rax, (%rdi)
> > > movq %rdx, 8(%rdi)
> >
> > After increase cost for conversion w/o sse4.1, 2-element vectorization
> > in pr111023-*.c testcases now failed, to fix the regression you're
> > adding cost to the scalar part to make it vectorized again. But the
> > cost added is testcase specific, in general I didn't see addition cost
> > is needed for non-zero array elements.
> > The edge case we're currently facing is that vectorization of some
> > 2-element operations is still worthwhile, while for others it's not
> > cost-effictive. we currently have no good way to fine-tune thsese
> > scenarios by adjusting the per-statement cost.
> > One possible solution would be to introduce some heuristics -- trackng
> > the number of loads,stores,general ops, and in finish_costs, lower the
> > scalar cost when the vectorization case is very small. But it's still
> > uncerntain whether this would just be a whack-a-mole fix, where
> > solving one problem inadvertently creates another.
> > >
> > > > > >
> > > > > > > + else if (vector_int_conversion_p && !TARGET_SSE4_1)
> > > > > > > + {
> > > > > > > + /* Two instructions are needed to emulate vector integer
> > > > > > > + conversions. One more instruction is needed to emulate
> > > > > > > + 2-element vector conversion of QImode and HImode. */
> > > > > > > + machine_mode rhs_mode = TYPE_MODE (rhs_type);
> > > > > > > + rhs_mode = GET_MODE_INNER (rhs_mode);
> > > > > > > + scale = (rhs_mode == SImode || GET_MODE_NUNITS (mode) != 2
> > > > > > > + ? 2 : 3);
> > > > > > > + }
> > > > > >
> > > > > > It's reasonable to increase cost for signed conversion w/o sse4.1,
> > > > > > but for
> > > > > > unsigned conversion it's not needed, also I don't think
> > > > > > GET_MODE_NUNITS
> > > > > > (mode) == 2 makes difference from 4/8, it looks tricky.
> > >
> > > Without SSE4.1, ix86_expand_sse_extend generates more instructions for
> > > unsigned vector extension for
> > >
> > > extern unsigned char *var1;
> > > extern unsigned short var2;
> > >
> > > void
> > > func (void)
> > > {
> > > var2 = var1[1] + var1[0];
> > > }
> > >
> > > movq var1(%rip), %rax
> > > pxor %xmm1, %xmm1
> > > pinsrw $0, (%rax), %xmm0
> > > punpcklbw %xmm1, %xmm0
> >
> >
> > pinsrw is vector load for v2qi, the unsigned convert is pxor +
> > punpcklbw, pxor is cheap and can be hoist out of loop/hotspot, so the
> > meaningful comparison between w/ sse4.1 and w/o sse4.1 for unsigned
> > conversion is
> > punpcklbw vs pmovzxbw, no additional cost is needed.
>
> Here is the v4 patch to adjust costs of scalar BIT_FIELD_REF with
> non-zero offset and moving from vector to scalar as well as move
> 2 vector elements to 2 scalars.
>
> --
> H.J.
> ---
> Adjust the costs for scalar statements of vector variable and non-zero
> offset of BIT_FIELD_REF access as well as vector integer conversion
> without SSE4.1. For
>
> extern char *var1;
> extern int var2;
>
> void
> func (void)
> {
> var2 = var1[1] + var1[0];
> }
>
> we generate
>
> movq var1(%rip), %rdx
> movsbl 1(%rdx), %eax
> movsbl (%rdx), %edx
> addl %edx, %eax
> movl %eax, var2(%rip)
>
> instead of
>
> movq var1(%rip), %rax
> pxor %xmm1, %xmm1
> pinsrw $0, (%rax), %xmm0
> pcmpgtb %xmm0, %xmm1
> punpcklbw %xmm1, %xmm0
> movdqa %xmm0, %xmm1
> psraw $15, %xmm1
> punpcklwd %xmm1, %xmm0
> movd %xmm0, %edx
> pshufd $0xe5, %xmm0, %xmm2
> movd %xmm2, %eax
> addl %edx, %eax
> movl %eax, var2(%rip)
>
> with -O2 -march=x86-64. With SSE4.1 enabled, we generate
>
> movq var1(%rip), %rax
> pinsrw $0, (%rax), %xmm0
> pmovsxbd %xmm0, %xmm0
> movd %xmm0, %edx
> pextrd $1, %xmm0, %eax
> addl %edx, %eax
> movl %eax, var2(%rip)
>
> Also update gcc.target/i386/pr125026.c since we now generate
>
> movd %eax, %xmm1
> pshufd $0xe0, %xmm1, %xmm0
> movq %xmm0, 24(%rsp)
>
> instead of
>
> movl %eax, 24(%rsp)
> movl %eax, 28(%rsp)
The last one looks worse then?
+ if (stmt_info
+ && stmt_info->stmt
+ && is_gimple_assign (stmt_info->stmt))
+ {
+ tree rhs = gimple_assign_rhs1 (stmt_info->stmt);
+ tree var = rhs;
+ if (gimple_assign_rhs_code (stmt_info->stmt)
+ == BIT_FIELD_REF)
this should model scalar lane extraction cost from a vector? Why
add to ix86_default_vector_cost rather than handling this fully
in the earlier switch over RHS stmt code?
For AVX/AVX512 there might be additional cost for accessing
the correct 128bit part, though possibly shared between multiple
scalar stmts.
+ /* Additional instruction to move from vector. */
+ if (VECTOR_TYPE_P (TREE_TYPE (var)))
+ scale++;
but not if FP mode?
+ else if (vector_int_conversion_p && !TARGET_SSE4_1)
+ {
+ machine_mode rhs_mode = TYPE_MODE (rhs_type);
+ rhs_mode = GET_MODE_INNER (rhs_mode);
+ if (rhs_mode != SImode)
+ {
+ /* Need an extra instruction without SSE4.1. */
+ scale = 1;
+ /* Need two more instructions for signed conversion. */
+ if (vectype && !TYPE_UNSIGNED (vectype))
+ scale += 2;
similarly - as we have vec_fp_conversion_cost why not have
vec_int_conversion_cost as well? I'll also note the vectorizer
will eventually cost multiple steps for, say char -> int so you
get this multiple times. For such "complex" operations handling
them in add_slp_cost might be easier (for the vector side), as
you get to cover the "whole" operation there.
+ /* Need an extra instruction to move 2 vector elements to 2
+ scalars. */
+ else if (kind == vec_to_scalar && GET_MODE_NUNITS (mode) == 2)
+ scale = 1;
what's this about?
> gcc/
>
> PR target/126802
> * config/i386/i386.cc (ix86_vector_costs::add_stmt_cost): Adjust
> the costs for scalar statements of vector variable and non-zero
> offset of BIT_FIELD_REF access as well as vector integer conversion
> without SSE4.1.
>
> testsuite/
>
> PR target/126802
> * gcc.target/i386/pr125026.c: Updated.
> * gcc.target/i386/pr126802-1a.c: New test.
> * gcc.target/i386/pr126802-1b.c: Likewise.
> * gcc.target/i386/pr126802-1c.c: Likewise.
> * gcc.target/i386/pr126802-2a.c: Likewise.
> * gcc.target/i386/pr126802-2b.c: Likewise.
> * gcc.target/i386/pr126802-2c.c: Likewise.
> * gcc.target/i386/pr126802-3a.c: Likewise.
> * gcc.target/i386/pr126802-3b.c: Likewise.
> * gcc.target/i386/pr126802-3c.c: Likewise.
> * gcc.target/i386/pr126802-4a.c: Likewise.
> * gcc.target/i386/pr126802-4b.c: Likewise.
> * gcc.target/i386/pr126802-4c.c: Likewise.
> * gcc.target/i386/pr126802-5a.c: Likewise.
> * gcc.target/i386/pr126802-5b.c: Likewise.
> * gcc.target/i386/pr126802-5c.c: Likewise.
> * gcc.target/i386/pr126802-6a.c: Likewise.
> * gcc.target/i386/pr126802-6b.c: Likewise.
> * gcc.target/i386/pr126802-6c.c: Likewise.
> * gcc.target/i386/pr126802-7a.c: Likewise.
> * gcc.target/i386/pr126802-7b.c: Likewise.
> * gcc.target/i386/pr126802-7c.c: Likewise.
