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

Some builtin intrinsics that are not constexpr but could be #46593

Closed
9 tasks done
RKSimon opened this issue Aug 20, 2020 · 21 comments
Closed
9 tasks done

Some builtin intrinsics that are not constexpr but could be #46593

RKSimon opened this issue Aug 20, 2020 · 21 comments
Assignees
Labels
bugzilla Issues migrated from bugzilla clang:frontend Language frontend issues, e.g. anything involving "Sema" good first issue https://github.com/llvm/llvm-project/contribute

Comments

@RKSimon
Copy link
Collaborator

RKSimon commented Aug 20, 2020

Bugzilla Link 47249
Version unspecified
OS Windows NT
CC @erichkeane,@zygoloid
Fixed by commit(s) e7d9182 , 2ceac91 , 00158ae

This isn't exhaustive, but all are low hanging fruit:

bit twiddling:

vectors:

  • __builtin_convertvector
  • __builtin_shufflevector

-fms-extensions variants:

  • __lzcnt16, __lzcnt, __lzcnt64 D157420 00158ae
  • __popcnt16, __popcnt, __popcnt64 D157420 00158ae
  • _rotl8, _rotl16, _rotl, _rotl64, _lrotl
  • _rotr8, _rotr16, _rotr, _rotr64, _lrotr
@RKSimon
Copy link
Collaborator Author

RKSimon commented Aug 21, 2020

https://reviews.llvm.org/D86339 - __builtin_bitreverse support

@RKSimon
Copy link
Collaborator Author

RKSimon commented Aug 21, 2020

https://reviews.llvm.org/D86342 - __builtin_rotateleft / __builtin_rotateright support

@RKSimon
Copy link
Collaborator Author

RKSimon commented Aug 22, 2020

https://reviews.llvm.org/D86339 - __builtin_bitreverse support

e7d9182

@RKSimon
Copy link
Collaborator Author

RKSimon commented Aug 22, 2020

https://reviews.llvm.org/D86342 - __builtin_rotateleft /
__builtin_rotateright support

2ceac91

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
@chfast
Copy link
Member

chfast commented Jul 17, 2023

Also __builtin_addc and __builtin_subc?

@Endilll
Copy link
Contributor

Endilll commented Jul 17, 2023

Feels like a good first issue.

CC @shafik @AaronBallman

@RKSimon
Copy link
Collaborator Author

RKSimon commented Jul 17, 2023

Feels like a good first issue.

Agreed although they need splitting into separate tasks - the ms-extension variants are pretty trivial, but convertvector might be harder and I expect shufflevector to be tricky.

@shafik
Copy link
Collaborator

shafik commented Jul 17, 2023

@RKSimon Looking at the PRs above they look relatively straight forward.

@RKSimon RKSimon added the good first issue https://github.com/llvm/llvm-project/contribute label Jul 17, 2023
@llvmbot
Copy link
Member

llvmbot commented Jul 17, 2023

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

  1. Assign the issue to you.
  2. Fix the issue locally.
  3. Run the test suite locally.
    3.1) Remember that the subdirectories under test/ create fine-grained testing targets, so you can
    e.g. use make check-clang-ast to only run Clang's AST tests.
  4. Create a git commit
  5. Run git clang-format HEAD~1 to format your changes.
  6. Submit the patch to Phabricator.
    6.1) Detailed instructions can be found here

For more instructions on how to submit a patch to LLVM, see our documentation.

If you have any further questions about this issue, don't hesitate to ask via a comment on this Github issue.

@llvm/issue-subscribers-good-first-issue

@alexguirre
Copy link
Contributor

Hi, seems like the ROTL/ROTR ms-extension intrinsics were already made constexpr in a1dc3d2.

I would like to work on the remaining intrinsics.

@RKSimon
Copy link
Collaborator Author

RKSimon commented Aug 7, 2023

Hi, seems like the ROTL/ROTR ms-extension intrinsics were already made constexpr in a1dc3d2.

I would like to work on the remaining intrinsics.

Great - I'd recommend starting with the the lzcnt/popcnt ms-extension intrinsics as they should be similar to the previous cases.

After that, __builtin_convertvector is probably easier than __builtin_shufflevector but both are very different to the other intrinsics, and will require some refactoring for missing vector type special case handling.

@alexguirre
Copy link
Contributor

Submitted a patch for lzcnt/popcnt https://reviews.llvm.org/D157420

@BertalanD
Copy link
Member

Also __builtin_addc and __builtin_subc?

I'm on it: D156156. I'll try to find time to finish it this week.

@RKSimon
Copy link
Collaborator Author

RKSimon commented Aug 9, 2023

Also #51787 (elementwise and reduction builtins)

RKSimon pushed a commit that referenced this issue Aug 14, 2023
As discussed on #46593 - this enables us to use __lzcnt / __popcnt intrinsics inside constexpr code.

Differential Revision: https://reviews.llvm.org/D157420
@Destroyerrrocket
Copy link
Contributor

Could I take the last two missing expressions?

@shafik
Copy link
Collaborator

shafik commented Dec 29, 2023

Could I take the last two missing expressions?

It does not look like anyone is currently working on them, so I can assign this to you if you want.

@Destroyerrrocket
Copy link
Contributor

Thank you, I'll take it then!

@RKSimon
Copy link
Collaborator Author

RKSimon commented Dec 30, 2023

@Destroyerrrocket Go for it! I'd recommend you start with __builtin_convertvector

@Destroyerrrocket
Copy link
Contributor

Destroyerrrocket commented Dec 30, 2023

Hi! I've made the changes for both intrinsics here: #76615
(I think that now you're meant to use github PRs? The bot seems to suggest to use Phabricator, but it seems down)
Can someone take a look? :D

@Endilll
Copy link
Contributor

Endilll commented Dec 30, 2023

(I think that now you're meant to use github PRs? The bot seems to suggest to use Phabricator, but it seems down)

Yes, we moved over to GitHub PRs since that comment was posted.

@RKSimon
Copy link
Collaborator Author

RKSimon commented Sep 22, 2024

Closing - __builtin_shufflevector and __builtin_convertvector were handled in #76615

@RKSimon RKSimon closed this as completed Sep 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla clang:frontend Language frontend issues, e.g. anything involving "Sema" good first issue https://github.com/llvm/llvm-project/contribute
Projects
None yet
Development

No branches or pull requests

8 participants