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

[Relay] Non-recursive Dtor for Let #9461

Merged
merged 8 commits into from
Nov 8, 2021
Merged

[Relay] Non-recursive Dtor for Let #9461

merged 8 commits into from
Nov 8, 2021

Conversation

hzfan
Copy link
Contributor

@hzfan hzfan commented Nov 5, 2021

Destructor for Let overflows when the graph size grows. This PR replaces the recursive destructor with non-recursive one.

cc @yzhliu @comaniac

@hzfan hzfan changed the title [Relay] Non-recursive Detor for Let [Relay] Non-recursive Dtor for Let Nov 5, 2021
Copy link
Contributor

@comaniac comaniac left a comment

Choose a reason for hiding this comment

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

LGTM.
This PR is similar to the previous one (#7832) that makes deconstructing Call nodes non-recursive.

Also cc @mbrookhart

Copy link
Contributor

@mbrookhart mbrookhart left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@mbrookhart
Copy link
Contributor

FYI, I had almost exactly this code in here: #8434

And then I got dragged away to other problems and never fixed the un-related bugs in other changes I made :D :(

@hzfan
Copy link
Contributor Author

hzfan commented Nov 5, 2021

@mbrookhart Would you mind if I port the dependency_graph.cc part in #8434 to this PR ? I encounter a similar problem when printing IR.

@mbrookhart
Copy link
Contributor

Please, take whatever you want, I never got back to figuring out which of the changes I made ended up causing CI errors.

Copy link
Member

@yzhliu yzhliu left a comment

Choose a reason for hiding this comment

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

LGTM

@masahi masahi merged commit 028f4fa into apache:main Nov 8, 2021
mehrdadh pushed a commit to mehrdadh/tvm that referenced this pull request Dec 1, 2021
* non-recursive let desctruction

* fix serialization

* Fix serialization

* lint

* lint

* lint

* add tests for serial/deserial

* lint

Co-authored-by: Haozheng Fan <hzfan@apache.com>
mehrdadh pushed a commit to mehrdadh/tvm that referenced this pull request Dec 1, 2021
* non-recursive let desctruction

* fix serialization

* Fix serialization

* lint

* lint

* lint

* add tests for serial/deserial

* lint

Co-authored-by: Haozheng Fan <hzfan@apache.com>
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 7, 2022
* non-recursive let desctruction

* fix serialization

* Fix serialization

* lint

* lint

* lint

* add tests for serial/deserial

* lint

Co-authored-by: Haozheng Fan <hzfan@apache.com>
yangulei pushed a commit to yangulei/tvm that referenced this pull request Jan 11, 2022
* non-recursive let desctruction

* fix serialization

* Fix serialization

* lint

* lint

* lint

* add tests for serial/deserial

* lint

Co-authored-by: Haozheng Fan <hzfan@apache.com>
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 13, 2022
* non-recursive let desctruction

* fix serialization

* Fix serialization

* lint

* lint

* lint

* add tests for serial/deserial

* lint

Co-authored-by: Haozheng Fan <hzfan@apache.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.

5 participants