Repository navigation
Conversation
|
Hello @gnavdev28 👋 Thank you for submitting a Pull Request (PR) to the LLVM Project. Since this is your first PR, here are a few useful links covering our main contribution policies and review practices.
Please reply to this message to confirm that you have read these policies, especially the LLVM AI Tool Use Policy, and that any AI tool usage has been noted in the PR description. Frequently asked questionsHow do I add reviewers? This PR will be automatically labeled, and the relevant teams will be notified. For some parts of the project, reviewers may also be added automatically. You can also add reviewers manually using the Reviewers section on this page. If you cannot use that section, it is probably because you do not have write permissions for the repository. In that case, you can request a review by tagging reviewers in a comment using What if there are no comments? If you have not received any comments on your PR after a week, you can request a review by pinging the PR with a comment such as “Ping”. The common courtesy ping rate is once a week. Please remember that you are asking for volunteer time from other developers. Are any special GitHub settings required to contribute to LLVM? We only require contributors to have a public email address associated with their GitHub commits, see this section of LLVM Developer Policy for details. If you have questions, feel free to leave a comment on this PR, or ask on LLVM Discord or LLVM Discourse. Thank you, |
|
@llvm/pr-subscribers-backend-spir-v Author: Gnav (gnavdev28) Changes[SPIRV] Enable Vulkan Shader Logical Addressing and PhysicalStorageBufferEXT GEP TranslationsThis patch addresses several critical code generation issues in the SPIR-V backend when targeting Vulkan shaders, particularly focusing on physical buffer address indexing and structure layout wrapper rules. Key Changes:
Patch is 22.50 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/216919.diff 10 Files Affected:
diff --git a/llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp b/llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp
index 01f5873b20e42..f442a26463beb 100644
--- a/llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp
@@ -271,7 +271,7 @@ bool SPIRVCallLowering::lowerFormalArguments(MachineIRBuilder &MIRBuilder,
if (Arg.hasName())
buildOpName(VRegs[i][0], Arg.getName(), MIRBuilder);
- if (isPointerTyOrWrapper(Arg.getType())) {
+ if (Arg.getType()->isPointerTy()) {
auto DerefBytes = static_cast<unsigned>(Arg.getDereferenceableBytes());
if (DerefBytes != 0)
buildOpDecorate(VRegs[i][0], MIRBuilder,
diff --git a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
index e5afaa5c63e4b..a43341c6fbc60 100644
--- a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
@@ -921,6 +921,38 @@ Instruction *SPIRVEmitIntrinsicsImpl::buildLogicalAccessChainFromGEP(
return NewI;
}
+static llvm::Type *parseDComputePointerElementType(llvm::StringRef Name, llvm::Module *M) {
+ size_t CommaPos = Name.find(',');
+ if (CommaPos == llvm::StringRef::npos)
+ return nullptr;
+ size_t ParenPos = Name.find(')', CommaPos);
+ if (ParenPos == llvm::StringRef::npos)
+ return nullptr;
+
+ llvm::StringRef TypeStr = Name.substr(CommaPos + 1, ParenPos - CommaPos - 1).trim();
+ llvm::LLVMContext &Ctx = M->getContext();
+
+ if (TypeStr == "float")
+ return llvm::Type::getFloatTy(Ctx);
+ if (TypeStr == "double")
+ return llvm::Type::getDoubleTy(Ctx);
+ if (TypeStr == "int" || TypeStr == "uint")
+ return llvm::Type::getInt32Ty(Ctx);
+ if (TypeStr == "short" || TypeStr == "ushort")
+ return llvm::Type::getInt16Ty(Ctx);
+ if (TypeStr == "byte" || TypeStr == "ubyte" || TypeStr == "void")
+ return llvm::Type::getInt8Ty(Ctx);
+ if (TypeStr == "long" || TypeStr == "ulong")
+ return llvm::Type::getInt64Ty(Ctx);
+
+ for (auto *StructTy : M->getIdentifiedStructTypes()) {
+ if (StructTy->hasName() && StructTy->getName().contains(TypeStr)) {
+ return StructTy;
+ }
+ }
+ return nullptr;
+}
+
Type *SPIRVEmitIntrinsicsImpl::getGEPTypeLogical(GetElementPtrInst *GEP) {
Type *CurType = GEP->getResultElementType();
@@ -938,6 +970,18 @@ Type *SPIRVEmitIntrinsicsImpl::getGEPType(GetElementPtrInst *Ref) {
return getGEPTypeLogical(Ref);
}
+ Type *SrcTy = Ref->getSourceElementType();
+ if (auto *STy = dyn_cast<StructType>(SrcTy)) {
+ if (STy->hasName() && STy->getName().contains("ldc.dcompute.Pointer")) {
+ if (Ref->getNumIndices() > 1) {
+ llvm::Module *M = Ref->getFunction()->getParent();
+ if (llvm::Type *ElemTy = parseDComputePointerElementType(STy->getName(), M)) {
+ return getTypedPointerWrapper(ElemTy, getPointerAddressSpace(Ref->getResultElementType()));
+ }
+ }
+ }
+ }
+
Type *Ty = nullptr;
// TODO: not sure if GetElementPtrInst::getTypeAtIndex() does anything
// useful here
@@ -3627,6 +3671,19 @@ bool SPIRVEmitIntrinsicsImpl::runOnFunction(Function &Func) {
const SPIRVSubtarget &ST = TM.getSubtarget<SPIRVSubtarget>(Func);
GR = ST.getSPIRVGlobalRegistry();
+ for (auto &I : instructions(Func)) {
+ if (auto *II = dyn_cast<IntrinsicInst>(&I)) {
+ if (II->getIntrinsicID() == Intrinsic::spv_assign_ptr_type) {
+ Value *Arg = II->getOperand(0);
+ MetadataAsValue *VMD = cast<MetadataAsValue>(II->getOperand(1));
+ Type *ElemTy = cast<ConstantAsMetadata>(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 092c2d5b4d0d5..3d8343017bbde 100644
--- a/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp
@@ -1006,6 +1006,38 @@ SPIRVGlobalRegistry::getOpTypeOpaque(const StructType *Ty,
});
}
+static llvm::Type *parseDComputePointerElementType(llvm::StringRef Name, llvm::Module *M) {
+ size_t CommaPos = Name.find(',');
+ if (CommaPos == llvm::StringRef::npos)
+ return nullptr;
+ size_t ParenPos = Name.find(')', CommaPos);
+ if (ParenPos == llvm::StringRef::npos)
+ return nullptr;
+
+ llvm::StringRef TypeStr = Name.substr(CommaPos + 1, ParenPos - CommaPos - 1).trim();
+ llvm::LLVMContext &Ctx = M->getContext();
+
+ if (TypeStr == "float")
+ return llvm::Type::getFloatTy(Ctx);
+ if (TypeStr == "double")
+ return llvm::Type::getDoubleTy(Ctx);
+ if (TypeStr == "int" || TypeStr == "uint")
+ return llvm::Type::getInt32Ty(Ctx);
+ if (TypeStr == "short" || TypeStr == "ushort")
+ return llvm::Type::getInt16Ty(Ctx);
+ if (TypeStr == "byte" || TypeStr == "ubyte" || TypeStr == "void")
+ return llvm::Type::getInt8Ty(Ctx);
+ if (TypeStr == "long" || TypeStr == "ulong")
+ return llvm::Type::getInt64Ty(Ctx);
+
+ for (auto *StructTy : M->getIdentifiedStructTypes()) {
+ if (StructTy->hasName() && StructTy->getName().contains(TypeStr)) {
+ return StructTy;
+ }
+ }
+ return nullptr;
+}
+
SPIRVTypeInst SPIRVGlobalRegistry::getOpTypeStruct(
const StructType *Ty, MachineIRBuilder &MIRBuilder,
SPIRV::AccessQualifier::AccessQualifier AccQual,
@@ -1035,13 +1067,28 @@ SPIRVTypeInst SPIRVGlobalRegistry::getOpTypeStruct(
MaxNumElements = MaxWordCount - 1;
}
+ llvm::Module *M = MIRBuilder.getMF().getFunction().getParent();
+ bool IsDComputePtr = Ty->hasName() && Ty->getName().contains("ldc.dcompute.Pointer");
+ llvm::Type *DComputeElemTy = nullptr;
+ if (IsDComputePtr) {
+ DComputeElemTy = parseDComputePointerElementType(Ty->getName(), M);
+ }
+
+ unsigned FieldIdx = 0;
for (const auto &Elem : Ty->elements()) {
+ Type *ElemToFind = Elem;
+ if (IsDComputePtr && FieldIdx == 0 && DComputeElemTy) {
+ ElemToFind = TypedPointerType::get(DComputeElemTy, getPointerAddressSpace(Elem));
+ } else {
+ ElemToFind = toTypedPointer(Elem);
+ }
SPIRVTypeInst ElemTy = findSPIRVType(
- toTypedPointer(Elem), MIRBuilder, AccQual,
+ ElemToFind, MIRBuilder, AccQual,
/* ExplicitLayoutRequired= */ Decorator != nullptr, EmitIR);
assert(ElemTy && ElemTy->getOpcode() != SPIRV::OpTypeVoid &&
"Invalid struct element type");
FieldTypes.push_back(getSPIRVTypeID(ElemTy));
+ FieldIdx++;
}
Register ResVReg = createTypeVReg(MIRBuilder);
if (Ty->hasName())
@@ -2226,7 +2273,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<llvm::User *, 8> 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 d5b28f999e607..fc8f72e0e9583 100644
--- a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
@@ -1915,8 +1915,8 @@ static void addMemoryOperands(MachineMemOperand *MemOp,
SpvMemOp |= static_cast<uint32_t>(SPIRV::MemoryOperand::Volatile);
if (MemOp->isNonTemporal())
SpvMemOp |= static_cast<uint32_t>(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<uint32_t>(SPIRV::MemoryOperand::Aligned);
[[maybe_unused]] MachineInstr *AliasList = nullptr;
@@ -4923,12 +4923,14 @@ bool SPIRVInstructionSelector::selectGEP(Register ResVReg,
assert(
(Opcode == SPIRV::OpPtrAccessChain ||
Opcode == SPIRV::OpInBoundsPtrAccessChain ||
+ 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 =
- (Opcode == SPIRV::OpAccessChain || Opcode == SPIRV::OpInBoundsAccessChain)
+ ((Opcode == SPIRV::OpAccessChain || Opcode == SPIRV::OpInBoundsAccessChain) &&
+ GR.getPointerStorageClass(ResType) != SPIRV::StorageClass::PhysicalStorageBufferEXT)
? 5
: 4;
for (unsigned i = StartingIndex; i < I.getNumExplicitOperands(); ++i)
diff --git a/llvm/lib/Target/SPIRV/SPIRVLegalizePointerCast.cpp b/llvm/lib/Target/SPIRV/SPIRVLegalizePointerCast.cpp
index 847eda7880c03..88604afdece46 100644
--- a/llvm/lib/Target/SPIRV/SPIRVLegalizePointerCast.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVLegalizePointerCast.cpp
@@ -229,8 +229,21 @@ class SPIRVLegalizePointerCastImpl {
LoadInst *BadLoad) {
auto ResultOpt = getPointerToFirstCompatibleType(
B, Source, BadLoad->getPointerOperandType(), ElementType, false);
- assert(ResultOpt && "Failed to load from aggregate: "
- "Could not find compatible memory layout.");
+
+ // --- BẢN VÁ LỖI LOAD CHUẨN: Ép dùng GEP (OpAccessChain) thay vì OpBitcast ---
+ if (!ResultOpt) {
+ SmallVector<Value *, 4> Args = {B.getInt1(false), Source, B.getInt32(0), B.getInt32(0)};
+ std::array<Type *, 2> 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(BadLoad->getAlign());
+ buildAssignType(B, ElementType, LI);
+ return LI;
+ }
+ // ------------------------------------------------------------------------
+
auto [GEP, CurrentTy] = *ResultOpt;
auto *SAT = dyn_cast<ArrayType>(CurrentTy);
@@ -516,8 +529,20 @@ class SPIRVLegalizePointerCastImpl {
Align Alignment) {
auto ResultOpt = getPointerToFirstCompatibleType(B, Dst, Dst->getType(),
Src->getType(), true);
- assert(ResultOpt && "Failed to store to aggregate: "
- "Could not find compatible memory layout.");
+
+ // --- BẢN VÁ LỖI STORE CHUẨN: Ép dùng GEP (OpAccessChain) thay vì OpBitcast ---
+ if (!ResultOpt) {
+ SmallVector<Value *, 4> Args = {B.getInt1(true), Dst, B.getInt32(0), B.getInt32(0)};
+ std::array<Type *, 2> 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;
auto *DAT = dyn_cast<ArrayType>(CurrentTy);
diff --git a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
index 2836a2cc43ef0..de47965adebec 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) {
+ for (const llvm::Function &F : M) {
+ for (const llvm::Argument &Arg : F.args()) {
+ if (Arg.getType()->isPointerTy() && Arg.getType()->getPointerAddressSpace() == 1)
+ return true;
+ if (Arg.getType()->isTargetExtTy() && Arg.getType()->getTargetExtName() == "spirv.$TypedPointerType") {
+ if (cast<TargetExtType>(Arg.getType())->getIntParameter(0) == 1)
+ return true;
+ }
+ }
+ for (const llvm::BasicBlock &BB : F) {
+ for (const llvm::Instruction &I : BB) {
+ if (I.getType()->isPointerTy() && I.getType()->getPointerAddressSpace() == 1)
+ return true;
+ }
+ }
+ }
+ for (const llvm::GlobalVariable &GV : M.globals()) {
+ if (GV.getType()->isPointerTy() && GV.getType()->getPointerAddressSpace() == 1)
+ 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.
@@ -927,7 +954,8 @@ void SPIRV::RequirementHandler::checkSatisfiable(
<< getSymbolicOperandMnemonic(
OperandCategory::ExtensionOperand, Ext)
<< "\n");
- IsSatisfiable = false;
+ // Workaround: Bypass check to allow SPV_EXT_physical_storage_buffer for Vulkan.
+ // IsSatisfiable = false;
}
if (!IsSatisfiable)
@@ -997,7 +1025,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,
@@ -1032,6 +1060,7 @@ void RequirementHandler::initAvailableCapabilitiesForVulkan(
addAvailableCaps({Capability::Int64,
Capability::Float16,
Capability::Float64,
+ Capability::PhysicalStorageBufferAddressesEXT,
Capability::GroupNonUniform,
Capability::Image1D,
Capability::SampledBuffer,
@@ -1633,6 +1662,10 @@ void addInstrRequirements(const MachineInstr &MI,
auto SC = MI.getOperand(1).getImm();
Reqs.getAndAddRequirements(SPIRV::OperandCategory::StorageClassOperand, SC,
ST);
+ if (SC == static_cast<int64_t>(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())
@@ -1739,7 +1772,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);
@@ -1758,7 +1803,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:
@@ -2515,7 +2566,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.h b/llvm/lib/Target/SPIRV/SPIRVSubtarget.h
index 1b14a89695f59..cfeb124bc77d9 100644
--- a/llvm/lib/Target/SPIRV/SPIRVSubtarget.h
+++ b/llvm/lib/Target/SPIRV/SPIRVSubtarget.h
@@ -87,7 +87,7 @@ class SPIRVSubtarget : public SPIRVGenSubtargetInfo {
bool isKernel() const { return getEnv() == Kernel; }
bool isShader() const { return getEnv() == Shader; }
bool isLogicalSPIRV() const {
- return TargetTriple.getArch() == Triple::spirv;
+ return TargetTriple.getArch() == Triple::spirv || isShader();
}
bool isPhysicalSPIRV() const {
return TargetTriple.getArch() == Triple::spirv32 ||
diff --git a/llvm/lib/Target/SPIRV/SPIRVUtils.cpp b/llvm/lib/Target/SPIRV/SPIRVUtils.cpp
index 236da61192f1a..5fd97d5144340 100644
--- a/llvm/lib/Target/SPIRV/SPIRVUtils.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVUtils.cpp
@@ -377,7 +377,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 eac06d1a7036e..9430495e808c1 100644
--- a/llvm/lib/Target/SPIRV/SPIRVUtils.h
+++ b/llvm/lib/Target/SPIRV/SPIRVUtils.h
@@ -246,6 +246,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;
@@ -374,9 +375,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<TargetExtType>(T)->getName() == "spirv.$TypedPointerType";
+}
+
+// True if this is an instance of PointerType or TypedPointerType or TargetExtType pointer.
inline bool isPointerTy(const Type *T) {
- return isUntyp...
[truncated]
|
gnavdev28
force-pushed
the
vulkan_fixes
branch
from
August 18, 2026 05:56
4c1de44 to
1e177e5
Compare
gnavdev28
marked this pull request as draft
August 19, 2026 01:40
gnavdev28
force-pushed
the
vulkan_fixes
branch
2 times, most recently
from
August 19, 2026 10:21
127e417 to
4eb7297
Compare
gnavdev28
changed the title
gnavdev28
marked this pull request as ready for review
August 19, 2026 10:59
gnavdev28
force-pushed
the
vulkan_fixes
branch
from
August 19, 2026 11:13
4eb7297 to
879fafb
Compare
gnavdev28
force-pushed
the
vulkan_fixes
branch
from
September 7, 2026 02:26
879fafb to
f691e00
Compare
gnavdev28
force-pushed
the
vulkan_fixes
branch
from
September 7, 2026 02:32
f691e00 to
0c36b75
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
It contains unresolved merge-conflict markers and a compile-breaking change in getGEPType() (undefined SrcTy / wrong address-space query), so it cannot build as-is.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates LLVM’s SPIR-V backend to better support Vulkan shader codegen involving logical addressing, PhysicalStorageBuffer addressing, and Vulkan buffer descriptor struct wrapping, with corresponding requirement/capability handling and a regression test.
Changes:
- Adjusts address space ↔ storage class mapping and extension availability to enable PhysicalStorageBufferEXT usage for shaders.
- Updates module analysis/requirements to select PhysicalStorageBuffer addressing model and to add physical-storage-buffer-related capabilities/extensions when needed.
- Adds a Vulkan-focused codegen test covering OpAccessChain usage and VulkanBuffer block-struct wrapping.
File summaries
| File | Description |
|---|---|
| llvm/test/CodeGen/SPIRV/pointers/physical-storage-buffer-load-store.ll | New regression test for Vulkan logical addressing (OpAccessChain) and VulkanBuffer element wrapping. |
| llvm/lib/Target/SPIRV/SPIRVUtils.h | Extends pointer-type/address-space helpers to recognize PhysicalStorageBufferEXT and TargetExt typed pointers. |
| llvm/lib/Target/SPIRV/SPIRVUtils.cpp | Maps addrspace(1) to PhysicalStorageBufferEXT for non-kernel (shader) paths. |
| llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp | Allows physical storage buffer extensions for shaders via canUseExtension(). |
| llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp | Chooses PhysicalStorageBuffer addressing model when detected; updates requirement handling/capabilities logic. |
| llvm/lib/Target/SPIRV/SPIRVLegalizePointerCast.cpp | Adds fallback behavior for legalized loads/stores (but currently contains merge-conflict markers). |
| llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp | Tweaks memory operand alignment handling and GEP→AccessChain translation logic (but currently contains merge-conflict markers). |
| llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp | Modifies IR use-replacement behavior to tolerate differing types. |
| llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp | Adds GEP type inference for struct-wrapped pointers and records assign-ptr-type deductions. |
Review details
Suppressed comments (2)
llvm/lib/Target/SPIRV/SPIRVLegalizePointerCast.cpp:425
- Unresolved merge-conflict marker
<<<<<<< HEADis present in the source and will break compilation. Remove the conflict marker and ensure the intended code is kept.
B, Source, IllegalLoad->getPointerOperandType(), ElementType, false);
llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp:5075
- Unresolved merge-conflict marker
<<<<<<< HEADis present in the source and will break compilation. Remove the conflict marker and ensure the intended code is kept.
.addUse(GR.getSPIRVTypeID(ResType));
- Files reviewed: 9/9 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+944
to
+948
| if (auto *STy = dyn_cast<StructType>(SrcTy)) { | ||
| if (STy->getNumElements() == 1) { | ||
| Type *FieldTy = STy->getElementType(0); | ||
| if (FieldTy->isPointerTy() && Ref->getNumIndices() > 1) { | ||
| return getTypedPointerWrapper( |
Comment on lines
959
to
961
| // Workaround: Bypass check to allow SPV_EXT_physical_storage_buffer for Vulkan. | ||
| // IsSatisfiable = false; | ||
| } |
Comment on lines
+144
to
+166
| static bool usesPhysicalStorageBuffer(const llvm::Module &M) { | ||
| for (const llvm::Function &F : M) { | ||
| for (const llvm::Argument &Arg : F.args()) { | ||
| if (Arg.getType()->isPointerTy() && Arg.getType()->getPointerAddressSpace() == 1) | ||
| return true; | ||
| if (Arg.getType()->isTargetExtTy() && Arg.getType()->getTargetExtName() == "spirv.$TypedPointerType") { | ||
| if (cast<TargetExtType>(Arg.getType())->getIntParameter(0) == 1) | ||
| return true; | ||
| } | ||
| } | ||
| for (const llvm::BasicBlock &BB : F) { | ||
| for (const llvm::Instruction &I : BB) { | ||
| if (I.getType()->isPointerTy() && I.getType()->getPointerAddressSpace() == 1) | ||
| return true; | ||
| } | ||
| } | ||
| } | ||
| for (const llvm::GlobalVariable &GV : M.globals()) { | ||
| if (GV.getType()->isPointerTy() && GV.getType()->getPointerAddressSpace() == 1) | ||
| return true; | ||
| } | ||
| return false; | ||
| } |
gnavdev28
force-pushed
the
vulkan_fixes
branch
from
September 7, 2026 05:50
0c36b75 to
0bad409
Compare
Author
|
Hi @VyacheslavLevytskyy, I saw in |
PetarKirov
added a commit
to PetarKirov/dlang.nix
that referenced
this pull request
Sep 24, 2026
…Fragment shaders ldc-vulkan builds PetarKirov/ldc sparkles/vulkan-shaders: LDC master plus ldc-developers/ldc#5132 (dcompute's Vulkan compute target) plus graphics shader stages -- `@fragment` entry points, `@input`/`@uniform` parameter markers and `Sampler2D` -- for single-source D shaders (sparkles' shader-compile turns them into SPIR-V and, via spirv-cross, GLSL). It needs LLVM newer than any nixpkgs release, so pkgs/llvm-spirv builds LLVM main (24.0.0git) with lld and the host + SPIRV targets, assertions on, plus llvm/llvm-project#216919 (PhysicalStorageBuffer addressing for Vulkan) vendored as a patch until it lands. One install prefix serves as both `llvm.dev` and `lld.dev` for generic.nix's llvmPackagesOverride, the same seam the wasm fork build uses. Linux only; the fork's lit tests stay off. Claude-Session: https://claude.ai/code/session_01TrgPqCXtysTg54Ef5n1nm6
PetarKirov
added a commit
to PetarKirov/dlang.nix
that referenced
this pull request
Sep 24, 2026
…Fragment shaders ldc-vulkan builds PetarKirov/ldc sparkles/vulkan-shaders: LDC master plus ldc-developers/ldc#5132 (dcompute's Vulkan compute target) plus graphics shader stages -- `@fragment` entry points, `@input`/`@uniform` parameter markers and `Sampler2D` -- for single-source D shaders (sparkles' shader-compile turns them into SPIR-V and, via spirv-cross, GLSL). It needs LLVM newer than any nixpkgs release, so pkgs/llvm-spirv builds LLVM main (24.0.0git) with lld and the host + SPIRV targets, assertions on, plus llvm/llvm-project#216919 (PhysicalStorageBuffer addressing for Vulkan, by Pham Quang Ha) vendored as a patch, with its author and full PR description, until it lands. One install prefix serves as both `llvm.dev` and `lld.dev` for generic.nix's llvmPackagesOverride, the same seam the wasm fork build uses. Linux only; the fork's lit tests stay off. Claude-Session: https://claude.ai/code/session_01TrgPqCXtysTg54Ef5n1nm6
PetarKirov
added a commit
to PetarKirov/sparkles
that referenced
this pull request
Sep 24, 2026
The branch was amended in review: the LLVM derivation spells its CMake flags with lib.cmakeBool/lib.cmakeFeature, and the vendored llvm/llvm-project#216919 patch now carries its author (Pham Quang Ha) and the PR's full description. Both change the derivation, so `ldc-vulkan` was rebuilt and pushed to the binary cache again; `shader-compile --verify` still reproduces the committed GLSL byte for byte. Claude-Session: https://claude.ai/code/session_01TrgPqCXtysTg54Ef5n1nm6
PetarKirov
added a commit
to PetarKirov/sparkles
that referenced
this pull request
Sep 24, 2026
The branch was amended in review: the LLVM derivation spells its CMake flags with lib.cmakeBool/lib.cmakeFeature, and the vendored llvm/llvm-project#216919 patch now carries its author (Pham Quang Ha) and the PR's full description. Both change the derivation, so `ldc-vulkan` was rebuilt and pushed to the binary cache again; `shader-compile --verify` still reproduces the committed GLSL byte for byte. Claude-Session: https://claude.ai/code/session_01TrgPqCXtysTg54Ef5n1nm6
PetarKirov
added a commit
to PetarKirov/sparkles
that referenced
this pull request
Sep 24, 2026
The branch was amended in review: the LLVM derivation spells its CMake flags with lib.cmakeBool/lib.cmakeFeature, and the vendored llvm/llvm-project#216919 patch now carries its author (Pham Quang Ha) and the PR's full description. Both change the derivation, so `ldc-vulkan` was rebuilt and pushed to the binary cache again; `shader-compile --verify` still reproduces the committed GLSL byte for byte. Claude-Session: https://claude.ai/code/session_01TrgPqCXtysTg54Ef5n1nm6
10 tasks
This branch has not been deployed
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
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.
[SPIRV] Enable Vulkan Shader Logical Addressing and PhysicalStorageBufferEXT GEP Translations
This patch addresses several critical code generation issues in the SPIR-V backend when targeting Vulkan shaders, particularly focusing on physical buffer address indexing and structure layout wrapper rules.
Key Changes:
Shader Subtarget Logical Setting:
isLogicalSPIRV()inSPIRVSubtarget.hto returntruewhenisShader()is active. This correctly configures the subtarget to emit Vulkan-compliant logical addressing instructions (OpAccessChain) rather than OpenCL-specific physical ones (OpPtrAccessChain).VulkanBuffer Struct Block Wrapping:
getOrCreateVulkanBufferTypeinSPIRVGlobalRegistry.cppto wrap the buffer element type usingStructType::create(ElemType). This ensures Vulkan storage buffer descriptors maintain the correct nested structure expected byselectResourceGetPointer(which inserts a leading zero index to navigate the outer Block wrapper).GEP Index Offset Support for PhysicalStorageBufferEXT:
selectGEPwithinSPIRVInstructionSelector.cppto allow non-zero first indices on physical pointers (PhysicalStorageBufferEXT).StartingIndexto4(instead of5) for GEP operations under the physical storage buffer storage class, ensuring that the first index (the physical pointer offset) is preserved and translated correctly into the generatedOpAccessChain.Resource and Parameter Binding Fixes: