On Wed, 11 Feb 2026, Andrew Pinski wrote:
> On Wed, Feb 11, 2026 at 2:09 PM Roger Sayle <[email protected]>
> wrote:
> >
> > This patch is my proposed fix for (the regression aspects of) PR123238,
> > a code quality regression on x86_64 triggered by the generation of
> > VCOND_MASK. The regression is actually just bad luck. From gimple,
> > VCOND_MASK(a==b,c,d) is equivalent to VCOND_MASK(a!=b,d,c), and which
> > form gets generated was previously arbitrary. This is reasonable for
> > many (most?) targets, but on x86_64 there's an asymmetry, equality
> > can be performed in 1 instruction, but inequality requires three.
> >
> > Teaching the middle-end's expand pass which form is preferred could
> > in theory be done with a new (very specific) target hook, that would
> > require documentation, but a more generic solution is for expand's
> > expand_vec_cond_mask_optab_fn to make use of rtx_costs, and reverse
> > the sense of VCOND_MASK if that would be an improvement. This has
> > the convenient property that the default rtx_costs of all comparison
> > operators is the same, resulting in no change unless explicitly
> > specified by the backend.
> >
> > 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? The i386 bits and the middle-end
> > bits?
I think there's a correctness bit as well, you essentially perform TER
without validating whether that's valid when doing
+ else if (cmp_code == SSA_NAME)
+ {
+ gimple *def_stmt = SSA_NAME_DEF_STMT (op0);
+ if (is_gimple_assign (def_stmt))
You'd need to use get_gimple_for_ssa_name here and deal with a NULL.
I also believe the case of an embedded tcc_comparison does not occur.
This is indeed a bit of an awkward case for the vcond -> vcmp, vcond_mask
split.
> I think it makes more sense to do this swapping in isel rather than in
> expand with the rest of the VCOND code. Also please print out the
> cost for the rtl that is being tried in the dump file.
> CC Richard for his thoughts on this.
As we want to get rid of TER (and be not limited by TER either), doing
this as part of ISEL would be indeed better. The question is what to
do with, say,
_1 = _2 != _3;
_4 = _1 ? _5 : _6;
_7 = .MASK_LOAD (..., _1);
for the multi-use case it might make sense to invert the defintion
of _1 and add mask inversion before the use in the .MASK_LOAD.
TER restrictions would make the patch as written not apply to this
case, leaving the non-inverted mask.
Another possibility would be to tackle this on the RTL side by
a pass like STV if there'd be sth like a define-insn-and-split
that keeps the != compare simple and split only after such pass.
That could make costing possibly more precise than can be done
at RTL expansion.
That said, I'd prefer a patch to ISEL where, in
gimple_expand_vec_cond_expr we'd do essentially the same as your
expansion patch (wrt cost compares), and replace the above IL
with
_8 = _2 == _3;
_1 = ~_8;
_4 = .VCOND_MASK (_1, _6, _5);
with multiple VEC_CONDs using _1 we'd then look through an
inversion to pick up the proper compare (we can rely on
such inversion to be never originally present for an
invertable comparison).
Richard.
>
> Thanks,
> Andrew Pinski
>
>
> >
> >
> > 2026-02-11 Roger Sayle <[email protected]>
> >
> > gcc/ChangeLog
> > PR target/123238
> > * expr.cc (convert_tree_comp_to_rtx): Make global.
> > * expr.h (convert_tree_comp_to_rtx): Prototype here.
> > * internal-fn.cc (expand_vec_cond_mask_optab_fn): Use rtx_costs
> > to determine whether swapping operands would result in better
> > code.
> >
> > * config/i386/i386-expand.cc (ix86_expand_int_vec_cmp): On
> > AVX512 targets use a ternlog instead of a comparison to negate
> > the mask (requires one instruction instead of two).
> > * config/i386/i386.cc (ix86_rtx_costs): Refactor code for UNSPEC.
> > Provide costs for UNSPEC_BLENDV and UNSPEC_MOVMSK. Provide
> > costs for comparison operators of integer vector modes.
> >
> > gcc/testsuite/ChangeLog
> > PR target/123238
> > * gcc.target/i386/pr123238.c: New test case.
> >
> >
> > Roger
> > --
> >
>
--
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
