Skip to content

[RISCV] Support floating point VCIX #67094

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 1 commit into from
Sep 25, 2023
Merged

Conversation

4vtomat
Copy link
Member

@4vtomat 4vtomat commented Sep 22, 2023

No description provided.

@4vtomat 4vtomat requested a review from a team September 22, 2023 07:05
@llvmbot llvmbot added clang Clang issues not falling into any other category backend:RISC-V clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Sep 22, 2023
Copy link
Member

@kito-cheng kito-cheng left a comment

Choose a reason for hiding this comment

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

The target is support LLVM IR part only, we would like to prevent expose that on the C intrinsic level if possible, because that's intentionally to expose vector with unsigned integer only.

@4vtomat
Copy link
Member Author

4vtomat commented Sep 24, 2023

The target is support LLVM IR part only, we would like to prevent expose that on the C intrinsic level if possible, because that's intentionally to expose vector with unsigned integer only.

Sure~

@4vtomat 4vtomat force-pushed the support_float_for_vcix branch from c6d4731 to 159f313 Compare September 24, 2023 09:37
Copy link
Member

@kito-cheng kito-cheng left a comment

Choose a reason for hiding this comment

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

LGTM

@4vtomat 4vtomat force-pushed the support_float_for_vcix branch from 159f313 to 81b724a Compare September 25, 2023 05:18
@4vtomat 4vtomat merged commit 408b081 into llvm:main Sep 25, 2023
@4vtomat 4vtomat deleted the support_float_for_vcix branch September 25, 2023 05:19
; CHECK-NEXT: sf.vc.xv 3, 31, v8, a0
; CHECK-NEXT: ret
entry:
tail call void @llvm.riscv.sf.vc.xv.se.iXLen.nxv1f16.i16.iXLen(iXLen 3, iXLen 31, <vscale x 1 x half> %vs2, i16 %rs1, iXLen %vl)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we really need to support integer scalars with FP vectors?

Copy link
Member

Choose a reason for hiding this comment

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

It's VCIX which could let user define what they want...which means it's also possible to design some function take an integer with floating point vector value, so I would say it possible, and support that on the LLVM IR only is harmless and not costly.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This patch increased the size of the isel table by 33K or 1.5%.

VCIX could also have an instruction that takes an FP vector and returns an int vector or vice versa, but we don't support that. So what is our criteria for what we support?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, okay, I got your point, I am thinking does it possible to convert those intrinsic to integer variant during lowering instead of expanding those combination in td?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that's a good idea, we just convert all floating point vector to integer vector during lowering, that can reduce the effort to create all of iteration in td file and also it could increase much size of isel table.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:RISC-V clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants