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.

Sign up for GitHub

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

Jump to bottom

Make memory allocation macros windows-compatible #4071

Closed

dvorjackz wants to merge 1 commit into pytorch:main from dvorjackz:export-D58850316

Contributor

dvorjackz commented Jun 26, 2024 •

edited

Loading

Summary:
Make ET_TRY_ALLOCATE_OR, ET_TRY_ALLOCATE_INSTANCE_OR, and ET_TRY_ALLOCATE_LIST_OR raise static assertion errors at compile time when compiled with statement expression-incompatible compilers (namely MSVC, among others).

The recommended alternative is to eschew the above macros in favor of directly allocating the memory, as hinted by the assertion comments.

The memory_allocator_test.cpp now only tests the macros if compiled using GCC 3.0+/Clang (statement expression-compatible). This is for backwards compatibility temporarily. Eventually the goal is remove all call-sites of ET_TRY_ALLOCATE_OR, ET_TRY_ALLOCATE_INSTANCE_OR, and ET_TRY_ALLOCATE_LIST_OR yet.

Differential Revision: D58850316

pytorch-bot bot commented Jun 26, 2024 •

edited

Loading

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/4071

📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 5c67c77 with merge base 929fc80 ():
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

facebook-github-bot added the CLA Signed label

Contributor

facebook-github-bot commented Jun 26, 2024

This pull request was exported from Phabricator. Differential Revision: D58850316

facebook-github-bot added the fb-exported label

Contributor

facebook-github-bot commented Jun 26, 2024

This pull request was exported from Phabricator. Differential Revision: D58850316

