Use `__cpp_aligned_new` instead of hand-rolling the implementation by tamird · Pull Request #11293 · pytorch/executorch

tamird · 2025-06-02T18:46:18Z

The current feature detection seems to be incorrect, resulting in fallback to
the manual implementation, which in turn returns untagged pointers under
hwasan.
Use new expressions instead to delegate the work to the C++ compiler.

Differential Revision: D75806635

pytorch-bot · 2025-06-02T18:46:21Z

🔗 Helpful Links

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

📄 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 f7fbffe with merge base bca2cf5 ():
💚 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 · 2025-06-02T18:46:34Z

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

tamird · 2025-06-02T18:47:43Z

@pytorchbot label "release notes: none"

facebook-github-bot · 2025-06-02T19:20:35Z

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

facebook-github-bot · 2025-06-02T19:37:49Z

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

facebook-github-bot · 2025-06-02T22:06:09Z

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

facebook-github-bot · 2025-06-02T22:06:49Z

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

facebook-github-bot · 2025-06-03T01:34:33Z

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

facebook-github-bot · 2025-06-03T09:54:30Z

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

facebook-github-bot · 2025-06-03T09:59:49Z

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

facebook-github-bot · 2025-06-03T10:07:50Z

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

facebook-github-bot · 2025-06-03T10:33:30Z

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

facebook-github-bot · 2025-06-03T14:55:52Z

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

facebook-github-bot · 2025-06-03T15:17:53Z

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

facebook-github-bot · 2025-06-03T15:38:15Z

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

facebook-github-bot · 2025-06-03T16:30:54Z

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

swolchok

Do we know whether this actually works on Apple platforms? #10660 (the PR that added the previous implementation) cited some concerns about aligned_alloc.

I'm inclined to accept this on the grounds that we should presume that standard library functions that exist on a platform actually work until proven otherwise, but I'd like to let @lucylq chime in.

tamird · 2025-06-03T18:03:05Z

Do we know whether this actually works on Apple platforms? #10660 (the PR that added the previous implementation) cited some concerns about aligned_alloc.

I'm inclined to accept this on the grounds that we should presume that standard library functions that exist on a platform actually work until proven otherwise, but I'd like to let @lucylq chime in.

We have tests on apple platforms, so that should be sufficient to prove that it works. Yes?

facebook-github-bot · 2025-06-03T18:04:56Z

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

facebook-github-bot · 2025-06-03T18:27:45Z

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

lucylq

thanks for the changes!

facebook-github-bot · 2025-06-03T18:42:22Z

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

facebook-github-bot · 2025-06-03T19:01:38Z

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

facebook-github-bot · 2025-06-03T20:27:36Z

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

facebook-github-bot · 2025-06-04T00:57:11Z

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

facebook-github-bot · 2025-06-04T08:13:49Z

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

tamird · 2025-06-04T12:44:06Z

@swolchok this keeps failing to land with:

The following GitHub Pull Request(s) are not mergeable on GitHub: #11293. This Diff cannot land if the Pull Request cannot be merged. Please rebase the diff and re-export it to GitHub or resolve the merge conflict on GitHub and import the changes.

Yet this PR shows no merge conflicts. This has persisted through several rebases. Do you know what's going on?

tamird · 2025-06-04T14:39:02Z

The problem seems to be:

$ curl -sfSL https://api.github.com/repos/pytorch/executorch/pulls/11293 | jq .mergeable_state
"unstable"

which is an undocumented field. Why is that used to determine whether it is mergeable or not?

facebook-github-bot · 2025-06-04T14:39:37Z

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

facebook-github-bot · 2025-06-04T14:43:21Z

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

swolchok · 2025-06-04T15:00:51Z

@swolchok this keeps failing to land with:

The following GitHub Pull Request(s) are not mergeable on GitHub: #11293. This Diff cannot land if the Pull Request cannot be merged. Please rebase the diff and re-export it to GitHub or resolve the merge conflict on GitHub and import the changes.

Yet this PR shows no merge conflicts. This has persisted through several rebases. Do you know what's going on?

how are you updating the PR? if you sent it out internally, you need to update the internal diff and export that diff to github; I see force pushes which looks like maybe you are updating the PR through some separate mechanism like working with github directly.

