Skip to content

[CIR][CIRGen][Builtin][Neon] Lower neon_vqmovns_s32 and add CIR PoisonAttr #1199

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
merged 3 commits into from
Dec 12, 2024

Conversation

ghehg
Copy link
Contributor

@ghehg ghehg commented Dec 3, 2024

CIR PoisonOp is needed in this context as alternative would be to use VecCreateOp to prepare an arg for VecInsertElement, but VecCreate is for different purpose and it would insert all elements which is not totally unnecessary in this context.

Here is the intrinsic def

@ghehg ghehg force-pushed the bigMac branch 3 times, most recently from 3dc47ce to 7157833 Compare December 3, 2024 13:46
@ghehg ghehg marked this pull request as ready for review December 3, 2024 15:30
@bcardosolopes
Copy link
Member

Please introduce PoisonAttr, just like we have a UndefAttr (introduced by @smeenai). The idea is that such attribute could be used by both a full vector type or a regular type used in a vector lane, just like UndefAttr can.

@ghehg
Copy link
Contributor Author

ghehg commented Dec 6, 2024

Please introduce PoisonAttr, just like we have a UndefAttr (introduced by @smeenai). The idea is that such attribute > could be used by both a full vector type or a regular type used in a vector lane, just like UndefAttr can.

Sure, sounds best approach to me. let's do it. I'll create a vector type constantOP with thsi PoisonAttr, and it will become LLVM::PoisonOp after lowering. That CIR constant OP will be used as input to VecCreateOp, so we also change VecCreateOp lowering, so it won't generate initialization code during lowering when the input is poison constant.

@ghehg ghehg force-pushed the bigMac branch 2 times, most recently from 7ad1b26 to 3c21cec Compare December 10, 2024 14:27
@ghehg
Copy link
Contributor Author

ghehg commented Dec 10, 2024

Updated the PR using PoisonAttr. Instead of using VecCreateOp, I used VecSplatOp to get poison of vector type from a scalar poison constnat. VecCreateOp would require an array of constants with poison attr, which makes less suitable than VecSplatOp.

@ghehg ghehg changed the title [CIR][CIRGen][Builtin][Neon] Lower neon_vqmovns_s32 and add CIR PoisonOp [CIR][CIRGen][Builtin][Neon] Lower neon_vqmovns_s32 and add CIR PoisonAttr Dec 10, 2024
Copy link
Member

@bcardosolopes bcardosolopes left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for your patience!

@bcardosolopes bcardosolopes merged commit 0a1b06c into llvm:main Dec 12, 2024
6 checks passed
lanza pushed a commit that referenced this pull request Mar 18, 2025
…nAttr (#1199)

CIR PoisonOp is needed in this context as alternative would be to use
VecCreateOp to prepare an arg for VecInsertElement, but VecCreate is for
different purpose and [it would insert all
elements](https://github.com/llvm/clangir/blob/eacaabba76ebdbf87217fefaa77f92c45cf4509c/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp#L1679)
which is not totally unnecessary in this context.

Here is the [intrinsic def
](https://developer.arm.com/architectures/instruction-sets/intrinsics/#f:@navigationhierarchiessimdisa=[Neon]&q=vqmovns_)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants