> -----Original Message-----
> From: Robin Dapp <[email protected]>
> Sent: 23 September 2026 16:37
> To: Tamar Christina <[email protected]>; [email protected]
> Cc: nd <[email protected]>; [email protected]; [email protected];
> Robin Dapp <[email protected]>
> Subject: Re: [patch 9/14][isel]: form conditional vector compare IFNs
>
> > /* Optimize
> > mask_2 = 1 << cnt_1;
> > _4 = __atomic_fetch_or_* (ptr_6, mask_2, _3);
> > @@ -1369,6 +1440,20 @@ pass_gimple_isel::execute (struct function *fun)
> > /* Give the target first try at replacing the instruction. */
> > cfg_changed |= targetm.instruction_selection (fun, &gsi);
> >
> > + gimple *isel_stmt = NULL;
> > + gassign *dead_cmp_def = NULL;
> > + if (!gsi_end_p (gsi))
> > + isel_stmt = gimple_expand_conditional_compare (&gsi,
> &dead_cmp_def);
> > + if (isel_stmt)
> > + {
> > + gsi_replace (&gsi, isel_stmt, true);
> > + if (dead_cmp_def)
> > + {
> > + gimple_stmt_iterator cmp_gsi = gsi_for_stmt
> (dead_cmp_def);
> > + gsi_remove (&cmp_gsi, true);
> > + }
> > + }
>
> At some point here we should ensure that the else value is zero before
> performing the replacement. I realize it's not necessary for SVE but if
> we have the (fixed-zero) else value like I mentioned in 7/14, we should
> make sure here.
But there's no else value to emit. The OPTAB and ifn do not have an else.
since any non-zero else value don't make sense unless as I mentioned in
the earlier patch you can make sure all consumer are LEN masked which
today there is no guaranteed.
>
> > diff --git a/gcc/match.pd b/gcc/match.pd
> > index
> c1aba410b66e38c947a4d52064fda1594934f297..db5038b7dccd030a09b6
> 7d5070a2ce2b74651184 100644
> > --- a/gcc/match.pd
> > +++ b/gcc/match.pd
> > @@ -13364,3 +13364,20 @@ and,
> > (if (INTEGRAL_TYPE_P (type))
> > (with { tree itype = TREE_TYPE (@2); }
> > (convert (minus @2 (convert:itype @1))))))
>
> > +/* Transform .VCOND_MASK_LEN (a `cmp` b, {-1, ...}, {0, ...}, len,
> > bias)
> > + into cond_len_vec_cmp (cmp, mask, a, b, len, bias). */
> > + (match (convert_conditional_len_compare @0 @1 @2 @3 @4 @5)
> > + (IFN_VCOND_MASK_LEN @0 (cmp@5 @1 @2) integer_minus_onep
> integer_zerop @3 @4)
> > + (if (VECTOR_BOOLEAN_TYPE_P (type)
> > + && !FLOAT_TYPE_P (TREE_TYPE (@1))))))
>
> I don't think this is 100% correct, because the arguments don't match
> (easily fixed of course). The larger issue is that the tail doesn't
> match as cond_len_vec_cmp doesn't have a guaranteed zeroing of tail
> elements (via else value or otherwise). The result would then only be
> valid under length masking which we can't enforce locally.
> It would likely work in all relevant cases but it's still not ideal.
>
> However, I don't want to bother you with len-only stuff you cannot
> really test. My suggestion is to drop this as well as other new
> vcond_mask_len handling. I'm 99.9% sure vcond_mask_len should go away
> for early break anyway so it doesn't make sense to improve its uses.
> I'd also drop the _len isel path. Once the len design is right, we
> shouldn't be seeing the m & cmp at all and just have the implicit
> length.
I'm happy to drop it, but I don't think you want to replace m & cmp with a
opaque conditional operation. Support for VLA explicitly didn't do this as
you lose all optimizations. i.e. {0,0, ... } & cmp is obviously {0,0, ..} and
such
optimizations are important when we unroll, without this we can't remove
dead edges.
So again I'm happy to drop this, but you'd be fighting missed optimizations
for a long time by doing this for no obvious gain to me. I really think you
should pick a defined else value in GIMPLE for these CFG operations.
Thanks,
Tamar
>
> --
> Regards
> Robin
