On Thu Jul 30, 2026 at 12:53 AM IST, Robin Dapp wrote: > Hi Ragesh, > > > This updated patch implements recognition only, as you suggested: during > > scalar-cycle analysis we classify the mask ? n+1 : n loop-header PHI and > > 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] = res) is covered > > 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 later > > 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 > strip conversions. Your example is signed so should be safe apriori, > but I guess ifcvt transforms it to unsigned? I guess we need at least > max_stmt_executions on the addition to verify we're not exceeding the > type's bound? On top, things would get complicated with non-nop > conversions in the chain and you would need to keep track of the minimum > type somewhere? Maybe restrict the strip function to nop conversions > 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 > 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. > -- > Regards > Robin