facebook-github-bot · 2025-06-04T16:43:58Z

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

tamird · 2025-06-04T17:09:38Z

@swolchok this keeps failing to land with:

The following GitHub Pull Request(s) are not mergeable on GitHub: #11293. This Diff cannot land if the Pull Request cannot be merged. Please rebase the diff and re-export it to GitHub or resolve the merge conflict on GitHub and import the changes.

Yet this PR shows no merge conflicts. This has persisted through several rebases. Do you know what's going on?

how are you updating the PR? if you sent it out internally, you need to update the internal diff and export that diff to github; I see force pushes which looks like maybe you are updating the PR through some separate mechanism like working with github directly.

No, those force pushes are done by phabricator.

It seems that PRs that have both internal and external changes are broken because the mergeable_state is always "unstable".

…11293) Summary: Pull Request resolved: #11293 The current feature detection seems to be incorrect, resulting in fallback to the manual implementation, which in turn returns untagged pointers under hwasan. Use new expressions instead to delegate the work to the C++ compiler. Reviewed By: lucylq Differential Revision: D75806635

facebook-github-bot · 2025-06-04T18:47:17Z

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

tamird requested review from JacobSzwejbka, lucylq and swolchok as code owners June 2, 2025 18:46

facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported labels Jun 2, 2025

pytorch-bot bot added the release notes: none Do not include this in the release notes label Jun 2, 2025

tamird changed the title ~~Use posix_memalign on all non-CRT platforms~~ Use __cpp_aligned_new instead of hand-rolling the implementation Jun 3, 2025

swolchok reviewed Jun 3, 2025

View reviewed changes

lucylq approved these changes Jun 3, 2025

View reviewed changes

facebook-github-bot merged commit aa67f08 into pytorch:main Jun 4, 2025
169 of 171 checks passed

Conversation

tamird commented Jun 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Jun 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

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

✅ No Failures

Uh oh!

facebook-github-bot commented Jun 2, 2025

Uh oh!

tamird commented Jun 2, 2025

Uh oh!

facebook-github-bot commented Jun 2, 2025

Uh oh!

facebook-github-bot commented Jun 2, 2025

Uh oh!

facebook-github-bot commented Jun 2, 2025

Uh oh!

facebook-github-bot commented Jun 2, 2025

Uh oh!

facebook-github-bot commented Jun 3, 2025

Uh oh!

facebook-github-bot commented Jun 3, 2025

Uh oh!

facebook-github-bot commented Jun 3, 2025

Uh oh!

facebook-github-bot commented Jun 3, 2025

Uh oh!

facebook-github-bot commented Jun 3, 2025

Uh oh!

facebook-github-bot commented Jun 3, 2025

Uh oh!

facebook-github-bot commented Jun 3, 2025

Uh oh!

facebook-github-bot commented Jun 3, 2025

Uh oh!

facebook-github-bot commented Jun 3, 2025

Uh oh!

swolchok left a comment

Choose a reason for hiding this comment

Uh oh!

tamird commented Jun 3, 2025

Uh oh!

facebook-github-bot commented Jun 3, 2025

Uh oh!

facebook-github-bot commented Jun 3, 2025

Uh oh!

lucylq left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Jun 3, 2025

Uh oh!

facebook-github-bot commented Jun 3, 2025

Uh oh!

facebook-github-bot commented Jun 3, 2025

Uh oh!

facebook-github-bot commented Jun 4, 2025

Uh oh!

facebook-github-bot commented Jun 4, 2025

Uh oh!

tamird commented Jun 4, 2025

Uh oh!

tamird commented Jun 4, 2025

Uh oh!

facebook-github-bot commented Jun 4, 2025

Uh oh!

facebook-github-bot commented Jun 4, 2025

Uh oh!

swolchok commented Jun 4, 2025

Uh oh!

facebook-github-bot commented Jun 4, 2025

Uh oh!

tamird commented Jun 4, 2025

Uh oh!

facebook-github-bot commented Jun 4, 2025

Uh oh!

Uh oh!

Reviewers

tamird commented Jun 2, 2025 •

edited

Loading

pytorch-bot bot commented Jun 2, 2025 •

edited

Loading