[SPIR-V] Implement builtins for OpIAddCarry/OpISubBorrow and improve/fix type inference #115192
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-backend-spir-v Author: Vyacheslav Levytskyy (VyacheslavLevytskyy) ChangesThis PR is to fix early definition of SPIR-V types during call lowering. Namely, the goal of the test case is to ensure that correct types are applied to virtual registers which were used as arguments in call lowering and so caused early definition of SPIR-V types. Reproducers are attached as a new test cases. Full diff: https://github.com/llvm/llvm-project/pull/115192.diff 4 Files Affected:
diff --git a/llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp b/llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp
index 98cf598a1f031a..8d1b82465d3df2 100644
--- a/llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp
@@ -546,12 +546,35 @@ bool SPIRVCallLowering::lowerCall(MachineIRBuilder &MIRBuilder,
ArgVRegs.push_back(ArgReg);
SPIRVType *SpvType = GR->getSPIRVTypeForVReg(ArgReg);
if (!SpvType) {
- SpvType = GR->getOrCreateSPIRVType(Arg.Ty, MIRBuilder);
- GR->assignSPIRVTypeToVReg(SpvType, ArgReg, MF);
+ Type *ArgTy = nullptr;
+ if (auto *PtrArgTy = dyn_cast<PointerType>(Arg.Ty)) {
+ // If Arg.Ty is an untyped pointer (i.e., ptr [addrspace(...)]) and we
+ // don't have access to original value in LLVM IR or info about
+ // deduced pointee type, then we should wait with setting the type for
+ // the virtual register until pre-legalizer step when we access
+ // @llvm.spv.assign.ptr.type.p...(...)'s info.
+ if (Arg.OrigValue)
+ if (Type *ElemTy = GR->findDeducedElementType(Arg.OrigValue))
+ ArgTy = TypedPointerType::get(ElemTy, PtrArgTy->getAddressSpace());
+ } else {
+ ArgTy = Arg.Ty;
+ }
+ if (ArgTy) {
+ SpvType = GR->getOrCreateSPIRVType(ArgTy, MIRBuilder);
+ GR->assignSPIRVTypeToVReg(SpvType, ArgReg, MF);
+ }
}
if (!MRI->getRegClassOrNull(ArgReg)) {
- MRI->setRegClass(ArgReg, GR->getRegClass(SpvType));
- MRI->setType(ArgReg, GR->getRegType(SpvType));
+ // Either we have SpvType created, or Arg.Ty is an untyped pointer and
+ // we know its virtual register's class and type even if we don't know
+ // pointee type.
+ MRI->setRegClass(ArgReg, SpvType ? GR->getRegClass(SpvType)
+ : &SPIRV::pIDRegClass);
+ MRI->setType(
+ ArgReg,
+ SpvType ? GR->getRegType(SpvType)
+ : LLT::pointer(cast<PointerType>(Arg.Ty)->getAddressSpace(),
+ GR->getPointerSize()));
}
}
auto instructionSet = canUseOpenCL ? SPIRV::InstructionSet::OpenCL_std
diff --git a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
index 8b7e9c48de6c75..191533a52cccd9 100644
--- a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
@@ -1219,6 +1219,8 @@ void SPIRVEmitIntrinsics::replacePointerOperandWithPtrCast(
SmallVector<Value *, 2> Args = {Pointer, VMD, B.getInt32(AddressSpace)};
auto *PtrCastI = B.CreateIntrinsic(Intrinsic::spv_ptrcast, {Types}, Args);
I->setOperand(OperandToReplace, PtrCastI);
+ // We need to set up a pointee type for the newly created spv_ptrcast.
+ buildAssignPtr(B, ExpectedElementType, PtrCastI);
}
void SPIRVEmitIntrinsics::insertPtrCastOrAssignTypeInstr(Instruction *I,
diff --git a/llvm/test/CodeGen/SPIRV/pointers/phi-valid-operand-types-vs-calllowering-unwrapped.ll b/llvm/test/CodeGen/SPIRV/pointers/phi-valid-operand-types-vs-calllowering-unwrapped.ll
new file mode 100644
index 00000000000000..09c0a92d596ce3
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/pointers/phi-valid-operand-types-vs-calllowering-unwrapped.ll
@@ -0,0 +1,50 @@
+; The goal of the test case is to ensure that correct types are applied to virtual registers
+; which were used as arguments in call lowering and so caused early definition of SPIR-V types.
+
+; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val %}
+
+%t_id = type { %t_arr }
+%t_arr = type { [1 x i64] }
+%t_bf16 = type { i16 }
+
+define weak_odr dso_local spir_kernel void @foo(ptr addrspace(1) align 4 %_arg_ERR, ptr byval(%t_id) align 8 %_arg_ERR3) {
+entry:
+ %FloatArray.i = alloca [4 x float], align 4
+ %BF16Array.i = alloca [4 x %t_bf16], align 2
+ %0 = load i64, ptr %_arg_ERR3, align 8
+ %add.ptr.i = getelementptr inbounds i32, ptr addrspace(1) %_arg_ERR, i64 %0
+ %FloatArray.ascast.i = addrspacecast ptr %FloatArray.i to ptr addrspace(4)
+ %BF16Array.ascast.i = addrspacecast ptr %BF16Array.i to ptr addrspace(4)
+ call spir_func void @__devicelib_ConvertFToBF16INTELVec4(ptr addrspace(4) %FloatArray.ascast.i, ptr addrspace(4) %BF16Array.ascast.i)
+ br label %for.cond.i
+
+for.cond.i: ; preds = %for.inc.i, %entry
+ %lsr.iv1 = phi ptr [ %scevgep2, %for.inc.i ], [ %FloatArray.i, %entry ]
+ %lsr.iv = phi ptr addrspace(4) [ %scevgep, %for.inc.i ], [ %BF16Array.ascast.i, %entry ]
+ %i.0.i = phi i32 [ 0, %entry ], [ %inc.i, %for.inc.i ]
+ %cmp.i = icmp ult i32 %i.0.i, 4
+ br i1 %cmp.i, label %for.body.i, label %exit
+
+for.body.i: ; preds = %for.cond.i
+ %1 = load float, ptr %lsr.iv1, align 4
+ %call.i.i = call spir_func float @__devicelib_ConvertBF16ToFINTEL(ptr addrspace(4) align 2 dereferenceable(2) %lsr.iv)
+ %cmp5.i = fcmp une float %1, %call.i.i
+ br i1 %cmp5.i, label %if.then.i, label %for.inc.i
+
+if.then.i: ; preds = %for.body.i
+ store i32 1, ptr addrspace(1) %add.ptr.i, align 4
+ br label %for.inc.i
+
+for.inc.i: ; preds = %if.then.i, %for.body.i
+ %inc.i = add nuw nsw i32 %i.0.i, 1
+ %scevgep = getelementptr i8, ptr addrspace(4) %lsr.iv, i64 2
+ %scevgep2 = getelementptr i8, ptr %lsr.iv1, i64 4
+ br label %for.cond.i
+
+exit: ; preds = %for.cond.i
+ ret void
+}
+
+declare void @llvm.memcpy.p0.p1.i64(ptr noalias nocapture writeonly, ptr addrspace(1) noalias nocapture readonly, i64, i1 immarg)
+declare dso_local spir_func void @__devicelib_ConvertFToBF16INTELVec4(ptr addrspace(4), ptr addrspace(4))
+declare dso_local spir_func float @__devicelib_ConvertBF16ToFINTEL(ptr addrspace(4) align 2 dereferenceable(2))
diff --git a/llvm/test/CodeGen/SPIRV/pointers/phi-valid-operand-types-vs-calllowering.ll b/llvm/test/CodeGen/SPIRV/pointers/phi-valid-operand-types-vs-calllowering.ll
new file mode 100644
index 00000000000000..a31638e0b87043
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/pointers/phi-valid-operand-types-vs-calllowering.ll
@@ -0,0 +1,49 @@
+; The goal of the test case is to ensure that correct types are applied to virtual registers
+; which were used as arguments in call lowering and so caused early definition of SPIR-V types.
+
+; RUN: %if spirv-tools %{ llc -O2 -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val %}
+
+%t_id = type { %t_arr }
+%t_arr = type { [1 x i64] }
+%t_bf16 = type { i16 }
+
+define weak_odr dso_local spir_kernel void @foo(ptr addrspace(1) align 4 %_arg_ERR, ptr byval(%t_id) align 8 %_arg_ERR3) {
+entry:
+ %FloatArray.i = alloca [4 x float], align 4
+ %BF16Array.i = alloca [4 x %t_bf16], align 2
+ %0 = load i64, ptr %_arg_ERR3, align 8
+ %add.ptr.i = getelementptr inbounds i32, ptr addrspace(1) %_arg_ERR, i64 %0
+ %FloatArray.ascast.i = addrspacecast ptr %FloatArray.i to ptr addrspace(4)
+ %BF16Array.ascast.i = addrspacecast ptr %BF16Array.i to ptr addrspace(4)
+ call spir_func void @__devicelib_ConvertFToBF16INTELVec4(ptr addrspace(4) %FloatArray.ascast.i, ptr addrspace(4) %BF16Array.ascast.i)
+ br label %for.cond.i
+
+for.cond.i: ; preds = %for.inc.i, %entry
+ %i.0.i = phi i32 [ 0, %entry ], [ %inc.i, %for.inc.i ]
+ %cmp.i = icmp ult i32 %i.0.i, 4
+ br i1 %cmp.i, label %for.body.i, label %exit
+
+for.body.i: ; preds = %for.cond.i
+ %idxprom.i = zext nneg i32 %i.0.i to i64
+ %arrayidx.i = getelementptr inbounds [4 x float], ptr %FloatArray.i, i64 0, i64 %idxprom.i
+ %1 = load float, ptr %arrayidx.i, align 4
+ %arrayidx4.i = getelementptr inbounds [4 x %t_bf16], ptr addrspace(4) %BF16Array.ascast.i, i64 0, i64 %idxprom.i
+ %call.i.i = call spir_func float @__devicelib_ConvertBF16ToFINTEL(ptr addrspace(4) align 2 dereferenceable(2) %arrayidx4.i)
+ %cmp5.i = fcmp une float %1, %call.i.i
+ br i1 %cmp5.i, label %if.then.i, label %for.inc.i
+
+if.then.i: ; preds = %for.body.i
+ store i32 1, ptr addrspace(1) %add.ptr.i, align 4
+ br label %for.inc.i
+
+for.inc.i: ; preds = %if.then.i, %for.body.i
+ %inc.i = add nuw nsw i32 %i.0.i, 1
+ br label %for.cond.i
+
+exit: ; preds = %for.cond.i
+ ret void
+}
+
+declare void @llvm.memcpy.p0.p1.i64(ptr noalias nocapture writeonly, ptr addrspace(1) noalias nocapture readonly, i64, i1 immarg)
+declare dso_local spir_func void @__devicelib_ConvertFToBF16INTELVec4(ptr addrspace(4), ptr addrspace(4))
+declare dso_local spir_func float @__devicelib_ConvertBF16ToFINTEL(ptr addrspace(4) align 2 dereferenceable(2))
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
VyacheslavLevytskyy
changed the title
VyacheslavLevytskyy
changed the title
… used as arguments in call lowering and so caused early definition of SPIR-V types
…ss to erased from parent after visit instructions; fix validation of pointer types for frexp and lgamma_r; improve parsing of builtin names
…en applied to bool type
VyacheslavLevytskyy
force-pushed
the
fix_phi_and_builtins_0611
branch
from
2bce913 to
980f2b6
Compare
… in arithmetic with overflow instructions
michalpaszkowski
approved these changes
Nov 14, 2024
There was a problem hiding this comment.
LGTM! Thank you for the patch! This also fixes the issue #111818
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is to solve several intertwined issues with type inference while adding support for builtins for OpIAddCarry and OpISubBorrow:
__spirv_...-- introduces a new element to account for, namely,ptr sret (%struct) %0argument that is a place to put a result of the instruction;"anon<int, int> __spirv_IAddCarry<int, int>(int, int)"that was incorrectly parsed asanonbefore the PR);SPIRVEmitIntrinsics::replaceAllUsesWith()that fixes above mentioned problems;storageClassToAddressSpace(SPIRV::StorageClass::Function),