[SPIR-V] Lower vector-typed GEPs with more than 1 element in SPIRVEmitIntrinsics#197101
[SPIR-V] Lower vector-typed GEPs with more than 1 element in SPIRVEmitIntrinsics#197101aobolensk wants to merge 5 commits into
Conversation
|
@llvm/pr-subscribers-backend-spir-v Author: Arseniy Obolenskiy (aobolensk) ChangesThe spv_gep intrinsic expects a scalar pointer return, so a GEP vector result ( Resolves #186764 Full diff: https://github.com/llvm/llvm-project/pull/197101.diff 4 Files Affected:
diff --git a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
index b6e71c7b76348..42c2920fa5de0 100644
--- a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
@@ -1864,6 +1864,49 @@ Instruction *SPIRVEmitIntrinsics::visitGetElementPtrInst(GetElementPtrInst &I) {
IRBuilder<> B(I.getParent());
B.SetInsertPoint(&I);
+ // OpPtrAccessChain requires a scalar pointer result; scalarize per-lane
+ // GEPs that return <N x ptr> and rebuild the vector via insertelement.
+ if (auto *RetVTy = dyn_cast<FixedVectorType>(I.getType())) {
+ unsigned N = RetVTy->getNumElements();
+ Value *PtrOp = I.getPointerOperand();
+ bool PtrIsVec = isa<VectorType>(PtrOp->getType());
+ Type *ResultPtrTy = RetVTy->getElementType();
+ SmallVector<Type *, 2> Types = {ResultPtrTy,
+ PtrOp->getType()->getScalarType()};
+ Value *InBounds = B.getInt1(I.isInBounds());
+
+ Value *VecResult = PoisonValue::get(RetVTy);
+ for (unsigned Lane = 0; Lane < N; ++Lane) {
+ Value *LaneIdx = B.getInt32(Lane);
+ Value *ScalarPtr =
+ PtrIsVec ? B.CreateExtractElement(PtrOp, LaneIdx) : PtrOp;
+ SmallVector<Value *, 4> Args;
+ Args.push_back(InBounds);
+ Args.push_back(ScalarPtr);
+ for (Value *Idx : I.indices()) {
+ if (isa<VectorType>(Idx->getType()))
+ Args.push_back(B.CreateExtractElement(Idx, LaneIdx));
+ else
+ Args.push_back(Idx);
+ }
+ Value *ScalarGep = B.CreateIntrinsic(Intrinsic::spv_gep, Types, Args);
+ VecResult = B.CreateInsertElement(VecResult, ScalarGep, LaneIdx);
+ }
+
+ auto *NewI = cast<Instruction>(VecResult);
+ replaceAllUsesWithAndErase(B, &I, NewI);
+
+ if (CallInst *AssignCI = GR->findAssignPtrTypeInstr(NewI)) {
+ Type *TypedElem = TypedPointerType::get(
+ I.getSourceElementType(),
+ cast<PointerType>(ResultPtrTy)->getAddressSpace());
+ Type *TypedVec = VectorType::get(TypedElem, RetVTy->getElementCount());
+ GR->updateAssignType(AssignCI, NewI, PoisonValue::get(TypedVec));
+ }
+
+ return NewI;
+ }
+
if (TM.getSubtargetImpl()->isLogicalSPIRV() && !isFirstIndexZero(&I)) {
// Logical SPIR-V cannot use the OpPtrAccessChain instruction. If the first
// index of the GEP is not 0, then we need to try to adjust it.
diff --git a/llvm/test/CodeGen/SPIRV/extensions/SPV_INTEL_masked_gather_scatter/vector-of-pointers-gep-no-extension.ll b/llvm/test/CodeGen/SPIRV/extensions/SPV_INTEL_masked_gather_scatter/vector-of-pointers-gep-no-extension.ll
new file mode 100644
index 0000000000000..ca28f43a1b038
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/extensions/SPV_INTEL_masked_gather_scatter/vector-of-pointers-gep-no-extension.ll
@@ -0,0 +1,30 @@
+; RUN: not llc -O0 -mtriple=spirv64-unknown-unknown %s -o /dev/null 2>&1 | FileCheck %s
+
+; CHECK-NOT: {{[Ii]}}ntrinsic has incorrect return type
+; CHECK: error:{{.*}}Vector of pointers requires SPV_INTEL_masked_gather_scatter extension
+
+; The <1 x ptr> case used to fall through to the scalar spv_gep emission
+; path, where the i32 vector return type tripped the same IR verifier check.
+define spir_kernel void @test_vector_gep_v1(ptr addrspace(1) %p, ptr addrspace(1) %out) {
+ %gep = getelementptr i32, ptr addrspace(1) %p, <1 x i64> <i64 5>
+ %elem = extractelement <1 x ptr addrspace(1)> %gep, i32 0
+ %val = load i32, ptr addrspace(1) %elem
+ store i32 %val, ptr addrspace(1) %out
+ ret void
+}
+
+define spir_kernel void @test_vector_gep_v2(ptr addrspace(1) %p, ptr addrspace(1) %out) {
+ %gep = getelementptr i32, ptr addrspace(1) %p, <2 x i64> zeroinitializer
+ %elem = extractelement <2 x ptr addrspace(1)> %gep, i32 0
+ %val = load i32, ptr addrspace(1) %elem
+ store i32 %val, ptr addrspace(1) %out
+ ret void
+}
+
+define spir_kernel void @test_vector_gep_v4(ptr addrspace(1) %p, ptr addrspace(1) %out) {
+ %gep = getelementptr i32, ptr addrspace(1) %p, <4 x i64> zeroinitializer
+ %elem = extractelement <4 x ptr addrspace(1)> %gep, i32 0
+ %val = load i32, ptr addrspace(1) %elem
+ store i32 %val, ptr addrspace(1) %out
+ ret void
+}
diff --git a/llvm/test/CodeGen/SPIRV/extensions/SPV_INTEL_masked_gather_scatter/vector-of-pointers-gep.ll b/llvm/test/CodeGen/SPIRV/extensions/SPV_INTEL_masked_gather_scatter/vector-of-pointers-gep.ll
new file mode 100644
index 0000000000000..529cd843e71b7
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/extensions/SPV_INTEL_masked_gather_scatter/vector-of-pointers-gep.ll
@@ -0,0 +1,53 @@
+; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv64-unknown-unknown --spirv-ext=+SPV_INTEL_masked_gather_scatter %s -o - | FileCheck %s
+; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown --spirv-ext=+SPV_INTEL_masked_gather_scatter %s -o - -filetype=obj | spirv-val %}
+
+; CHECK-DAG: %[[#I32:]] = OpTypeInt 32 0
+; CHECK-DAG: %[[#PTR:]] = OpTypePointer CrossWorkgroup %[[#I32]]
+; CHECK-DAG: %[[#VPTR2:]] = OpTypeVector %[[#PTR]] 2
+; CHECK-DAG: %[[#VPTR4:]] = OpTypeVector %[[#PTR]] 4
+
+; The <1 x ptr> GEP collapses to a single scalar OpPtrAccessChain; no
+; vector-of-pointers value is materialized.
+; CHECK: OpFunction
+; CHECK: OpPtrAccessChain %[[#PTR]]
+; CHECK-NOT: OpCompositeInsert
+; CHECK: OpFunctionEnd
+define spir_kernel void @test_vector_gep_v1(ptr addrspace(1) %p, ptr addrspace(1) %out) {
+ %gep = getelementptr i32, ptr addrspace(1) %p, <1 x i64> <i64 5>
+ %elem = extractelement <1 x ptr addrspace(1)> %gep, i32 0
+ %val = load i32, ptr addrspace(1) %elem
+ store i32 %val, ptr addrspace(1) %out
+ ret void
+}
+
+; CHECK: OpFunction
+; CHECK: OpPtrAccessChain %[[#PTR]]
+; CHECK: OpCompositeInsert %[[#VPTR2]]
+; CHECK: OpPtrAccessChain %[[#PTR]]
+; CHECK: OpCompositeInsert %[[#VPTR2]]
+; CHECK: OpFunctionEnd
+define spir_kernel void @test_vector_gep_v2(ptr addrspace(1) %p, ptr addrspace(1) %out) {
+ %gep = getelementptr i32, ptr addrspace(1) %p, <2 x i64> zeroinitializer
+ %elem = extractelement <2 x ptr addrspace(1)> %gep, i32 0
+ %val = load i32, ptr addrspace(1) %elem
+ store i32 %val, ptr addrspace(1) %out
+ ret void
+}
+
+; CHECK: OpFunction
+; CHECK: OpPtrAccessChain %[[#PTR]]
+; CHECK: OpCompositeInsert %[[#VPTR4]]
+; CHECK: OpPtrAccessChain %[[#PTR]]
+; CHECK: OpCompositeInsert %[[#VPTR4]]
+; CHECK: OpPtrAccessChain %[[#PTR]]
+; CHECK: OpCompositeInsert %[[#VPTR4]]
+; CHECK: OpPtrAccessChain %[[#PTR]]
+; CHECK: OpCompositeInsert %[[#VPTR4]]
+; CHECK: OpFunctionEnd
+define spir_kernel void @test_vector_gep_v4(ptr addrspace(1) %p, ptr addrspace(1) %out) {
+ %gep = getelementptr i32, ptr addrspace(1) %p, <4 x i64> zeroinitializer
+ %elem = extractelement <4 x ptr addrspace(1)> %gep, i32 0
+ %val = load i32, ptr addrspace(1) %elem
+ store i32 %val, ptr addrspace(1) %out
+ ret void
+}
diff --git a/llvm/test/CodeGen/SPIRV/pointers/getelementptr-vector-index.ll b/llvm/test/CodeGen/SPIRV/pointers/getelementptr-vector-index.ll
index 0f710f9827679..a27b77e0929fb 100644
--- a/llvm/test/CodeGen/SPIRV/pointers/getelementptr-vector-index.ll
+++ b/llvm/test/CodeGen/SPIRV/pointers/getelementptr-vector-index.ll
@@ -3,15 +3,11 @@
; CHECK-DAG: %[[#INT32:]] = OpTypeInt 32 0
; CHECK-DAG: %[[#PTR_INT32:]] = OpTypePointer CrossWorkgroup %[[#INT32]]
-; CHECK-DAG: %[[#INT8:]] = OpTypeInt 8 0
-; CHECK-DAG: %[[#PTR_INT8:]] = OpTypePointer CrossWorkgroup %[[#INT8]]
; CHECK-DAG: %[[#INT64:]] = OpTypeInt 64 0
; CHECK-DAG: %[[#CONST_0:]] = OpConstantNull %[[#INT64]]
; CHECK-LABEL: Begin function test_vector_gep_with_load
-; CHECK: %[[#BC1:]] = OpBitcast %[[#PTR_INT8]] %[[#]]
-; CHECK: %[[#GEP:]] = OpPtrAccessChain %[[#PTR_INT8]] %[[#BC1]] %[[#CONST_0]]
-; CHECK: %[[#BC2:]] = OpBitcast %[[#PTR_INT32]] %[[#GEP]]
-; CHECK: %[[#VAL:]] = OpLoad %[[#INT32]] %[[#BC2]]
+; CHECK: %[[#GEP:]] = OpPtrAccessChain %[[#PTR_INT32]] %[[#]] %[[#CONST_0]]
+; CHECK: %[[#VAL:]] = OpLoad %[[#INT32]] %[[#GEP]]
; CHECK: OpStore %[[#]] %[[#VAL]]
; CHECK: OpFunctionEnd
define spir_kernel void @test_vector_gep_with_load(ptr addrspace(1) %p, ptr addrspace(1) %out) {
|
🪟 Windows x64 Test Results
✅ The build succeeded and all tests passed. |
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
The spv_gep intrinsic expects a scalar pointer return, so a GEP vector result (<N x ptr>) failed the IR verifier. Split such GEPs into one spv_gep per lane and stitch the result back with insertelement Resolves llvm#186764
cdd9dd0 to
badf58c
Compare
| ; CHECK: OpFunctionEnd | ||
| define spir_kernel void @test_vector_gep_v2(ptr addrspace(1) %p, ptr addrspace(1) %out) { | ||
| %gep = getelementptr i32, ptr addrspace(1) %p, <2 x i64> zeroinitializer | ||
| %elem = extractelement <2 x ptr addrspace(1)> %gep, i32 0 |
There was a problem hiding this comment.
Worth checking if we extract element from a valid object.
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
7c0118c to
8833be4
Compare
| ; CHECK-NEXT: OpStore %[[#OUT1]] %[[#VAL1]] | ||
| ; CHECK-NEXT: OpReturn | ||
| ; CHECK-NEXT: OpFunctionEnd | ||
| ; CHECK-NOT: OpCompositeInsert |
There was a problem hiding this comment.
We check, that OpCompositeInsert is not placed after OpFunctionEnd at L29 and before OpFunction at L39?
| ; CHECK: %[[#GEP:]] = OpPtrAccessChain %[[#PTR_INT32]] %[[#]] %[[#CONST_0]] | ||
| ; CHECK: %[[#VAL:]] = OpLoad %[[#INT32]] %[[#GEP]] |
There was a problem hiding this comment.
Should we also check no Bitcasts?
| Argument *Arg = F.getArg(ArgIdx); | ||
| if (auto *VTy = dyn_cast<FixedVectorType>(OriginalArgType); | ||
| VTy && isUntypedPointerTy(VTy->getElementType())) | ||
| if (Type *ElemTy = GR->findDeducedElementType(Arg)) |
There was a problem hiding this comment.
No test covers the case where a vector-of-pointers argument is used by GEPs with different element types.
There was a problem hiding this comment.
Uncovered scenario for now. Should we cover it here directly?
Fix spirv-val concern in the IR from #186764 with vector-typed GEPs (
<N x ptr>) that were lowered to a single spv_gep intrinsic with a vector returnResolves #186764