Skip to content

[PyTorch] Add attribute version of C10_ALWAYS_INLINE#136445

Closed
swolchok wants to merge 3 commits intogh/swolchok/648/basefrom
gh/swolchok/648/head
Closed

[PyTorch] Add attribute version of C10_ALWAYS_INLINE#136445
swolchok wants to merge 3 commits intogh/swolchok/648/basefrom
gh/swolchok/648/head

Conversation

@swolchok
Copy link
Contributor

@swolchok swolchok commented Sep 23, 2024 •
edited
Loading

Stack from ghstack (oldest at bottom):

Sometimes (such as on a lambda), you need __attribute__((always_inline)) but not inline.

Differential Revision: D63266917

@swolchok
[PyTorch] Add attribute version of C10_ALWAYS_INLINE
48fe99b 
Sometimes (such as on a lambda), you need `__attribute__((always_inline))` but not `inline`.

Differential Revision: [D63266917](https://our.internmc.facebook.com/intern/diff/D63266917/)

[ghstack-poisoned]
@pytorch-bot
Copy link

pytorch-bot bot commented Sep 23, 2024 •
edited
Loading

🔗 Helpful Links

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

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

✅ You can merge normally! (1 Unrelated Failure)

As of commit 7bf9b6f with merge base 99eb47f (image):

FLAKY - The following job failed but was likely due to flakiness present on trunk:

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

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Sep 23, 2024

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

@swolchok swolchok mentioned this pull request Sep 23, 2024
Closed
@swolchok swolchok added the topic: not user facing topic category label Sep 23, 2024
@swolchok swolchok requested review from albanD and malfet September 23, 2024 17:13
albanD
albanD reviewed Sep 23, 2024
View reviewed changes
Copy link
Collaborator

@albanD albanD left a comment

Choose a reason for hiding this comment

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

I'm not super familiar with these. But does this mean that for a bunch of compilers, C10_ALWAYS_INLINE_ATTRIBUTE is actually not going to inline? That sounds surprising?

@swolchok
Copy link
Contributor Author

swolchok commented Sep 23, 2024 •
edited
Loading

I'm not super familiar with these. But does this mean that for a bunch of compilers, C10_ALWAYS_INLINE_ATTRIBUTE is actually not going to inline? That sounds surprising?

which compilers? it does just as much inlining as C10_ALWAYS_INLINE. (The C++ inline keyword is a misnomer; what it actually means is "multiple definitions are permitted". See https://en.cppreference.com/w/cpp/language/inline )

@swolchok swolchok requested a review from albanD September 23, 2024 22:09
@swolchok
Update on "[PyTorch] Add attribute version of C10_ALWAYS_INLINE"
01dc6bc 
Sometimes (such as on a lambda), you need `__attribute__((always_inline))` but not `inline`.

Differential Revision: [D63266917](https://our.internmc.facebook.com/intern/diff/D63266917/)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Sep 30, 2024

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

malfet
malfet approved these changes Sep 30, 2024
View reviewed changes
Copy link
Contributor

@malfet malfet left a comment

Choose a reason for hiding this comment

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

This sounds fine, though not entirely sure why is it important

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Sep 30, 2024
@albanD
Copy link
Collaborator

albanD commented Oct 1, 2024

which compilers?

The ones that don't have __has_attribute(always_inline).
If the goal is to make it work on lambdas, why not call it C10_ALWAYS_INLINE_LAMBDA ?

@swolchok
Copy link
Contributor Author

swolchok commented Oct 1, 2024

The ones that don't have __has_attribute(always_inline).

C10_ALWAYS_INLINE works exactly the same way.

If the goal is to make it work on lambdas, why not call it C10_ALWAYS_INLINE_LAMBDA ?

Because it is not lambda-specific.

We could alternatively attempt to replace C10_ALWAYS_INLINE's definition with this one; I'm not sure why C10_ALWAYS_INLINE includes inline. I'd expect that to break bc (somebody will have used C10_ALWAYS_INLINE and left inline out) but I haven't tried.

@swolchok
Update on "[PyTorch] Add attribute version of C10_ALWAYS_INLINE"
7bf9b6f 
Sometimes (such as on a lambda), you need `__attribute__((always_inline))` but not `inline`.

Differential Revision: [D63266917](https://our.internmc.facebook.com/intern/diff/D63266917/)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Oct 2, 2024

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

pytorchmergebot pushed a commit that referenced this pull request Oct 3, 2024
@swolchok @pytorchmergebot
[PyTorch] Port ExecuTorch bfdot improvement back to ATen BlasKernel (#…
592e3a3 
…136331)

ExecuTorch's fork of BlasKernel.cpp grew bfdot support, complete with demonstration that it helps. Port it back to PyTorch. Supersedes #127488 . Includes pytorch/executorch#5444 .

Differential Revision: [D63045939](https://our.internmc.facebook.com/intern/diff/D63045939/)
Pull Request resolved: #136331
Approved by: https://github.com/malfet, https://github.com/albanD
ghstack dependencies: #136445
@github-actions github-actions bot deleted the gh/swolchok/648/head branch November 6, 2024 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@malfet malfet malfet approved these changes

@albanD albanD Awaiting requested review from albanD

Assignees

No one assigned

Labels

ciflow/trunk Trigger trunk jobs on your pull request fb-exported Merged topic: not user facing topic category

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@swolchok @facebook-github-bot @albanD @malfet @pytorchmergebot