search

llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.06k stars 11.59k forks source link

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

Open RKSimon opened 4 years ago

RKSimon commented 4 years ago
Bugzilla Link 47249
Version unspecified
OS Windows NT
CC @erichkeane,@zygoloid
Fixed by commit(s) e7d9182a666a , 2ceac91ec0fc , 00158ae236ddfdc

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

bit twiddling:

vectors:

-fms-extensions variants:

RKSimon commented 4 years ago

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

2ceac91ec0fc

RKSimon commented 4 years ago

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

e7d9182a666a

RKSimon commented 4 years ago

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

RKSimon commented 4 years ago

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

chfast commented 1 year ago

Also __builtin_addc and __builtin_subc?

Endilll commented 1 year ago

Feels like a good first issue.

CC @shafik @AaronBallman

RKSimon commented 1 year ago

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 commented 1 year ago

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

llvmbot commented 1 year ago

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 commented 1 year ago

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

I would like to work on the remaining intrinsics.

RKSimon commented 1 year ago

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 commented 1 year ago

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

BertalanD commented 1 year ago

Also __builtin_addc and __builtin_subc?

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

RKSimon commented 1 year ago

Also #51787 (elementwise and reduction builtins)

Destroyerrrocket commented 8 months ago

Could I take the last two missing expressions?

shafik commented 8 months ago

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 commented 8 months ago

Thank you, I'll take it then!

RKSimon commented 8 months ago

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

Destroyerrrocket commented 8 months ago

Hi! I've made the changes for both intrinsics here: https://github.com/llvm/llvm-project/pull/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 commented 8 months ago

(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.