-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[SPIRV] Added Support for the constrained conversion intrinsics #157437
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
base: main
Are you sure you want to change the base?
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-llvm-support @llvm/pr-subscribers-backend-spir-v Author: Subash B (SubashBoopathi) ChangesAdded SPIR-V support for constrained conversion intrinsics (sitofp, uitofp, fptosi, fptoui, fpext, fptrunc), including legalization, instruction selection, and corresponding tests. Full diff: https://github.com/llvm/llvm-project/pull/157437.diff 6 Files Affected:
diff --git a/llvm/include/llvm/Support/TargetOpcodes.def b/llvm/include/llvm/Support/TargetOpcodes.def
index b905576b61791..ea4eaff9cbd39 100644
--- a/llvm/include/llvm/Support/TargetOpcodes.def
+++ b/llvm/include/llvm/Support/TargetOpcodes.def
@@ -899,6 +899,12 @@ HANDLE_TARGET_OPCODE(G_STRICT_FREM)
HANDLE_TARGET_OPCODE(G_STRICT_FMA)
HANDLE_TARGET_OPCODE(G_STRICT_FSQRT)
HANDLE_TARGET_OPCODE(G_STRICT_FLDEXP)
+HANDLE_TARGET_OPCODE(G_STRICT_FPTOSI)
+HANDLE_TARGET_OPCODE(G_STRICT_SITOFP)
+HANDLE_TARGET_OPCODE(G_STRICT_UITOFP)
+HANDLE_TARGET_OPCODE(G_STRICT_FPTOUI)
+HANDLE_TARGET_OPCODE(G_STRICT_FPEXT)
+HANDLE_TARGET_OPCODE(G_STRICT_FPTRUNC)
/// read_register intrinsic
HANDLE_TARGET_OPCODE(G_READ_REGISTER)
diff --git a/llvm/include/llvm/Target/GenericOpcodes.td b/llvm/include/llvm/Target/GenericOpcodes.td
index ce4750db88c9a..637eab4ad6c77 100644
--- a/llvm/include/llvm/Target/GenericOpcodes.td
+++ b/llvm/include/llvm/Target/GenericOpcodes.td
@@ -1716,6 +1716,12 @@ def G_STRICT_FREM : ConstrainedInstruction<G_FREM>;
def G_STRICT_FMA : ConstrainedInstruction<G_FMA>;
def G_STRICT_FSQRT : ConstrainedInstruction<G_FSQRT>;
def G_STRICT_FLDEXP : ConstrainedInstruction<G_FLDEXP>;
+def G_STRICT_SITOFP: ConstrainedInstruction<G_SITOFP>;
+def G_STRICT_UITOFP: ConstrainedInstruction<G_UITOFP>;
+def G_STRICT_FPTOSI: ConstrainedInstruction<G_FPTOSI>;
+def G_STRICT_FPTOUI: ConstrainedInstruction<G_FPTOUI>;
+def G_STRICT_FPEXT: ConstrainedInstruction<G_FPEXT>;
+def G_STRICT_FPTRUNC: ConstrainedInstruction<G_FPTRUNC>;
//------------------------------------------------------------------------------
// Memory intrinsics
diff --git a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
index d7280eaba2440..00866db2a943e 100644
--- a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
@@ -2061,6 +2061,18 @@ static unsigned getConstrainedOpcode(Intrinsic::ID ID) {
return TargetOpcode::G_STRICT_FSQRT;
case Intrinsic::experimental_constrained_ldexp:
return TargetOpcode::G_STRICT_FLDEXP;
+ case Intrinsic::experimental_constrained_sitofp:
+ return TargetOpcode::G_STRICT_SITOFP;
+ case Intrinsic::experimental_constrained_uitofp:
+ return TargetOpcode::G_STRICT_UITOFP;
+ case Intrinsic::experimental_constrained_fptosi:
+ return TargetOpcode::G_STRICT_FPTOSI;
+ case Intrinsic::experimental_constrained_fptoui:
+ return TargetOpcode::G_STRICT_FPTOUI;
+ case Intrinsic::experimental_constrained_fpext:
+ return TargetOpcode::G_STRICT_FPEXT;
+ case Intrinsic::experimental_constrained_fptrunc:
+ return TargetOpcode::G_STRICT_FPTRUNC;
default:
return 0;
}
diff --git a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
index 6608b3f2cbefd..536692b435e7a 100644
--- a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
@@ -668,6 +668,18 @@ bool SPIRVInstructionSelector::spvSelect(Register ResVReg,
case TargetOpcode::G_UITOFP:
return selectIToF(ResVReg, ResType, I, false, SPIRV::OpConvertUToF);
+ case TargetOpcode::G_STRICT_SITOFP:
+ return selectUnOp(ResVReg, ResType, I, SPIRV::OpConvertSToF);
+ case TargetOpcode::G_STRICT_UITOFP:
+ return selectUnOp(ResVReg, ResType, I, SPIRV::OpConvertUToF);
+ case TargetOpcode::G_STRICT_FPTOSI:
+ return selectUnOp(ResVReg, ResType, I, SPIRV::OpConvertFToS);
+ case TargetOpcode::G_STRICT_FPTOUI:
+ return selectUnOp(ResVReg, ResType, I, SPIRV::OpConvertFToU);
+ case TargetOpcode::G_STRICT_FPEXT:
+ case TargetOpcode::G_STRICT_FPTRUNC:
+ return selectUnOp(ResVReg, ResType, I, SPIRV::OpFConvert);
+
case TargetOpcode::G_CTPOP:
return selectUnOp(ResVReg, ResType, I, SPIRV::OpBitCount);
case TargetOpcode::G_SMIN:
diff --git a/llvm/lib/Target/SPIRV/SPIRVLegalizerInfo.cpp b/llvm/lib/Target/SPIRV/SPIRVLegalizerInfo.cpp
index 721f64a329d31..d8106f159fab5 100644
--- a/llvm/lib/Target/SPIRV/SPIRVLegalizerInfo.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVLegalizerInfo.cpp
@@ -203,6 +203,17 @@ SPIRVLegalizerInfo::SPIRVLegalizerInfo(const SPIRVSubtarget &ST) {
.legalForCartesianProduct(allIntScalarsAndVectors,
allFloatScalarsAndVectors);
+ getActionDefinitionsBuilder({G_STRICT_SITOFP, G_STRICT_UITOFP})
+ .legalForCartesianProduct(allFloatScalarsAndVectors,
+ allIntScalarsAndVectors);
+
+ getActionDefinitionsBuilder({G_STRICT_FPTOSI, G_STRICT_FPTOUI})
+ .legalForCartesianProduct(allIntScalarsAndVectors,
+ allFloatScalarsAndVectors);
+
+ getActionDefinitionsBuilder({G_STRICT_FPEXT, G_STRICT_FPTRUNC})
+ .legalForCartesianProduct(allFloatScalarsAndVectors);
+
getActionDefinitionsBuilder({G_SITOFP, G_UITOFP})
.legalForCartesianProduct(allFloatScalarsAndVectors,
allScalarsAndVectors);
diff --git a/llvm/test/CodeGen/SPIRV/llvm-intrinsics/constrained-convert.ll b/llvm/test/CodeGen/SPIRV/llvm-intrinsics/constrained-convert.ll
new file mode 100644
index 0000000000000..8ae45e3d8e30d
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/llvm-intrinsics/constrained-convert.ll
@@ -0,0 +1,41 @@
+; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s
+; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val %}
+
+; CHECK-DAG: OpName %[[#sf:]] "conv"
+; CHECK-DAG: OpName %[[#uf:]] "conv1"
+; CHECK-DAG: OpName %[[#fs:]] "conv2"
+; CHECK-DAG: OpName %[[#fu:]] "conv3"
+; CHECK-DAG: OpName %[[#fe:]] "conv4"
+; CHECK-DAG: OpName %[[#ft:]] "conv5"
+
+; CHECK-DAG: OpDecorate %[[#sf]] FPRoundingMode RTE
+; CHECK-DAG: OpDecorate %[[#uf]] FPRoundingMode RTZ
+; CHECK-DAG: OpDecorate %[[#ft]] FPRoundingMode RTP
+; CHECK-NOT: OpDecorate %[[#fs]] FPRoundingMode
+; CHECK-NOT: OpDecorate %[[#fu]] FPRoundingMode
+; CHECK-NOT: OpDecorate %[[#fe]] FPRoundingMode
+
+; CHECK: %[[#sf]] = OpConvertSToF
+; CHECK: %[[#uf]] = OpConvertUToF
+; CHECK: %[[#fs]] = OpConvertFToS
+; CHECK: %[[#fu]] = OpConvertFToU
+; CHECK: %[[#fe]] = OpFConvert
+; CHECK: %[[#ft]] = OpFConvert
+
+define dso_local spir_kernel void @test(float %a, i32 %in, i32 %ui) {
+entry:
+ %conv = tail call float @llvm.experimental.constrained.sitofp.f32.i32(i32 %in, metadata !"round.tonearest", metadata !"fpexcept.strict")
+ %conv1 = tail call float @llvm.experimental.constrained.uitofp.f32.i32(i32 %ui, metadata !"round.towardzero", metadata !"fpexcept.ignore")
+ %conv2 = tail call i32 @llvm.experimental.constrained.fptosi.i32.f32(float %conv1, metadata !"fpexcept.ignore")
+ %conv3 = tail call i32 @llvm.experimental.constrained.fptoui.i32.f32(float %conv1, metadata !"fpexcept.ignore")
+ %conv4 = tail call double @llvm.experimental.constrained.fpext.f64.f32(float %conv1, metadata !"fpexcept.ignore")
+ %conv5 = tail call float @llvm.experimental.constrained.fptrunc.f32.f64(double %conv4, metadata !"round.upward", metadata !"fpexcept.ignore")
+ ret void
+}
+
+declare float @llvm.experimental.constrained.sitofp.f32.i32(i32, metadata, metadata)
+declare float @llvm.experimental.constrained.uitofp.f32.i32(i32, metadata, metadata)
+declare i32 @llvm.experimental.constrained.fptosi.i32.f32(float, metadata)
+declare i32 @llvm.experimental.constrained.fptoui.i32.f32(float, metadata)
+declare double @llvm.experimental.constrained.fpext.f64.f32(float, metadata)
+declare float @llvm.experimental.constrained.fptrunc.f32.f64(double, metadata, metadata)
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
a85fd2f to
0b4d0d7
Compare
|
|
||
| define dso_local spir_kernel void @test(float %a, i32 %in, i32 %ui) { | ||
| entry: | ||
| %conv = tail call float @llvm.experimental.constrained.sitofp.f32.i32(i32 %in, metadata !"round.tonearest", metadata !"fpexcept.strict") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test one case per function, and test all the combinations of types, and vectors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’ve updated the test as requested:
- Split the test into one case per function.
- Added coverage for different type combinations and vector variants.
- Simplified the RUN lines by removing -verify-machineinstrs and -O0.
| @@ -0,0 +1,41 @@ | |||
| ; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't need -verify-machineinstrs, and why -O0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thankyou @arsenm I’ll drop -verify-machineinstrs since it isn’t needed, and I’ll also remove -O0 so the test just uses the default optimization level.
3b27a3a to
f441f52
Compare
| ; CHECK-DAG: OpDecorate %[[#ft1]] FPRoundingMode RTZ | ||
| ; CHECK-DAG: OpDecorate %[[#ft2]] FPRoundingMode RTE | ||
|
|
||
| define dso_local spir_kernel void @test1(i32 %in) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't need dso_local, Also would be better to use a non-kernel and have a real use of the return value
| ; CHECK-DAG: OpFConvert %[[#]] %[[#]] | ||
| ; CHECK-DAG: OpFConvert %[[#]] %[[#]] | ||
|
|
||
| ; CHECK-DAG: OpDecorate %[[#sf]] FPRoundingMode RTE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thes should have a definitive order, and ideally the checks would be in the function they are testing. Can you just use update_llc_test_checks
| %conv20 = tail call double @llvm.experimental.constrained.fpext.f64.f32(float %in, metadata !"fpexcept.strict") | ||
| ret void | ||
| } | ||
| define dso_local spir_kernel void @test22(<4 x float> %in) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you name the combination of types in the function names instead of just test
db3f26d to
e0e7243
Compare
| } | ||
|
|
||
| define float @sitofp_f32_i16(i16 %in) { | ||
| ; CHECK-DAG: OpName %[[#]] "conv1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No check label, and don't need CHECK-DAG. Can you just use update_llc_test_checks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@arsenm I tried running utils/update_llc_test_checks.py on constrained-convert.ll. Instead of regenerating the CHECK: directives, it only inserted the autogenerated note at the top and the file end marker, while removing the existing checks in between. Could you clarify the correct way to use the script here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did it print a warning on error? Somehow only 2 spirv tests are using update_llc_test_checks. Is it getting confused by the second run line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@arsenm Yes, it did print a warning message: WARNING: Skipping non-llc RUN line.
When I removed the second RUN line, the warning disappeared, but the issue still persists — the script removes the existing CHECK: directives and only adds the autogenerated note at the top and the file end marker.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gentle reminder @arsenm any updates on this PR?
Added SPIR-V support for constrained conversion intrinsics (sitofp, uitofp, fptosi, fptoui, fpext, fptrunc), including legalization, instruction selection, and corresponding tests.