Fix leaks caused by self-referential parameter constraints by abadams · Pull Request #7700 · halide/Halide · GitHub
Skip to content

Fix leaks caused by self-referential parameter constraints#7700

Merged
abadams merged 4 commits into
mainfrom
abadams/remove_parameter_self_references
Jul 25, 2023
Merged

Fix leaks caused by self-referential parameter constraints#7700
abadams merged 4 commits into
mainfrom
abadams/remove_parameter_self_references

Conversation

@abadams

@abadams abadams commented Jul 24, 2023

Copy link
Copy Markdown
Member

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.

steven-johnson
steven-johnson approved these changes Jul 24, 2023
View reviewed changes

@steven-johnson steven-johnson left a comment

Copy link
Copy Markdown
Contributor

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

abadams commented Jul 24, 2023

Copy link
Copy Markdown
Member Author

@steven-johnson

steven-johnson commented Jul 24, 2023

Copy link
Copy Markdown
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 abadams mentioned this pull request Jul 24, 2023
Merged
@abadams

abadams commented Jul 25, 2023

Copy link
Copy Markdown
Member Author

@abadams abadams merged commit f41c392 into main Jul 25, 2023
@BrewTestBot BrewTestBot mentioned this pull request Feb 2, 2024
Closed
ardier pushed a commit to ardier/Halide-mutation that referenced this pull request Mar 3, 2024
@abadams @ardier
Fix leaks caused by self-referential parameter constraints (halide#7700)
6f3a5dc 
* 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

Reviewers

@steven-johnson steven-johnson steven-johnson approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@abadams @steven-johnson