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? > 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)? BTW your test requires an effective avx512 target but also adds an aarch64 option. Does that work? -- Regards Robin
