Skip to content

Commit bd33eef

Browse files
s-perrons-perron
authored
[HLSL][SPIRV] Use resource names (#143412)
The SPIR-V backend does not have access to the original name of a resource in the source, so it tries to create a name. This leads to some problems with reflection. That is why start to pass the name of the resource from Clang to the SPIR-V backend. Fixes #138533
1 parent dec5765 commit bd33eef

23 files changed

+135
-223
lines changed

‎clang/lib/CodeGen/CGHLSLBuiltins.cpp

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -295,17 +295,16 @@ Value *CodeGenFunction::EmitHLSLBuiltinExpr(unsigned BuiltinID,
295295
Value *SpaceOp = EmitScalarExpr(E->getArg(2));
296296
Value *RangeOp = EmitScalarExpr(E->getArg(3));
297297
Value *IndexOp = EmitScalarExpr(E->getArg(4));
298+
Value *Name = EmitScalarExpr(E->getArg(5));
298299
// FIXME: NonUniformResourceIndex bit is not yet implemented
299300
// (llvm/llvm-project#135452)
300301
Value *NonUniform =
301302
llvm::ConstantInt::get(llvm::Type::getInt1Ty(getLLVMContext()), false);
302303

303-
auto [IntrinsicID, HasNameArg] =
304+
llvm::Intrinsic::ID IntrinsicID =
304305
CGM.getHLSLRuntime().getCreateHandleFromBindingIntrinsic();
305-
SmallVector<Value *> Args{SpaceOp, RegisterOp, RangeOp, IndexOp,
306-
NonUniform};
307-
if (HasNameArg)
308-
Args.push_back(EmitScalarExpr(E->getArg(5)));
306+
SmallVector<Value *> Args{SpaceOp, RegisterOp, RangeOp,
307+
IndexOp, NonUniform, Name};
309308
return Builder.CreateIntrinsic(HandleTy, IntrinsicID, Args);
310309
}
311310
case Builtin::BI__builtin_hlsl_resource_handlefromimplicitbinding: {
@@ -314,16 +313,16 @@ Value *CodeGenFunction::EmitHLSLBuiltinExpr(unsigned BuiltinID,
314313
Value *RangeOp = EmitScalarExpr(E->getArg(2));
315314
Value *IndexOp = EmitScalarExpr(E->getArg(3));
316315
Value *OrderID = EmitScalarExpr(E->getArg(4));
316+
Value *Name = EmitScalarExpr(E->getArg(5));
317317
// FIXME: NonUniformResourceIndex bit is not yet implemented
318318
// (llvm/llvm-project#135452)
319319
Value *NonUniform =
320320
llvm::ConstantInt::get(llvm::Type::getInt1Ty(getLLVMContext()), false);
321321

322-
auto [IntrinsicID, HasNameArg] =
322+
llvm::Intrinsic::ID IntrinsicID =
323323
CGM.getHLSLRuntime().getCreateHandleFromImplicitBindingIntrinsic();
324-
SmallVector<Value *> Args{OrderID, SpaceOp, RangeOp, IndexOp, NonUniform};
325-
if (HasNameArg)
326-
Args.push_back(EmitScalarExpr(E->getArg(5)));
324+
SmallVector<Value *> Args{OrderID, SpaceOp, RangeOp,
325+
IndexOp, NonUniform, Name};
327326
return Builder.CreateIntrinsic(HandleTy, IntrinsicID, Args);
328327
}
329328
case Builtin::BI__builtin_hlsl_all: {

‎clang/lib/CodeGen/CGHLSLRuntime.cpp

Lines changed: 8 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -237,35 +237,6 @@ static void fillPackoffsetLayout(const HLSLBufferDecl *BufDecl,
237237
}
238238
}
239239

240-
std::pair<llvm::Intrinsic::ID, bool>
241-
CGHLSLRuntime::getCreateHandleFromBindingIntrinsic() {
242-
switch (getArch()) {
243-
case llvm::Triple::dxil:
244-
return std::pair(llvm::Intrinsic::dx_resource_handlefrombinding, true);
245-
case llvm::Triple::spirv:
246-
return std::pair(llvm::Intrinsic::spv_resource_handlefrombinding, false);
247-
default:
248-
llvm_unreachable("Intrinsic resource_handlefrombinding not supported by "
249-
"target architecture");
250-
}
251-
}
252-
253-
std::pair<llvm::Intrinsic::ID, bool>
254-
CGHLSLRuntime::getCreateHandleFromImplicitBindingIntrinsic() {
255-
switch (getArch()) {
256-
case llvm::Triple::dxil:
257-
return std::pair(llvm::Intrinsic::dx_resource_handlefromimplicitbinding,
258-
true);
259-
case llvm::Triple::spirv:
260-
return std::pair(llvm::Intrinsic::spv_resource_handlefromimplicitbinding,
261-
false);
262-
default:
263-
llvm_unreachable(
264-
"Intrinsic resource_handlefromimplicitbinding not supported by "
265-
"target architecture");
266-
}
267-
}
268-
269240
// Codegen for HLSLBufferDecl
270241
void CGHLSLRuntime::addBuffer(const HLSLBufferDecl *BufDecl) {
271242

@@ -625,31 +596,27 @@ void CGHLSLRuntime::initializeBufferFromBinding(const HLSLBufferDecl *BufDecl,
625596
llvm::ConstantInt::get(CGM.IntTy, RBA ? RBA->getSpaceNumber() : 0);
626597
Value *Name = nullptr;
627598

628-
auto [IntrinsicID, HasNameArg] =
599+
llvm::Intrinsic::ID IntrinsicID =
629600
RBA->hasRegisterSlot()
630601
? CGM.getHLSLRuntime().getCreateHandleFromBindingIntrinsic()
631602
: CGM.getHLSLRuntime().getCreateHandleFromImplicitBindingIntrinsic();
632603

633-
if (HasNameArg) {
634-
std::string Str(BufDecl->getName());
635-
std::string GlobalName(Str + ".str");
636-
Name = CGM.GetAddrOfConstantCString(Str, GlobalName.c_str()).getPointer();
637-
}
604+
std::string Str(BufDecl->getName());
605+
std::string GlobalName(Str + ".str");
606+
Name = CGM.GetAddrOfConstantCString(Str, GlobalName.c_str()).getPointer();
638607

639608
// buffer with explicit binding
640609
if (RBA->hasRegisterSlot()) {
641610
auto *RegSlot = llvm::ConstantInt::get(CGM.IntTy, RBA->getSlotNumber());
642-
SmallVector<Value *> Args{Space, RegSlot, RangeSize, Index, NonUniform};
643-
if (Name)
644-
Args.push_back(Name);
611+
SmallVector<Value *> Args{Space, RegSlot, RangeSize,
612+
Index, NonUniform, Name};
645613
initializeBuffer(CGM, GV, IntrinsicID, Args);
646614
} else {
647615
// buffer with implicit binding
648616
auto *OrderID =
649617
llvm::ConstantInt::get(CGM.IntTy, RBA->getImplicitBindingOrderID());
650-
SmallVector<Value *> Args{OrderID, Space, RangeSize, Index, NonUniform};
651-
if (Name)
652-
Args.push_back(Name);
618+
SmallVector<Value *> Args{OrderID, Space, RangeSize,
619+
Index, NonUniform, Name};
653620
initializeBuffer(CGM, GV, IntrinsicID, Args);
654621
}
655622
}

‎clang/lib/CodeGen/CGHLSLRuntime.h

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,10 @@ class CGHLSLRuntime {
118118

119119
GENERATE_HLSL_INTRINSIC_FUNCTION(CreateResourceGetPointer,
120120
resource_getpointer)
121+
GENERATE_HLSL_INTRINSIC_FUNCTION(CreateHandleFromBinding,
122+
resource_handlefrombinding)
123+
GENERATE_HLSL_INTRINSIC_FUNCTION(CreateHandleFromImplicitBinding,
124+
resource_handlefromimplicitbinding)
121125
GENERATE_HLSL_INTRINSIC_FUNCTION(BufferUpdateCounter, resource_updatecounter)
122126
GENERATE_HLSL_INTRINSIC_FUNCTION(GroupMemoryBarrierWithGroupSync,
123127
group_memory_barrier_with_group_sync)
@@ -126,15 +130,6 @@ class CGHLSLRuntime {
126130
// End of reserved area for HLSL intrinsic getters.
127131
//===----------------------------------------------------------------------===//
128132

129-
// Returns ID of the intrinsic that initializes resource handle from binding
130-
// and a bool value indicating whether the last argument of the intrinsic is
131-
// the resource name (not all targets need that).
132-
std::pair<llvm::Intrinsic::ID, bool> getCreateHandleFromBindingIntrinsic();
133-
134-
// Same as above but for implicit binding.
135-
std::pair<llvm::Intrinsic::ID, bool>
136-
getCreateHandleFromImplicitBindingIntrinsic();
137-
138133
protected:
139134
CodeGenModule &CGM;
140135

‎llvm/include/llvm/IR/IntrinsicsSPIRV.td

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -117,15 +117,15 @@ let TargetPrefix = "spv" in {
117117
// array size of the binding, as well as an index and an indicator
118118
// whether that index may be non-uniform.
119119
def int_spv_resource_handlefrombinding
120-
: DefaultAttrsIntrinsic<
121-
[llvm_any_ty],
122-
[llvm_i32_ty, llvm_i32_ty, llvm_i32_ty, llvm_i32_ty, llvm_i1_ty],
123-
[IntrNoMem]>;
120+
: DefaultAttrsIntrinsic<[llvm_any_ty],
121+
[llvm_i32_ty, llvm_i32_ty, llvm_i32_ty,
122+
llvm_i32_ty, llvm_i1_ty, llvm_ptr_ty],
123+
[IntrNoMem]>;
124124
def int_spv_resource_handlefromimplicitbinding
125-
: DefaultAttrsIntrinsic<
126-
[llvm_any_ty],
127-
[llvm_i32_ty, llvm_i32_ty, llvm_i32_ty, llvm_i32_ty, llvm_i1_ty],
128-
[IntrNoMem]>;
125+
: DefaultAttrsIntrinsic<[llvm_any_ty],
126+
[llvm_i32_ty, llvm_i32_ty, llvm_i32_ty,
127+
llvm_i32_ty, llvm_i1_ty, llvm_ptr_ty],
128+
[IntrNoMem]>;
129129

130130
def int_spv_firstbituhigh : DefaultAttrsIntrinsic<[LLVMScalarOrSameVectorWidth<0, llvm_i32_ty>], [llvm_anyint_ty], [IntrNoMem]>;
131131
def int_spv_firstbitshigh : DefaultAttrsIntrinsic<[LLVMScalarOrSameVectorWidth<0, llvm_i32_ty>], [llvm_anyint_ty], [IntrNoMem]>;

‎llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp

Lines changed: 1 addition & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -799,107 +799,15 @@ Register SPIRVGlobalRegistry::buildGlobalVariable(
799799
return Reg;
800800
}
801801

802-
static std::string GetSpirvImageTypeName(const SPIRVType *Type,
803-
MachineIRBuilder &MIRBuilder,
804-
const std::string &Prefix,
805-
SPIRVGlobalRegistry &GR);
806-
807802
// Returns a name based on the Type. Notes that this does not look at
808803
// decorations, and will return the same string for two types that are the same
809804
// except for decorations.
810-
static std::string buildSpirvTypeName(const SPIRVType *Type,
811-
MachineIRBuilder &MIRBuilder,
812-
SPIRVGlobalRegistry &GR) {
813-
switch (Type->getOpcode()) {
814-
case SPIRV::OpTypeSampledImage: {
815-
return GetSpirvImageTypeName(Type, MIRBuilder, "sampled_image_", GR);
816-
}
817-
case SPIRV::OpTypeImage: {
818-
return GetSpirvImageTypeName(Type, MIRBuilder, "image_", GR);
819-
}
820-
case SPIRV::OpTypeArray: {
821-
MachineRegisterInfo *MRI = MIRBuilder.getMRI();
822-
Register ElementTypeReg = Type->getOperand(1).getReg();
823-
auto *ElementType = MRI->getUniqueVRegDef(ElementTypeReg);
824-
uint32_t ArraySize = getArrayComponentCount(MRI, Type);
825-
return (buildSpirvTypeName(ElementType, MIRBuilder, GR) + Twine("[") +
826-
Twine(ArraySize) + Twine("]"))
827-
.str();
828-
}
829-
case SPIRV::OpTypeFloat:
830-
return ("f" + Twine(Type->getOperand(1).getImm())).str();
831-
case SPIRV::OpTypeSampler:
832-
return ("sampler");
833-
case SPIRV::OpTypeInt:
834-
if (Type->getOperand(2).getImm())
835-
return ("i" + Twine(Type->getOperand(1).getImm())).str();
836-
return ("u" + Twine(Type->getOperand(1).getImm())).str();
837-
case SPIRV::OpTypePointer: {
838-
uint32_t StorageClass = GR.getPointerStorageClass(Type);
839-
SPIRVType *PointeeType = GR.getPointeeType(Type);
840-
return ("p_" + Twine(StorageClass) + Twine("_") +
841-
buildSpirvTypeName(PointeeType, MIRBuilder, GR))
842-
.str();
843-
}
844-
case SPIRV::OpTypeStruct: {
845-
std::string TypeName = "{";
846-
for (uint32_t I = 1; I < Type->getNumOperands(); ++I) {
847-
SPIRVType *MemberType =
848-
GR.getSPIRVTypeForVReg(Type->getOperand(I).getReg());
849-
TypeName += '_' + buildSpirvTypeName(MemberType, MIRBuilder, GR);
850-
}
851-
return TypeName + "}";
852-
}
853-
case SPIRV::OpTypeVector: {
854-
MachineRegisterInfo *MRI = MIRBuilder.getMRI();
855-
Register ElementTypeReg = Type->getOperand(1).getReg();
856-
auto *ElementType = MRI->getUniqueVRegDef(ElementTypeReg);
857-
uint32_t VectorSize = GR.getScalarOrVectorComponentCount(Type);
858-
return (buildSpirvTypeName(ElementType, MIRBuilder, GR) + Twine("[") +
859-
Twine(VectorSize) + Twine("]"))
860-
.str();
861-
}
862-
case SPIRV::OpTypeRuntimeArray: {
863-
MachineRegisterInfo *MRI = MIRBuilder.getMRI();
864-
Register ElementTypeReg = Type->getOperand(1).getReg();
865-
auto *ElementType = MRI->getUniqueVRegDef(ElementTypeReg);
866-
uint32_t ArraySize = 0;
867-
return (buildSpirvTypeName(ElementType, MIRBuilder, GR) + Twine("[") +
868-
Twine(ArraySize) + Twine("]"))
869-
.str();
870-
}
871-
default:
872-
llvm_unreachable("Trying to the the name of an unknown type.");
873-
}
874-
}
875-
876-
static std::string GetSpirvImageTypeName(const SPIRVType *Type,
877-
MachineIRBuilder &MIRBuilder,
878-
const std::string &Prefix,
879-
SPIRVGlobalRegistry &GR) {
880-
Register SampledTypeReg = Type->getOperand(1).getReg();
881-
auto *SampledType = MIRBuilder.getMRI()->getUniqueVRegDef(SampledTypeReg);
882-
std::string TypeName =
883-
Prefix + buildSpirvTypeName(SampledType, MIRBuilder, GR);
884-
for (uint32_t I = 2; I < Type->getNumOperands(); ++I) {
885-
TypeName = (TypeName + '_' + Twine(Type->getOperand(I).getImm())).str();
886-
}
887-
return TypeName;
888-
}
889-
890805
Register SPIRVGlobalRegistry::getOrCreateGlobalVariableWithBinding(
891-
const SPIRVType *VarType, uint32_t Set, uint32_t Binding,
806+
const SPIRVType *VarType, uint32_t Set, uint32_t Binding, StringRef Name,
892807
MachineIRBuilder &MIRBuilder) {
893808
Register VarReg =
894809
MIRBuilder.getMRI()->createVirtualRegister(&SPIRV::iIDRegClass);
895810

896-
// TODO(138533): The name should come from the llvm-ir, but how that name will
897-
// be passed from the HLSL to the backend has not been decided. Using this
898-
// place holder for now.
899-
std::string Name =
900-
("__resource_" + buildSpirvTypeName(VarType, MIRBuilder, *this) + "_" +
901-
Twine(Set) + "_" + Twine(Binding))
902-
.str();
903811
buildGlobalVariable(VarReg, VarType, Name, nullptr,
904812
getPointerStorageClass(VarType), nullptr, false, false,
905813
SPIRV::LinkageType::Import, MIRBuilder, false);

‎llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -548,6 +548,7 @@ class SPIRVGlobalRegistry : public SPIRVIRMapping {
548548
bool IsInstSelector);
549549
Register getOrCreateGlobalVariableWithBinding(const SPIRVType *VarType,
550550
uint32_t Set, uint32_t Binding,
551+
StringRef Name,
551552
MachineIRBuilder &MIRBuilder);
552553

553554
// Convenient helpers for getting types with check for duplicates.

‎llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ class SPIRVInstructionSelector : public InstructionSelector {
322322
SPIRV::StorageClass::StorageClass SC,
323323
uint32_t Set, uint32_t Binding,
324324
uint32_t ArraySize, Register IndexReg,
325-
bool IsNonUniform,
325+
bool IsNonUniform, StringRef Name,
326326
MachineIRBuilder MIRBuilder) const;
327327
SPIRVType *widenTypeToVec4(const SPIRVType *Type, MachineInstr &I) const;
328328
bool extractSubvector(Register &ResVReg, const SPIRVType *ResType,
@@ -3380,22 +3380,22 @@ bool SPIRVInstructionSelector::selectImageWriteIntrinsic(
33803380
Register SPIRVInstructionSelector::buildPointerToResource(
33813381
const SPIRVType *SpirvResType, SPIRV::StorageClass::StorageClass SC,
33823382
uint32_t Set, uint32_t Binding, uint32_t ArraySize, Register IndexReg,
3383-
bool IsNonUniform, MachineIRBuilder MIRBuilder) const {
3383+
bool IsNonUniform, StringRef Name, MachineIRBuilder MIRBuilder) const {
33843384
const Type *ResType = GR.getTypeForSPIRVType(SpirvResType);
33853385
if (ArraySize == 1) {
33863386
SPIRVType *PtrType =
33873387
GR.getOrCreateSPIRVPointerType(ResType, MIRBuilder, SC);
33883388
assert(GR.getPointeeType(PtrType) == SpirvResType &&
33893389
"SpirvResType did not have an explicit layout.");
3390-
return GR.getOrCreateGlobalVariableWithBinding(PtrType, Set, Binding,
3390+
return GR.getOrCreateGlobalVariableWithBinding(PtrType, Set, Binding, Name,
33913391
MIRBuilder);
33923392
}
33933393

33943394
const Type *VarType = ArrayType::get(const_cast<Type *>(ResType), ArraySize);
33953395
SPIRVType *VarPointerType =
33963396
GR.getOrCreateSPIRVPointerType(VarType, MIRBuilder, SC);
33973397
Register VarReg = GR.getOrCreateGlobalVariableWithBinding(
3398-
VarPointerType, Set, Binding, MIRBuilder);
3398+
VarPointerType, Set, Binding, Name, MIRBuilder);
33993399

34003400
SPIRVType *ResPointerType =
34013401
GR.getOrCreateSPIRVPointerType(ResType, MIRBuilder, SC);
@@ -4081,6 +4081,9 @@ bool SPIRVInstructionSelector::loadHandleBeforePosition(
40814081
uint32_t ArraySize = foldImm(HandleDef.getOperand(4), MRI);
40824082
Register IndexReg = HandleDef.getOperand(5).getReg();
40834083
bool IsNonUniform = ArraySize > 1 && foldImm(HandleDef.getOperand(6), MRI);
4084+
std::string Name =
4085+
getStringValueFromReg(HandleDef.getOperand(7).getReg(), *MRI);
4086+
40844087
bool IsStructuredBuffer = ResType->getOpcode() == SPIRV::OpTypePointer;
40854088
MachineIRBuilder MIRBuilder(HandleDef);
40864089
SPIRVType *VarType = ResType;
@@ -4091,8 +4094,9 @@ bool SPIRVInstructionSelector::loadHandleBeforePosition(
40914094
SC = GR.getPointerStorageClass(ResType);
40924095
}
40934096

4094-
Register VarReg = buildPointerToResource(VarType, SC, Set, Binding, ArraySize,
4095-
IndexReg, IsNonUniform, MIRBuilder);
4097+
Register VarReg =
4098+
buildPointerToResource(VarType, SC, Set, Binding, ArraySize, IndexReg,
4099+
IsNonUniform, Name, MIRBuilder);
40964100

40974101
if (IsNonUniform)
40984102
buildOpDecorate(HandleReg, HandleDef, TII, SPIRV::Decoration::NonUniformEXT,

‎llvm/lib/Target/SPIRV/SPIRVUtils.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,16 @@ std::string getStringImm(const MachineInstr &MI, unsigned StartIndex) {
8080
return getSPIRVStringOperand(MI, StartIndex);
8181
}
8282

83+
std::string getStringValueFromReg(Register Reg, MachineRegisterInfo &MRI) {
84+
MachineInstr *Def = getVRegDef(MRI, Reg);
85+
assert(Def && Def->getOpcode() == TargetOpcode::G_GLOBAL_VALUE &&
86+
"Expected G_GLOBAL_VALUE");
87+
const GlobalValue *GV = Def->getOperand(1).getGlobal();
88+
Value *V = GV->getOperand(0);
89+
const ConstantDataArray *CDA = cast<ConstantDataArray>(V);
90+
return CDA->getAsCString().str();
91+
}
92+
8393
void addNumImm(const APInt &Imm, MachineInstrBuilder &MIB) {
8494
const auto Bitwidth = Imm.getBitWidth();
8595
if (Bitwidth == 1)

‎llvm/lib/Target/SPIRV/SPIRVUtils.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,10 @@ void addStringImm(const StringRef &Str, IRBuilder<> &B,
125125
// the reverse of the logic in addStringImm.
126126
std::string getStringImm(const MachineInstr &MI, unsigned StartIndex);
127127

128+
// Returns the string constant that the register refers to. It is assumed that
129+
// Reg is a global value that contains a string.
130+
std::string getStringValueFromReg(Register Reg, MachineRegisterInfo &MRI);
131+
128132
// Add the given numerical immediate to MIB.
129133
void addNumImm(const APInt &Imm, MachineInstrBuilder &MIB);
130134

0 commit comments

Comments
 (0)