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 incorrect usage of CUDACachingAllocator #46605

Closed
wants to merge 4 commits into from

Conversation

xwang233
Copy link
Collaborator

We need an object to hold the ownership of allocated memory in the scope, instead of directly using the raw pointer.

@xwang233 xwang233 requested a review from ezyang October 20, 2020 18:58
@xwang233
Copy link
Collaborator Author

cc @ptrblck

@zou3519 zou3519 added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Oct 21, 2020
_apply_single_inverse_helper<scalar_t>(
&self_data[i * self_mat_stride], &self_inv_data[i * self_inv_mat_stride], pivot, p_infos + i, n);
&self_data[i * self_mat_stride], &self_inv_data[i * self_inv_mat_stride],
reinterpret_cast<int*>(dataPtr.get()), p_infos + i, n);
Copy link
Contributor

Choose a reason for hiding this comment

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

this predates this pr, but, shouldn't need a reinterpret cast here, static cast from void* should be fine

Copy link
Contributor

@ezyang ezyang left a comment

Choose a reason for hiding this comment

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

Thanks. So when we gonna get "disable caching allocator and cuda-memcheck" ci job 👍

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@ezyang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@ezyang
Copy link
Contributor

ezyang commented Oct 23, 2020

need to merge

@xwang233
Copy link
Collaborator Author

xwang233 commented Oct 23, 2020

It conflicted with my own changes. 😂 Resolved. Thanks!

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@ezyang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@dr-ci
Copy link

dr-ci bot commented Oct 23, 2020

💊 CI failures summary and remediations

As of commit 072391b (more details on the Dr. CI page):


💚 💚 Looks good so far! There are no failures yet. 💚 💚


This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 4 times.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@ezyang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@ezyang merged this pull request in e0fd590.

@facebook-github-bot
Copy link
Contributor

@ezyang merged this pull request in e0fd590.

1 similar comment
@facebook-github-bot
Copy link
Contributor

@ezyang merged this pull request in e0fd590.

facebook-github-bot pushed a commit that referenced this pull request Dec 7, 2020
Summary:
This is similar to #46605, where the c10::complex part of the code was not merged yet at that moment.

Pull Request resolved: #48817

Reviewed By: malfet

Differential Revision: D25333179

Pulled By: ezyang

fbshipit-source-id: a92bdad5ad4b36bef7f050b21a59676c38e7b1fc
emcastillo pushed a commit to emcastillo/pytorch that referenced this pull request Mar 16, 2022
Summary:
We need an object to hold the ownership of allocated memory in the scope, instead of directly using the raw pointer.

Pull Request resolved: pytorch#46605

Reviewed By: zou3519

Differential Revision: D24453548

Pulled By: ezyang

fbshipit-source-id: d29e5a69afa6c0d9e519849910e04524667d0a26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Merged open source triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants