On Mon, Jun 1, 2026 at 6:56 PM Abhishek Kaushik
<[email protected]> wrote:
>
> Hello,
>
> Thank you for the review
>
> On 5/28/26 5:36 PM, Jeffrey Law wrote:
> > So we very much want to look through those conversions and use the
> > narrower range.  But it seems like ranger should have associated the
> > narrower range with output SSA_NAME for those conversions and we
> > should have been using that output SSA_NAME rather chasing things down
> > through a CONVERT_EXPR.
> >
> > So I guess the question is how do we get a CONVERT_EXPR in here rather
> > than an SSA_NAME?  That may point to a cleaner solution.
> >
> > jeff
> The CONVERT_EXPR comes from the existing call to
> expand_simple_operations in number_of_iterations_exit_assumptions:
>
>    iv0.base = expand_simple_operations (iv0.base);
>    iv1.base = expand_simple_operations (iv1.base);
>
> expand_simple_operations treats casts as simple operations, so in this
> case the SSA_NAME for the converted value has already been expanded back
> into a CONVERT_EXPR by the time determine_value_range sees it.
>
> I did try avoiding this expansion, and also tried changing
> expand_simple_operations to return the original SSA_NAME for this case.
> Both approaches caused regressions in cunrolli and lsplit, which made the
> change seem too broad.  Given that expand_simple_operations is used by
> niter analysis to expose simple operations in bounds, handling
> value-preserving conversions in determine_value_range seemed like the
> more localized fix.
>
> That said, I may be missing a cleaner place to recover the range of the
> converted SSA_NAME.  If you think there is a better point to do this, I’m
> happy to rework the patch in that direction.

niter analysis is now set up to work with passes having ranger enabled
and ranger allows analysis of GENERIC expressions via range_of_expr
just fine (it needs a context stmt though).  I suggest to rework to make
use of that instead of walking stmts in niter as that seems wrong to me.

Another possibility would be to make expand_simple_operations also
track/combine value-range of the result, but I think this shouldn't be needed.

Richard.

>
> Thanks,
> Abhishek

Reply via email to