Skip to content

[Windows] [memory_allocator.h] remove ET_TRY macros #8914

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

Merged

Conversation

SamGondelman
Copy link
Contributor

@SamGondelman SamGondelman commented Mar 4, 2025

Summary

This is the third PR in my attempt to get Windows support working. I hit these static asserts because these macros weren't implemented. Let's remove the macros and just directly allocate the memory everywhere.

Test plan

Verify everything builds + unit tests.

Copy link

pytorch-bot bot commented Mar 4, 2025

🔗 Helpful Links

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

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

❌ 1 Cancelled Job

As of commit 29afa5f with merge base 24671a9 (image):

CANCELLED JOB - The following job was cancelled. Please retry:

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

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 4, 2025
Copy link
Contributor

@jackzhxng jackzhxng left a comment

Choose a reason for hiding this comment

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

The goal is to eventually remove all usages of ET_TRY_ALLOCATE_OR, ET_TRY_ALLOCATE_INSTANCE_OR, and ET_TRY_ALLOCATE_LIST_OR, these exist to retain compatibility with the non mvc compilers while enabling mvc compilation. Since you are hitting these static assertions, I'm guessing that you are using one of these three macros somewhere in your code - that's where you should replace the macro with the recommended memory_allocator method

@SamGondelman SamGondelman force-pushed the windows-memory-allocator branch from c1c9c92 to e361d95 Compare March 5, 2025 05:46
@SamGondelman SamGondelman changed the title [Windows] [memory_allocator.h] implement memory allocators for windows [Windows] [memory_allocator.h] remove ET_TRY macros Mar 5, 2025
@SamGondelman SamGondelman added the release notes: build Changes related to build, including dependency upgrades, build flags, optimizations, etc. label Mar 5, 2025
@SamGondelman SamGondelman force-pushed the windows-memory-allocator branch from e361d95 to 30eca6a Compare March 5, 2025 07:15
@SamGondelman SamGondelman force-pushed the windows-memory-allocator branch from 30eca6a to 29afa5f Compare March 5, 2025 07:31
@facebook-github-bot
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/trunk CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. release notes: build Changes related to build, including dependency upgrades, build flags, optimizations, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants