diff --git a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp index 2a46f2f2ed445b..8b5a18aca1e820 100644 --- a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp +++ b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp @@ -937,6 +937,22 @@ Type *SPIRVEmitIntrinsicsImpl::getGEPType(GetElementPtrInst *Ref) { return getGEPTypeLogical(Ref); } + // For structs whose sole field is a pointer, derive the GEP element type + // from the first field directly. This handles any struct-wrapped pointer + // (e.g. VulkanBuffer descriptors, PhysicalStorageBuffer wrappers) without + // relying on LDC-specific struct naming conventions. + Type *SrcTy = Ref->getSourceElementType(); + if (auto *STy = dyn_cast(SrcTy)) { + if (STy->getNumElements() == 1) { + Type *FieldTy = STy->getElementType(0); + if (FieldTy->isPointerTy() && Ref->getNumIndices() > 1) { + return getTypedPointerWrapper( + Ref->getResultElementType(), + getPointerAddressSpace(Ref->getType())); + } + } + } + Type *Ty = nullptr; // TODO: not sure if GetElementPtrInst::getTypeAtIndex() does anything // useful here @@ -3676,6 +3692,18 @@ bool SPIRVEmitIntrinsicsImpl::runOnFunction(Function &Func) { const SPIRVSubtarget &ST = TM.getSubtarget(Func); GR = ST.getSPIRVGlobalRegistry(); + for (auto &I : instructions(Func)) { + auto *II = dyn_cast(&I); + if (!II || II->getIntrinsicID() != Intrinsic::spv_assign_ptr_type) + continue; + Value *Arg = II->getOperand(0); + MetadataAsValue *VMD = cast(II->getOperand(1)); + Type *ElemTy = cast(VMD->getMetadata())->getType(); + GR->addDeducedElementType(Arg, ElemTy); + GR->addDeducedElementType(II, ElemTy); + GR->addAssignPtrTypeInstr(Arg, II); + } + if (!CurrF) HaveFunPtrs = ST.canUseExtension(SPIRV::Extension::SPV_INTEL_function_pointers); diff --git a/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp b/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp index 021bc1806d284c..fea1e4474e12c5 100644 --- a/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp +++ b/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp @@ -2326,7 +2326,13 @@ void SPIRVGlobalRegistry::buildMemAliasingOpDecorate( } void SPIRVGlobalRegistry::replaceAllUsesWith(Value *Old, Value *New, bool DeleteOld) { - Old->replaceAllUsesWith(New); + if (Old->getType() == New->getType()) { + Old->replaceAllUsesWith(New); + } else { + llvm::SmallVector LocalUsers(Old->users()); + for (llvm::User *U : LocalUsers) + U->replaceUsesOfWith(Old, New); + } updateIfExistDeducedElementType(Old, New, DeleteOld); updateIfExistAssignPtrTypeInstr(Old, New, DeleteOld); } diff --git a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp index 35242809b76b11..09510828ac7702 100644 --- a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp +++ b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp @@ -1980,8 +1980,8 @@ static void addMemoryOperands(MachineMemOperand *MemOp, SpvMemOp |= static_cast(SPIRV::MemoryOperand::Volatile); if (MemOp->isNonTemporal()) SpvMemOp |= static_cast(SPIRV::MemoryOperand::Nontemporal); - // Aligned memory operand requires the Kernel capability. - if (!ST->isShader() && MemOp->getAlign().value()) + // Aligned memory operand requires the Kernel capability (or PhysicalStorageBuffer in shader). + if ((!ST->isShader() || MemOp->getAddrSpace() == 1) && MemOp->getAlign().value()) SpvMemOp |= static_cast(SPIRV::MemoryOperand::Aligned); [[maybe_unused]] MachineInstr *AliasList = nullptr; @@ -5115,12 +5115,19 @@ bool SPIRVInstructionSelector::selectGEP(Register ResVReg, Opcode == SPIRV::OpUntypedAccessChainKHR || Opcode == SPIRV::OpUntypedInBoundsAccessChainKHR); - assert((!IsAccessChainOpcode || (getImm(I.getOperand(4), MRI) && - foldImm(I.getOperand(4), MRI) == 0)) && - "Cannot translate GEP to OpAccessChain."); + assert((!IsAccessChainOpcode || + GR.getPointerStorageClass(ResType) == + SPIRV::StorageClass::PhysicalStorageBufferEXT || + (getImm(I.getOperand(4), MRI) && foldImm(I.getOperand(4), MRI) == 0)) && + "Cannot translate GEP to OpAccessChain. First index must be 0."); // Adding indices. - const unsigned StartingIndex = IsAccessChainOpcode ? 5 : 4; + const unsigned StartingIndex = + (IsAccessChainOpcode && + GR.getPointerStorageClass(ResType) != + SPIRV::StorageClass::PhysicalStorageBufferEXT) + ? 5 + : 4; for (unsigned i = StartingIndex; i < I.getNumExplicitOperands(); ++i) Res.addUse(I.getOperand(i).getReg()); Res.constrainAllUses(TII, TRI, RBI); diff --git a/llvm/lib/Target/SPIRV/SPIRVLegalizePointerCast.cpp b/llvm/lib/Target/SPIRV/SPIRVLegalizePointerCast.cpp index 228e975a3711c8..22781604019416 100644 --- a/llvm/lib/Target/SPIRV/SPIRVLegalizePointerCast.cpp +++ b/llvm/lib/Target/SPIRV/SPIRVLegalizePointerCast.cpp @@ -426,8 +426,17 @@ class SPIRVLegalizePointerCastImpl { if (!ResultOpt) { if (tryReinterpretLoad(B, ElementType, Source, CastedPtr, IllegalLoad)) return nullptr; - llvm_unreachable("Failed to load from aggregate: " - "Could not find compatible memory layout."); + + SmallVector Args = {B.getInt1(false), Source, B.getInt32(0), + B.getInt32(0)}; + std::array Types = {Source->getType(), Source->getType()}; + Value *GEP = B.CreateIntrinsic(Intrinsic::spv_gep, {Types}, {Args}); + GR->buildAssignPtr(B, ElementType, GEP); + + LoadInst *LI = B.CreateLoad(ElementType, GEP); + LI->setAlignment(IllegalLoad->getAlign()); + buildAssignType(B, ElementType, LI); + return LI; } auto [GEP, CurrentTy] = *ResultOpt; @@ -731,8 +740,16 @@ class SPIRVLegalizePointerCastImpl { if (tryReinterpretStore(B, Src->getType(), Dst, CastedPtr, Src, Alignment)) return; - llvm_unreachable("Failed to store to aggregate: " - "Could not find compatible memory layout."); + + SmallVector Args = {B.getInt1(true), Dst, B.getInt32(0), + B.getInt32(0)}; + std::array Types = {Dst->getType(), Dst->getType()}; + Value *GEP = B.CreateIntrinsic(Intrinsic::spv_gep, {Types}, {Args}); + GR->buildAssignPtr(B, Src->getType(), GEP); + + StoreInst *SI = B.CreateStore(Src, GEP); + SI->setAlignment(Alignment); + return; } auto [GEP, CurrentTy] = *ResultOpt; diff --git a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp index 39918b6610fce1..121e722110d02c 100644 --- a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp +++ b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp @@ -141,6 +141,30 @@ getSymbolicOperandRequirements(SPIRV::OperandCategory::OperandCategory Category, return {false, {}, {}, VersionTuple(), VersionTuple()}; } +static bool usesPhysicalStorageBuffer(const llvm::Module &M) { + auto isAddrSpace1PtrTy = [](const Type *Ty) { + return isPointerTy(Ty) && getPointerAddressSpace(Ty) == 1; + }; + + for (const llvm::Function &F : M) { + for (const llvm::Argument &Arg : F.args()) { + if (isAddrSpace1PtrTy(Arg.getType())) + return true; + } + for (const llvm::BasicBlock &BB : F) { + for (const llvm::Instruction &I : BB) { + if (isAddrSpace1PtrTy(I.getType())) + return true; + } + } + } + for (const llvm::GlobalVariable &GV : M.globals()) { + if (isAddrSpace1PtrTy(GV.getType())) + return true; + } + return false; +} + void SPIRVModuleAnalysis::setBaseInfo(const Module &M) { MAI.MaxID = 0; for (int i = 0; i < SPIRV::NUM_MODULE_SECTIONS; i++) @@ -170,8 +194,11 @@ void SPIRVModuleAnalysis::setBaseInfo(const Module &M) { : PtrSize == 64 ? SPIRV::AddressingModel::Physical64 : SPIRV::AddressingModel::Logical; } else { - // TODO: Add support for PhysicalStorageBufferAddress. - MAI.Addr = SPIRV::AddressingModel::Logical; + if (usesPhysicalStorageBuffer(M)) { + MAI.Addr = SPIRV::AddressingModel::PhysicalStorageBuffer64EXT; + } else { + MAI.Addr = SPIRV::AddressingModel::Logical; + } } } // Get the OpenCL version number from metadata. @@ -999,7 +1026,7 @@ void RequirementHandler::initAvailableCapabilities(const SPIRVSubtarget &ST) { void RequirementHandler::initAvailableCapabilitiesForOpenCL( const SPIRVSubtarget &ST) { // Add the min requirements for different OpenCL and SPIR-V versions. - addAvailableCaps({Capability::Addresses, Capability::Float16Buffer, + addAvailableCaps({Capability::Addresses, Capability::Float16Buffer, Capability::PhysicalStorageBufferAddressesEXT, Capability::Int8, Capability::Kernel, Capability::Vector16, Capability::Groups, Capability::GenericPointer, Capability::StorageImageWriteWithoutFormat, @@ -1034,6 +1061,7 @@ void RequirementHandler::initAvailableCapabilitiesForVulkan( addAvailableCaps({Capability::Int64, Capability::Float16, Capability::Float64, + Capability::PhysicalStorageBufferAddressesEXT, Capability::GroupNonUniform, Capability::Image1D, Capability::SampledBuffer, @@ -1656,6 +1684,10 @@ void addInstrRequirements(const MachineInstr &MI, auto SC = MI.getOperand(1).getImm(); Reqs.getAndAddRequirements(SPIRV::OperandCategory::StorageClassOperand, SC, ST); + if (SC == static_cast(SPIRV::StorageClass::PhysicalStorageBufferEXT)) { + Reqs.addExtension(SPIRV::Extension::SPV_KHR_physical_storage_buffer); + Reqs.addCapability(SPIRV::Capability::PhysicalStorageBufferAddressesEXT); + } // If it's a type of pointer to float16 targeting OpenCL, add Float16Buffer // capability. if (ST.isShader()) @@ -1762,7 +1794,19 @@ void addInstrRequirements(const MachineInstr &MI, addOpDecorateReqs(MI, 2, Reqs, ST); break; case SPIRV::OpInBoundsPtrAccessChain: - Reqs.addCapability(SPIRV::Capability::Addresses); + if (MAI.Mem == SPIRV::MemoryModel::OpenCL) { + Reqs.addCapability(SPIRV::Capability::Addresses); + } else { + Reqs.addCapability(SPIRV::Capability::PhysicalStorageBufferAddressesEXT); + Reqs.addExtension(SPIRV::Extension::SPV_KHR_physical_storage_buffer); + Reqs.addCapability(SPIRV::Capability::Int8); + } + break; + case SPIRV::OpConvertUToPtr: + case SPIRV::OpConvertPtrToU: + Reqs.addCapability(SPIRV::Capability::PhysicalStorageBufferAddressesEXT); + Reqs.addExtension(SPIRV::Extension::SPV_KHR_physical_storage_buffer); + Reqs.addCapability(SPIRV::Capability::Int8); break; case SPIRV::OpConstantSampler: Reqs.addCapability(SPIRV::Capability::LiteralSampler); @@ -1781,7 +1825,13 @@ void addInstrRequirements(const MachineInstr &MI, break; case SPIRV::OpTypeForwardPointer: // TODO: check if it's OpenCL's kernel. - Reqs.addCapability(SPIRV::Capability::Addresses); + if (MAI.Mem == SPIRV::MemoryModel::OpenCL) { + Reqs.addCapability(SPIRV::Capability::Addresses); + } else { + Reqs.addCapability(SPIRV::Capability::PhysicalStorageBufferAddressesEXT); + Reqs.addExtension(SPIRV::Extension::SPV_KHR_physical_storage_buffer); + Reqs.addCapability(SPIRV::Capability::Int8); + } break; case SPIRV::OpAtomicFlagTestAndSet: case SPIRV::OpAtomicLoad: @@ -2538,7 +2588,13 @@ void addInstrRequirements(const MachineInstr &MI, break; } case SPIRV::OpCopyMemorySized: { - Reqs.addCapability(SPIRV::Capability::Addresses); + if (MAI.Mem == SPIRV::MemoryModel::OpenCL) { + Reqs.addCapability(SPIRV::Capability::Addresses); + } else { + Reqs.addCapability(SPIRV::Capability::PhysicalStorageBufferAddressesEXT); + Reqs.addExtension(SPIRV::Extension::SPV_KHR_physical_storage_buffer); + Reqs.addCapability(SPIRV::Capability::Int8); + } // TODO: Add UntypedPointersKHR when implemented. break; } diff --git a/llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp b/llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp index 5899f60360a113..50a4f75a933f1e 100644 --- a/llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp +++ b/llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp @@ -125,6 +125,9 @@ SPIRVSubtarget &SPIRVSubtarget::initSubtargetDependencies(StringRef CPU, } bool SPIRVSubtarget::canUseExtension(SPIRV::Extension::Extension E) const { + if (isShader() && (E == SPIRV::Extension::SPV_KHR_physical_storage_buffer || + E == SPIRV::Extension::SPV_EXT_physical_storage_buffer)) + return true; return AvailableExtensions.contains(E); } diff --git a/llvm/lib/Target/SPIRV/SPIRVUtils.cpp b/llvm/lib/Target/SPIRV/SPIRVUtils.cpp index dacb20f022818a..92311adbd9a7ee 100644 --- a/llvm/lib/Target/SPIRV/SPIRVUtils.cpp +++ b/llvm/lib/Target/SPIRV/SPIRVUtils.cpp @@ -378,7 +378,10 @@ addressSpaceToStorageClass(unsigned AddrSpace, const SPIRVSubtarget &STI) { case 0: return SPIRV::StorageClass::Function; case 1: - return SPIRV::StorageClass::CrossWorkgroup; + if (STI.isKernel()) + return SPIRV::StorageClass::CrossWorkgroup; + else + return SPIRV::StorageClass::PhysicalStorageBufferEXT; case 2: return SPIRV::StorageClass::UniformConstant; case 3: diff --git a/llvm/lib/Target/SPIRV/SPIRVUtils.h b/llvm/lib/Target/SPIRV/SPIRVUtils.h index 83c37eb8afb50a..cf0a022d14c69e 100644 --- a/llvm/lib/Target/SPIRV/SPIRVUtils.h +++ b/llvm/lib/Target/SPIRV/SPIRVUtils.h @@ -247,6 +247,7 @@ storageClassToAddressSpace(SPIRV::StorageClass::StorageClass SC) { case SPIRV::StorageClass::Function: return 0; case SPIRV::StorageClass::CrossWorkgroup: + case SPIRV::StorageClass::PhysicalStorageBufferEXT: return 1; case SPIRV::StorageClass::UniformConstant: return 2; @@ -379,9 +380,16 @@ inline bool isUntypedPointerTy(const Type *T) { return T && T->getTypeID() == Type::PointerTyID; } -// True if this is an instance of PointerType or TypedPointerType. +// True if this is an instance of TargetExtType representing spirv.$TypedPointerType. +inline bool isTargetExtPointerTy(const Type *T) { + if (!T || T->getTypeID() != Type::TargetExtTyID) + return false; + return cast(T)->getName() == "spirv.$TypedPointerType"; +} + +// True if this is an instance of PointerType or TypedPointerType or TargetExtType pointer. inline bool isPointerTy(const Type *T) { - return isUntypedPointerTy(T) || isTypedPointerTy(T); + return isUntypedPointerTy(T) || isTypedPointerTy(T) || isTargetExtPointerTy(T); } // True if this is a vector whose element type is an (untyped) PointerType. @@ -391,12 +399,15 @@ inline bool isUntypedPointerVectorTy(const Type *T) { } // Get the address space of this pointer or pointer vector type for instances of -// PointerType or TypedPointerType. +// PointerType, TypedPointerType, or TargetExtType pointer. inline unsigned getPointerAddressSpace(const Type *T) { Type *SubT = T->getScalarType(); - return SubT->getTypeID() == Type::PointerTyID - ? cast(SubT)->getAddressSpace() - : cast(SubT)->getAddressSpace(); + if (SubT->getTypeID() == Type::PointerTyID) + return cast(SubT)->getAddressSpace(); + if (SubT->getTypeID() == Type::TargetExtTyID && + cast(SubT)->getName() == "spirv.$TypedPointerType") + return cast(SubT)->getIntParameter(0); + return cast(SubT)->getAddressSpace(); } // Return true if the Argument is decorated with a pointee type diff --git a/llvm/test/CodeGen/SPIRV/pointers/physical-storage-buffer-load-store.ll b/llvm/test/CodeGen/SPIRV/pointers/physical-storage-buffer-load-store.ll new file mode 100644 index 00000000000000..994b3600225a23 --- /dev/null +++ b/llvm/test/CodeGen/SPIRV/pointers/physical-storage-buffer-load-store.ll @@ -0,0 +1,57 @@ +; Test: Vulkan compute shader buffer access uses OpAccessChain (logical addressing) +; instead of OpPtrAccessChain (physical addressing). +; +; Before the fix in SPIRVSubtarget.h, isLogicalSPIRV() returned false for +; Vulkan shaders, so the backend generated OpPtrAccessChain (OpenCL style). +; After the fix, isLogicalSPIRV() returns true when isShader() is true, +; so the backend correctly uses OpAccessChain (Vulkan/GLSL style). +; +; Also verifies that the VulkanBuffer element type is wrapped in a Block struct +; (fixed in SPIRVGlobalRegistry.cpp). The buffer descriptor must be: +; OpTypeStruct { OpTypeRuntimeArray { OpTypeStruct { float } } } +; not just a raw pointer to float. + +; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv-unknown-vulkan1.3-compute %s -o - | FileCheck %s +; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv-unknown-vulkan1.3-compute %s -o - -filetype=obj | spirv-val %} + +; CHECK-DAG: %[[#FLOAT:]] = OpTypeFloat 32 +; --- VulkanBuffer must wrap element in a struct (Block), not expose raw float --- +; CHECK-DAG: %[[#ELEM_WRAP:]] = OpTypeStruct %[[#FLOAT]] +; CHECK-DAG: %[[#RTARR:]] = OpTypeRuntimeArray %[[#ELEM_WRAP]] +; CHECK-DAG: %[[#BUF_TY:]] = OpTypeStruct %[[#RTARR]] + +; --- Access must use OpAccessChain, NOT OpPtrAccessChain --- +; CHECK: OpAccessChain +; CHECK-NOT: OpPtrAccessChain + +%struct.elem = type { float } + +define void @main() #0 { +entry: + ; Bind a VulkanBuffer of floats at set=0, binding=0 + %buf = call target("spirv.VulkanBuffer", [0 x %struct.elem], 12, 0) + @llvm.spv.resource.handlefrombinding.tspirv.VulkanBuffer_a0s_struct.elems_12_0t( + i32 0, i32 0, i32 1, i32 0, ptr null) + + ; Get pointer to element [0] of the buffer + %ptr = call noundef align 4 dereferenceable(4) ptr addrspace(11) + @llvm.spv.resource.getpointer.p11.tspirv.VulkanBuffer_a0s_struct.elems_12_0t( + target("spirv.VulkanBuffer", [0 x %struct.elem], 12, 0) %buf, i32 0) + + ; Load the float value + %val = load float, ptr addrspace(11) %ptr, align 4 + + ; Store it back (write back test) + store float %val, ptr addrspace(11) %ptr, align 4 + + ret void +} + +declare target("spirv.VulkanBuffer", [0 x %struct.elem], 12, 0) + @llvm.spv.resource.handlefrombinding.tspirv.VulkanBuffer_a0s_struct.elems_12_0t(i32, i32, i32, i32, ptr) + +declare ptr addrspace(11) + @llvm.spv.resource.getpointer.p11.tspirv.VulkanBuffer_a0s_struct.elems_12_0t( + target("spirv.VulkanBuffer", [0 x %struct.elem], 12, 0), i32) + +attributes #0 = { "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" }