Re: [PATCH v3 1/4] Detect predicated-index PHI

On Thu Jul 30, 2026 at 12:53 AM IST, Robin Dapp wrote:
> Hi Ragesh,
>
> > This updated patch implements recognition only, as you suggested: durin=
g
> > scalar-cycle analysis we classify the mask ? n+1 : n loop-header PHI an=
d
> > record the increment mask on its stmt_vec_info. Masked-store pairing is
> > deferred to a follow-up patch; we do not record anything in
> > loop_vec_info. Until we can vectorize this def type, vect_is_simple_use
> > rejects these PHIs so the loop fails cleanly.
> >
> > This patch, we believe, is self-contained with a graceful exit so it
> > can land upstream on its own and mark clear progress toward the full
> > compress-store path.
> >
> > Your foo example (res++ under a condition, then q[i] =3D res) is covere=
d
> > by vect-predicated-index-2.c: the matcher does not fire because
> > if-conversion leaves a guarded add on the backedge, not mask ? res+1 :
> > res (predicated-index PHI: backedge is not COND_EXPR.). Even if we late=
r
> > extend recognition to more increment forms, a follow-up pairing pass
> > would still need to reject uses where the counter is stored as a value
> > rather than serving only as a compress index.
>
> I'm still concerned about overflow in the phi, in particular if we just=
=20
> strip conversions.  Your example is signed so should be safe apriori,=20
> but I guess ifcvt transforms it to unsigned?  I guess we need at least=20
> max_stmt_executions on the addition to verify we're not exceeding the=20
> type's bound?  On top, things would get complicated with non-nop=20
> conversions in the chain and you would need to keep track of the minimum=
=20
> type somewhere?  Maybe restrict the strip function to nop conversions=20
> for now?
>
Hi Robin,

Thanks for the review.

I understand your concern: after stripping casts we need the correct
arithmetic type on the +1 chain, and we should not treat a predicated
index as valid for compress unless wrap can be ruled out.

We could extend the matcher to follow the effective arithmetic type
on the +1 chain (after stripping casts) and to require a no-wrap proof
such as max_stmt_executions before we accept the PHI. If that
compile-time proof is too conservative and we reject valid cases,
we may also consider adding at a runtime check. I will explore the
details and propose an approach in a follow-up.

>
> > One refactoring we may do - If-conversion will likely need the same
> > shape check (mask ? n+1 : n). Would you prefer the matcher reuse that
> > if possible, avoiding code duplication across two passes?
>
> Can't we already tell in ifcvt by seeing a COND_ADD (..., n, 1, n)?

I will check whether we can reuse if-conversion's logic for this.

>
> BTW your test requires an effective avx512 target but also adds an=20
> aarch64 option.  Does that work?
>

You are right. We will keep the detection tests generic (vect_int only,
no AVX-512 requirement) until compress lowering is implemented, then
add separate arch-specific tests.

> --=20
> Regards
>  Robin