-
Notifications
You must be signed in to change notification settings - Fork 499
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
[JitBackend] Miscellaneous improvements to Jit infra #1124
Conversation
…lifetime This significantly simplifies node lifetime management, with one documented caveat -- `Use::user()` returns a `Node&` instead of `NodePtr`, and it also stores the data as `Node&` under the hood.
…ng results When evaluator decides whether to keep the result of a node, use the number of users instead of the raw shared_ptr refcount (which is overly conservative and breaks the soft abstraction of `NodePtr`).
This is often useful in graph transformation/analysis. The downcast mechanism also tries to keep the soft abstraction of `NodePtr`, `BinaryNodePtr`, etc (without leaking `std::shared_ptr` to users).
…n subgraph bounding box
@jacobkahn has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@StrongerXi has updated the pull request. You must reimport the pull request before landing. |
@StrongerXi has updated the pull request. You must reimport the pull request before landing. |
@StrongerXi has updated the pull request. You must reimport the pull request before landing. |
@jacobkahn has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@jacobkahn merged this pull request in d49bef5. |
Summary
A few changes to the Jit infra, mainly addressing 2 things:
Node
.Test Plan (required)
Some unit tests are updated.
Some changes rely on existing test.
The
GraphVizPrinter
is tested manually because I can't think of a good way to automate its testing.