On Tue Sep 22, 2026 at 4:07 AM PDT, Richard Biener wrote:
> On Tue, 22 Sep 2026, Hongtao Liu wrote:
>
>> On Sat Sep 19, 2026 at 9:26 PM PDT, H.J. Lu 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.
>>
>> diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
>> index a8edf28f687..db77f0ae929 100644
>> --- a/gcc/config/i386/i386.cc
>> +++ b/gcc/config/i386/i386.cc
>> @@ -27109,6 +27109,18 @@ ix86_vector_costs::add_stmt_cost (int count,
>> vect_cost_for_stmt kind,
>> if (stmt_cost == -1)
>> stmt_cost = ix86_default_vector_cost (kind, mode);
>>
>> + /* Charge the xmm->gpr move ending an integer basic-block reduction, like
>> + add_slp_cost does for the gpr->xmm direction. It is on the critical
>> + path and the scalar form can be reassociated instead. */
>> + if (kind == vec_to_scalar
>> + && !node
>> + && where == vect_body
>> + && vectype
>> + && !fp
>> + && m_vinfo
>> + && is_a <bb_vec_info> (m_vinfo))
>> + stmt_cost += COSTS_N_INSNS (ix86_cost->integer_to_sse) / 2;
>>
>> I'm think something like this,
>> NOTE: currently vect_is_reduction failed to detect reduction in BB
>> vectorization since reduc_indx is only set in the loop vectorization.
>> There're 2 cases for BB vectorization with vec_to_scalar, one is reduction,
>> another is lane extraction, to exclude lane extraction, I add !node to the
>> condition. Maybe middle-end should also set reduc_idx for BB vectorization?
>
> The thing is the actual reduction is not represented with SLP nodes, it
> is represented by the SLP instance kind and carried out via
> vectorize_slp_instance_root_stmt and analyzed/costed in
> vectorizable_bb_reduc_epilogue - the costs notably do not have a SLP node
> associated with them.
>
> The costing of the reduction assumes we're doing a pow2 reduction,
> combining upper/lower halves until we reach 1 element and then
> extract lane zero (vec_to_scalar).
>
> I do not have a good idea for a proper representation of the reduction
> as SLP node, but we can in principle mark the stmt_info with a flag
> like STMT_VINFO_BB_REDUCTION_RESULT.
>
> I'll also note that in principle all vec_to_scalar should now move
> from the vector to the scalar (and thus for integer to GPR) domain.
> Some might possibly end up in memory. The main use should be for
> live lane extraction btw (that gets attributed to vect_epilogue
> though - for BB vect the "place" shouldn't really matter, but we
> should use vect_body consistently here I guess, I'll post a patch
> to fix that up).
>
> So I wonder whether you tried to add this cost to all vect_to_scalar ops?
Just try to figure out a reasonable solution to fix the issue which H.J tried
to solve.
The original testcase is
extern unsigned char *var1;
extern unsigned short var2;
void
func (void)
{
var2 = var1[1] + var1[0];
}
>
> Richard.
>
>>
>> >> >
>> >> > > > >
>> >> > > > > > + 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.
>>
>>
>
> --
> Richard Biener <[email protected]>
> SUSE Software Solutions Germany GmbH,
> Frankenstrasse 146, 90461 Nuernberg, Germany;
> GF: Stefan Gaiser, Jochen Jaser, Abhinav Puri; (HRB 36809, AG Nuernberg)
