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

Track likely values through lets in loop partitioning #7930

Merged
merged 4 commits into from
Nov 16, 2023
Merged

Conversation

abadams
Copy link
Member

@abadams abadams commented Nov 1, 2023

Fixes #7929

Improves runtime of lens_blur app by ~20%

Fixes #7929

Improves runtime of lens_blur app by ~20%
Now that we look through lets, we end up in more situations where both
sides have a captured likely.
clamp(likely(arg_var), min, min + extent - 1));
likely(clamp(likely(arg_var), min, min + extent - 1)));

// We want loop partitioning to both cause the clamp to go away, and
Copy link
Member

Choose a reason for hiding this comment

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

This comment seems to be after the line it explains.

Copy link
Member

Choose a reason for hiding this comment

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

Also, it would be great if the comment explained why two likely tags cause it to go away.


namespace Halide {
namespace Internal {

/** Return true if an expression uses a likely tag that isn't captured
* by an enclosing Select, Min, or Max. */
bool has_uncaptured_likely_tag(const Expr &e);
bool has_uncaptured_likely_tag(const Expr &e, const Scope<> &scope);
Copy link
Member

Choose a reason for hiding this comment

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

The comments need to be updated to say that the scope is for variables of lets which has likely tag in them.

@abadams
Copy link
Member Author

abadams commented Nov 14, 2023

ptal

@abadams abadams merged commit ad0f24e into main Nov 16, 2023
3 checks passed
@steven-johnson steven-johnson deleted the abadams/fix_7929 branch November 30, 2023 15:25
ardier pushed a commit to ardier/Halide-mutation that referenced this pull request Mar 3, 2024
* Track likely values through lets in loop partitioning

Fixes halide#7929

Improves runtime of lens_blur app by ~20%

* Add uncaptured likely tags to selects in boundary condition helpers

Now that we look through lets, we end up in more situations where both
sides have a captured likely.

* Better comments
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.

The simplifier and CSE foil loop partitioning when one side of the bound is a constant
2 participants