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

Fix leaks caused by self-referential parameter constraints #7700

Merged
merged 4 commits into from
Jul 25, 2023

Conversation

abadams
Copy link
Member

@abadams abadams commented Jul 24, 2023

This should fix memory leaks caused by Parameters that have constraints that refer back to the same Parameter.

Note that it doesn't fix leaks caused by Parameter A having a constraint that refers to Parameter B which has a constraint that refers back to Parameter A.

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.

Does this allow us to re-enable leak detection on the bots, or is there more (known) work to be done?

@abadams
Copy link
Member Author

abadams commented Jul 24, 2023

There's more work to be done.

@steven-johnson
Copy link
Contributor

/home/halidenightly/build_bot/worker/halide-testbranch-main-llvm17-x86-64-linux-make/halide-source/src/Parameter.cpp: In function ‘Halide::Expr Halide::Internal::remove_self_references(const Halide::Internal::Parameter&, const Halide::Expr&)’:
/home/halidenightly/build_bot/worker/halide-testbranch-main-llvm17-x86-64-linux-make/halide-source/src/Parameter.cpp:206:14: error: ‘virtual Halide::Expr Halide::Internal::remove_self_references(const Halide::Internal::Parameter&, const Halide::Expr&)::RemoveSelfReferences::visit(const Halide::Internal::Variable*)’ can be marked override [-Werror=suggest-override]
  206 |         Expr visit(const Variable *var) {
      |              ^~~~~
/home/halidenightly/build_bot/worker/halide-testbranch-main-llvm17-x86-64-linux-make/halide-source/src/Parameter.cpp: In function ‘Halide::Expr Halide::Internal::restore_self_references(const Halide::Internal::Parameter&, const Halide::Expr&)’:
/home/halidenightly/build_bot/worker/halide-testbranch-main-llvm17-x86-64-linux-make/halide-source/src/Parameter.cpp:229:14: error: ‘virtual Halide::Expr Halide::Internal::restore_self_references(const Halide::Internal::Parameter&, const Halide::Expr&)::RestoreSelfReferences::visit(const Halide::Internal::Variable*)’ can be marked override [-Werror=suggest-override]
  229 |         Expr visit(const Variable *var) {
      |              ^~~~~

@abadams
Copy link
Member Author

abadams commented Jul 25, 2023

Failures unrelated

@abadams abadams merged commit f41c392 into main Jul 25, 2023
3 checks passed
ardier pushed a commit to ardier/Halide-mutation that referenced this pull request Mar 3, 2024
* Fix leaks caused by self-referential parameter constraints

* Add comment

* Add missing overrides

* Use const refs for non-mutated args
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.

2 participants