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

importer: fix usage after PyTorch update #1555

Merged
merged 1 commit into from Nov 4, 2022
Merged

importer: fix usage after PyTorch update #1555

merged 1 commit into from Nov 4, 2022

Conversation

ashay
Copy link
Collaborator

@ashay ashay commented Nov 4, 2022

Unless requested otherwise, PyTorch no longer installs most of the
header files under the caffe2 directory (see
pytorch/pytorch#87986). This breaks our
importer code since we need to use the MakeGuard() function to execute
statements in the event of exceptions.

To fix this issue, this patch implements a rudimentary version of
PyTorch's ScopeGuard, where once the class variable goes out of scope,
it executes a predefined method.

@ashay
Copy link
Collaborator Author

ashay commented Nov 4, 2022

Btw, do we need do something special to conform with the PyTorch license if we add one of their files into our repository?

The most recent commit implements a rudimentary version of ScopeGuard that appears sufficient for our case, so I don't think we need to worry about licensing issues.

Unless requested otherwise, PyTorch no longer installs most of the
header files under the caffe2 directory (see
pytorch/pytorch#87986).  This breaks our
importer code since we need to use the `MakeGuard()` function to execute
statements in the event of exceptions.

To fix this issue, this patch implements a rudimentary version of
PyTorch's ScopeGuard, where once the class variable goes out of scope,
it executes a predefined method.
@ashay ashay merged commit d99b2dd into llvm:main Nov 4, 2022
@ashay ashay deleted the ashay/fix-ivalue-importer branch November 4, 2022 20:02
// RAII pattern to insert an operation before going out of scope.
class InserterGuard {
private:
MlirBlock _importBlock;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: no leading _ per style guide. Yes this is weird but it works :)

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.

None yet

3 participants