Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

scoped_truth for the loop variable being always less than the loop extent. #8306

Merged
merged 3 commits into from
Jun 21, 2024

Conversation

mcourteaux
Copy link
Contributor

Fixes #8303

@mcourteaux mcourteaux force-pushed the loop-var-bounded-scoped-truth branch from 6fa904b to c0c4f73 Compare June 20, 2024 11:26
@abadams
Copy link
Member

abadams commented Jun 20, 2024

This is actually a bit more subtle, because it's loop_var < min + extent, not loop_var < extent, but a scoped truth comparison on non-constants will really just look for that exact Expr. Maybe loop_var < mutate(min + extent, nullptr)?

@mcourteaux
Copy link
Contributor Author

Aren't loops rebased to have a zero starting range at that point?

@abadams
Copy link
Member

abadams commented Jun 20, 2024

The simplifier can run on almost any piece of IR, before or after loops get rebased to zero, so if you wanted to go with that strategy you'd have to add an if (is_const_zero(op->min))

EDIT: But if is is zero, then the thing I suggested above will collapse to just the extent anyway.

@mcourteaux
Copy link
Contributor Author

@abadams Okay, I think I fixed it. Thanks for helping out!

// The loop variable will never exceed the loop bound.
Expr loop_var = Variable::make(Int(32), op->name);
Expr new_max = mutate(new_min + new_extent, nullptr);
ScopedFact fact_loop_var_less_than_extent = scoped_truth(loop_var < new_max);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth adding scoped_truth(new_min <= loop_var) while you're here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. My brain is clearly not thinking this through... I committed that as well now.

@abadams abadams requested a review from halidebuildbots June 20, 2024 20:59
@abadams abadams merged commit 198c25e into halide:main Jun 21, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Simplifier fails to optimize for-loop endpoint.
2 participants