Skip to content

Inline into extern function args during bounds inference #7261

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

Merged
merged 7 commits into from
Jan 5, 2023

Conversation

abadams
Copy link
Member

@abadams abadams commented Dec 29, 2022

Fixes #7260

Copy link
Contributor

@steven-johnson steven-johnson left a comment

Choose a reason for hiding this comment

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

Looks good so far, but every bot is failing...

@@ -199,6 +201,32 @@ bool is_fused_with_others(const vector<vector<Function>> &fused_groups,
return false;
}

class Inliner : public IRMutator {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: worth inserting a comment here about why this is necessary in addition to the previously existing inliner elsewhere

@abadams
Copy link
Member Author

abadams commented Jan 1, 2023

Failing the test python_correctness_realize_warnings because it only prints the warning twice instead of three times. I'm confused about why it would print it more than once. Any ideas?

@abadams
Copy link
Member Author

abadams commented Jan 1, 2023

I just changed the test to assert that the warning is printed at least once, and that nothing else is printed.

@steven-johnson
Copy link
Contributor

I guess the more interesting question here is how/why this PR caused any difference in that at all, whether or not the test made good sense in the first place...

@steven-johnson steven-johnson self-requested a review January 5, 2023 19:47
Copy link
Contributor

@steven-johnson steven-johnson left a comment

Choose a reason for hiding this comment

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

LGTM

@abadams abadams merged commit 4b74049 into main Jan 5, 2023
ardier pushed a commit to ardier/Halide-mutation that referenced this pull request Mar 3, 2024
* Inline into extern function args during bounds inference

Fixes halide#7260

* Run CSE once at the end

* Actually recursively inline

* clang-tidy

* trigger buildbots

* Make test invariant to the number of times the warning is printed

as long as it's at least once

Co-authored-by: Steven Johnson <srj@google.com>
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.

Inliner doesn't properly handle all Expr args for define_extern() Funcs
2 participants