dvorjackz added a commit to dvorjackz/executorch that referenced this pull request


          Make memory allocation macros windows-compatible (pytorch#4071)

158ac4e

Summary:
Pull Request resolved: pytorch#4071

Make windows-compatible versions of memory allocation macros `ET_TRY_ALLOCATE_OR`, `ET_TRY_ALLOCATE_INSTANCE_OR`, and `ET_TRY_ALLOCATE_LIST_OR` by using lambda functions. Previously the preprocessing did not work because the windows compiler does not support expression statements. Keep the original implementations to maintain compatibility with other call-sites, which will eventually be refactored.

Then refactor the `memory_allocator_test.cpp` to migrate away from using `ET_TRY_ALLOCATE_OR`, `ET_TRY_ALLOCATE_INSTANCE_OR`, and `ET_TRY_ALLOCATE_LIST_OR`.

Differential Revision: D58850316

dvorjackz force-pushed the export-D58850316 branch from 622b6e1 to 158ac4e Compare

June 26, 2024 21:47

Contributor

facebook-github-bot commented Jun 26, 2024

This pull request was exported from Phabricator. Differential Revision: D58850316

dvorjackz added a commit to dvorjackz/executorch that referenced this pull request


          Make memory allocation macros windows-compatible (pytorch#4071)

946f59d

Summary:
Pull Request resolved: pytorch#4071

Make windows-compatible versions of memory allocation macros `ET_TRY_ALLOCATE_OR`, `ET_TRY_ALLOCATE_INSTANCE_OR`, and `ET_TRY_ALLOCATE_LIST_OR` by using lambda functions. Previously the preprocessing did not work because the windows compiler does not support expression statements. Keep the original implementations to maintain compatibility with other call-sites, which will eventually be refactored.

Then refactor the `memory_allocator_test.cpp` to migrate away from using `ET_TRY_ALLOCATE_OR`, `ET_TRY_ALLOCATE_INSTANCE_OR`, and `ET_TRY_ALLOCATE_LIST_OR`.

Differential Revision: D58850316

dvorjackz force-pushed the export-D58850316 branch from 158ac4e to 946f59d Compare

June 26, 2024 21:57

Contributor

facebook-github-bot commented Jun 27, 2024

This pull request was exported from Phabricator. Differential Revision: D58850316

dvorjackz added a commit to dvorjackz/executorch that referenced this pull request


          Make memory allocation macros windows-compatible (pytorch#4071)

06be9bb

Summary:
Pull Request resolved: pytorch#4071

Make windows-compatible versions of memory allocation macros `ET_TRY_ALLOCATE_OR`, `ET_TRY_ALLOCATE_INSTANCE_OR`, and `ET_TRY_ALLOCATE_LIST_OR` by using lambda functions. Previously the preprocessing did not work because the windows compiler does not support expression statements. Keep the original implementations to maintain compatibility with other call-sites, which will eventually be refactored.

Then refactor the `memory_allocator_test.cpp` to migrate away from using `ET_TRY_ALLOCATE_OR`, `ET_TRY_ALLOCATE_INSTANCE_OR`, and `ET_TRY_ALLOCATE_LIST_OR`.

Differential Revision: D58850316

dvorjackz force-pushed the export-D58850316 branch from 946f59d to 06be9bb Compare

June 27, 2024 02:00

Contributor

facebook-github-bot commented Jun 27, 2024

This pull request was exported from Phabricator. Differential Revision: D58850316

dvorjackz added a commit to dvorjackz/executorch that referenced this pull request


          Make memory allocation macros windows-compatible (pytorch#4071)

d79b9e1

Summary:
Pull Request resolved: pytorch#4071

Make windows-compatible versions of memory allocation macros `ET_TRY_ALLOCATE_OR`, `ET_TRY_ALLOCATE_INSTANCE_OR`, and `ET_TRY_ALLOCATE_LIST_OR` by using lambda functions. Previously the preprocessing did not work because the windows compiler does not support expression statements. Keep the original implementations to maintain compatibility with other call-sites, which will eventually be refactored.

Then refactor the `memory_allocator_test.cpp` to migrate away from using `ET_TRY_ALLOCATE_OR`, `ET_TRY_ALLOCATE_INSTANCE_OR`, and `ET_TRY_ALLOCATE_LIST_OR`.

Differential Revision: D58850316

dvorjackz force-pushed the export-D58850316 branch from 06be9bb to d79b9e1 Compare

June 27, 2024 02:06

Contributor

facebook-github-bot commented Jun 27, 2024

This pull request was exported from Phabricator. Differential Revision: D58850316

dvorjackz added a commit to dvorjackz/executorch that referenced this pull request


          Make memory allocation macros windows-compatible (pytorch#4071)

Summary:
Pull Request resolved: pytorch#4071

Make windows-compatible versions of memory allocation macros `ET_TRY_ALLOCATE_OR`, `ET_TRY_ALLOCATE_INSTANCE_OR`, and `ET_TRY_ALLOCATE_LIST_OR` by using lambda functions. Previously the preprocessing did not work because the windows compiler does not support expression statements. Keep the original implementations to maintain compatibility with other call-sites, which will eventually be refactored.

Then refactor the `memory_allocator_test.cpp` to migrate away from using `ET_TRY_ALLOCATE_OR`, `ET_TRY_ALLOCATE_INSTANCE_OR`, and `ET_TRY_ALLOCATE_LIST_OR`.

Differential Revision: D58850316

dvorjackz force-pushed the export-D58850316 branch from d79b9e1 to 8051028 Compare

June 27, 2024 02:12

Contributor

facebook-github-bot commented Jun 27, 2024

This pull request was exported from Phabricator. Differential Revision: D58850316

dvorjackz force-pushed the export-D58850316 branch from 8051028 to 5ace5d5 Compare

June 27, 2024 03:59

dvorjackz added a commit to dvorjackz/executorch that referenced this pull request


          Make memory allocation macros windows-compatible (pytorch#4071)

5ace5d5

Summary:
Pull Request resolved: pytorch#4071

Make windows-compatible versions of memory allocation macros `ET_TRY_ALLOCATE_OR`, `ET_TRY_ALLOCATE_INSTANCE_OR`, and `ET_TRY_ALLOCATE_LIST_OR` by using lambda functions. Previously the preprocessing did not work because the windows compiler does not support expression statements. Keep the original implementations to maintain compatibility with other call-sites, which will eventually be refactored.

Then refactor the `memory_allocator_test.cpp` to migrate away from using `ET_TRY_ALLOCATE_OR`, `ET_TRY_ALLOCATE_INSTANCE_OR`, and `ET_TRY_ALLOCATE_LIST_OR`.

Differential Revision: D58850316

Contributor

facebook-github-bot commented Jun 27, 2024

This pull request was exported from Phabricator. Differential Revision: D58850316

dvorjackz added a commit to dvorjackz/executorch that referenced this pull request


          Make memory allocation macros windows-compatible (pytorch#4071)

398e940

Summary:
Pull Request resolved: pytorch#4071

Make windows-compatible versions of memory allocation macros `ET_TRY_ALLOCATE_OR`, `ET_TRY_ALLOCATE_INSTANCE_OR`, and `ET_TRY_ALLOCATE_LIST_OR` by using lambda functions. Previously the preprocessing did not work because the windows compiler does not support expression statements. Keep the original implementations to maintain compatibility with other call-sites, which will eventually be refactored.

Then refactor the `memory_allocator_test.cpp` to migrate away from using `ET_TRY_ALLOCATE_OR`, `ET_TRY_ALLOCATE_INSTANCE_OR`, and `ET_TRY_ALLOCATE_LIST_OR`.

Differential Revision: D58850316

dvorjackz force-pushed the export-D58850316 branch from 5ace5d5 to 398e940 Compare

June 27, 2024 04:05

dbort approved these changes

View reviewed changes

dvorjackz force-pushed the export-D58850316 branch from 398e940 to e52163d Compare

June 28, 2024 02:37

dvorjackz added a commit to dvorjackz/executorch that referenced this pull request


          Make memory allocation macros windows-compatible (pytorch#4071)

e52163d

Summary:
Pull Request resolved: pytorch#4071

Make windows-compatible versions of memory allocation macros `ET_TRY_ALLOCATE_OR`, `ET_TRY_ALLOCATE_INSTANCE_OR`, and `ET_TRY_ALLOCATE_LIST_OR` by using lambda functions. Previously the preprocessing did not work because the windows compiler does not support expression statements. Keep the original implementations to maintain compatibility with other call-sites, which will eventually be refactored.

Then refactor the `memory_allocator_test.cpp` to migrate away from using `ET_TRY_ALLOCATE_OR`, `ET_TRY_ALLOCATE_INSTANCE_OR`, and `ET_TRY_ALLOCATE_LIST_OR`.

Reviewed By: dbort

Differential Revision: D58850316

Contributor

facebook-github-bot commented Jun 28, 2024

This pull request was exported from Phabricator. Differential Revision: D58850316


          Make memory allocation macros windows-compatible (pytorch#4071)

5c67c77

Summary:
Pull Request resolved: pytorch#4071

Make windows-compatible versions of memory allocation macros `ET_TRY_ALLOCATE_OR`, `ET_TRY_ALLOCATE_INSTANCE_OR`, and `ET_TRY_ALLOCATE_LIST_OR` by using lambda functions. Previously the preprocessing did not work because the windows compiler does not support expression statements. Keep the original implementations to maintain compatibility with other call-sites, which will eventually be refactored.

Then refactor the `memory_allocator_test.cpp` to migrate away from using `ET_TRY_ALLOCATE_OR`, `ET_TRY_ALLOCATE_INSTANCE_OR`, and `ET_TRY_ALLOCATE_LIST_OR`.

Reviewed By: dbort

Differential Revision: D58850316

dvorjackz force-pushed the export-D58850316 branch from e52163d to 5c67c77 Compare

June 28, 2024 02:42

Contributor

facebook-github-bot commented Jun 28, 2024

This pull request was exported from Phabricator. Differential Revision: D58850316

facebook-github-bot closed this in

748a4f8

facebook-github-bot added the Merged label

Contributor

facebook-github-bot commented Jun 28, 2024

This pull request has been merged in 748a4f8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment