[HLSL] Rework semantic handling as attributes #166796
Open
+269
−361
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
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-codegen Author: Nathan Gauër (Keenuts) ChangesPreviously, we had 2 level of attributes:
The first was assigned during parsing, and carried no other meaning than "there is a semantic token". It was then converted to one of the N attributes later during Sema. This had a few issues:
Also, existing implementation had effective attributes attached to the type decl or parameters, meaning struct decl reuse across entrypoints of in a nested type was not supported, even if legal in HLSL. This PR tried to simplifies semantic attribute by having 3 attributes:
Initial parsing emits an Then, Sema does initial validation and transforms an UnparseSemantic into a ParsedSemantic. This validates a system semantic is known, or that the associated type is valid (like uint3 for a ThreadIndex). Then, once we parse an actual shader entrypoint, we can know how semantics are used in a real context. This step emits a list of AppliedSemantic. Those are the actual semantic in use for this specific entrypoint. This allows codegen to simply iterate over this list and emit the proper DXIL or SPIR-V codegen. Patch is 55.42 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/166796.diff 20 Files Affected:
diff --git a/clang/include/clang/AST/Attr.h b/clang/include/clang/AST/Attr.h
index 14d7caa0e16d7..e36184f232f8a 100644
--- a/clang/include/clang/AST/Attr.h
+++ b/clang/include/clang/AST/Attr.h
@@ -233,44 +233,19 @@ class HLSLAnnotationAttr : public InheritableAttr {
}
};
-class HLSLSemanticAttr : public HLSLAnnotationAttr {
- unsigned SemanticIndex = 0;
- LLVM_PREFERRED_TYPE(bool)
- unsigned SemanticIndexable : 1;
- LLVM_PREFERRED_TYPE(bool)
- unsigned SemanticExplicitIndex : 1;
-
- Decl *TargetDecl = nullptr;
-
+class HLSLSemanticBaseAttr : public HLSLAnnotationAttr {
protected:
- HLSLSemanticAttr(ASTContext &Context, const AttributeCommonInfo &CommonInfo,
- attr::Kind AK, bool IsLateParsed,
- bool InheritEvenIfAlreadyPresent, bool SemanticIndexable)
+ HLSLSemanticBaseAttr(ASTContext &Context,
+ const AttributeCommonInfo &CommonInfo, attr::Kind AK,
+ bool IsLateParsed, bool InheritEvenIfAlreadyPresent)
: HLSLAnnotationAttr(Context, CommonInfo, AK, IsLateParsed,
- InheritEvenIfAlreadyPresent) {
- this->SemanticIndexable = SemanticIndexable;
- this->SemanticExplicitIndex = false;
- }
+ InheritEvenIfAlreadyPresent) {}
public:
- bool isSemanticIndexable() const { return SemanticIndexable; }
-
- void setSemanticIndex(unsigned SemanticIndex) {
- this->SemanticIndex = SemanticIndex;
- this->SemanticExplicitIndex = true;
- }
-
- unsigned getSemanticIndex() const { return SemanticIndex; }
-
- bool isSemanticIndexExplicit() const { return SemanticExplicitIndex; }
-
- void setTargetDecl(Decl *D) { TargetDecl = D; }
- Decl *getTargetDecl() const { return TargetDecl; }
-
// Implement isa/cast/dyncast/etc.
static bool classof(const Attr *A) {
- return A->getKind() >= attr::FirstHLSLSemanticAttr &&
- A->getKind() <= attr::LastHLSLSemanticAttr;
+ return A->getKind() >= attr::FirstHLSLSemanticBaseAttr &&
+ A->getKind() <= attr::LastHLSLSemanticBaseAttr;
}
};
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 1013bfc575747..9081897bbf147 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -779,18 +779,6 @@ class DeclOrStmtAttr : InheritableAttr;
/// An attribute class for HLSL Annotations.
class HLSLAnnotationAttr : InheritableAttr;
-class HLSLSemanticAttr<bit Indexable> : HLSLAnnotationAttr {
- bit SemanticIndexable = Indexable;
- int SemanticIndex = 0;
- bit SemanticExplicitIndex = 0;
-
- let Spellings = [];
- let Subjects = SubjectList<[ParmVar, Field, Function]>;
- let LangOpts = [HLSL];
- let Args = [DeclArgument<Named, "Target">, IntArgument<"SemanticIndex">,
- BoolArgument<"SemanticExplicitIndex">];
-}
-
/// A target-specific attribute. This class is meant to be used as a mixin
/// with InheritableAttr or Attr depending on the attribute's needs.
class TargetSpecificAttr<TargetSpec target> {
@@ -5017,28 +5005,30 @@ def HLSLUnparsedSemantic : HLSLAnnotationAttr {
let Documentation = [InternalOnly];
}
-def HLSLUserSemantic : HLSLSemanticAttr</* Indexable= */ 1> {
- let Documentation = [InternalOnly];
-}
+class HLSLSemanticBaseAttr : HLSLAnnotationAttr {
+ int SemanticIndex = 0;
-def HLSLSV_Position : HLSLSemanticAttr</* Indexable= */ 1> {
- let Documentation = [HLSLSV_PositionDocs];
+ let Spellings = [];
+ let Subjects = SubjectList<[ParmVar, Field, Function]>;
+ let LangOpts = [HLSL];
}
-def HLSLSV_GroupThreadID : HLSLSemanticAttr</* Indexable= */ 0> {
- let Documentation = [HLSLSV_GroupThreadIDDocs];
-}
+def HLSLParsedSemantic : HLSLSemanticBaseAttr {
+ let Spellings = [];
+ let Subjects = SubjectList<[ParmVar, Field, Function]>;
+ let LangOpts = [HLSL];
+ let Documentation = [InternalOnly];
-def HLSLSV_GroupID : HLSLSemanticAttr</* Indexable= */ 0> {
- let Documentation = [HLSLSV_GroupIDDocs];
+ let Args = [StringArgument<"SemanticName">, IntArgument<"SemanticIndex">];
}
-def HLSLSV_GroupIndex : HLSLSemanticAttr</* Indexable= */ 0> {
- let Documentation = [HLSLSV_GroupIndexDocs];
-}
+def HLSLAppliedSemantic : HLSLSemanticBaseAttr {
+ let Spellings = [];
+ let Subjects = SubjectList<[ParmVar, Field, Function]>;
+ let LangOpts = [HLSL];
+ let Documentation = [InternalOnly];
-def HLSLSV_DispatchThreadID : HLSLSemanticAttr</* Indexable= */ 0> {
- let Documentation = [HLSLSV_DispatchThreadIDDocs];
+ let Args = [StringArgument<"SemanticName">, IntArgument<"SemanticIndex">];
}
def HLSLPackOffset: HLSLAnnotationAttr {
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 1be9a96aa44de..e064e69ce6ece 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -8672,38 +8672,6 @@ randomized.
}];
}
-def HLSLSV_GroupThreadIDDocs : Documentation {
- let Category = DocHLSLSemantics;
- let Content = [{
-The ``SV_GroupThreadID`` semantic, when applied to an input parameter, specifies which
-individual thread within a thread group is executing in. This attribute is
-only supported in compute shaders.
-
-The full documentation is available here: https://docs.microsoft.com/en-us/windows/win32/direct3dhlsl/sv-groupthreadid
- }];
-}
-
-def HLSLSV_GroupIDDocs : Documentation {
- let Category = DocHLSLSemantics;
- let Content = [{
-The ``SV_GroupID`` semantic, when applied to an input parameter, specifies which
-thread group a shader is executing in. This attribute is only supported in compute shaders.
-
-The full documentation is available here: https://docs.microsoft.com/en-us/windows/win32/direct3dhlsl/sv-groupid
- }];
-}
-
-def HLSLSV_GroupIndexDocs : Documentation {
- let Category = DocHLSLSemantics;
- let Content = [{
-The ``SV_GroupIndex`` semantic, when applied to an input parameter, specifies a
-data binding to map the group index to the specified parameter. This attribute
-is only supported in compute shaders.
-
-The full documentation is available here: https://docs.microsoft.com/en-us/windows/win32/direct3dhlsl/sv-groupindex
- }];
-}
-
def HLSLResourceBindingDocs : Documentation {
let Category = DocCatFunction;
let Content = [{
@@ -8750,35 +8718,6 @@ The full documentation is available here: https://learn.microsoft.com/en-us/wind
}];
}
-def HLSLSV_DispatchThreadIDDocs : Documentation {
- let Category = DocHLSLSemantics;
- let Content = [{
-The ``SV_DispatchThreadID`` semantic, when applied to an input parameter,
-specifies a data binding to map the global thread offset within the Dispatch
-call (per dimension of the group) to the specified parameter.
-When applied to a field of a struct, the data binding is specified to the field
-when the struct is used as a parameter type.
-The semantic on the field is ignored when not used as a parameter.
-This attribute is only supported in compute shaders.
-
-The full documentation is available here: https://docs.microsoft.com/en-us/windows/win32/direct3dhlsl/sv-dispatchthreadid
- }];
-}
-
-def HLSLSV_PositionDocs : Documentation {
- let Category = DocHLSLSemantics;
- let Content = [{
-The ``SV_Position`` semantic, when applied to an input parameter in a pixel
-shader, contains the location of the pixel center (x, y) in screen space.
-This semantic can be applied to the parameter, or a field in a struct used
-as an input parameter.
-This attribute is supported as an input in pixel, hull, domain and mesh shaders.
-This attribute is supported as an output in vertex, geometry and domain shaders.
-
-The full documentation is available here: https://docs.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-semantics
- }];
-}
-
def HLSLGroupSharedAddressSpaceDocs : Documentation {
let Category = DocCatVariable;
let Content = [{
diff --git a/clang/include/clang/Sema/SemaHLSL.h b/clang/include/clang/Sema/SemaHLSL.h
index 28b03ac4c4676..86da323892f98 100644
--- a/clang/include/clang/Sema/SemaHLSL.h
+++ b/clang/include/clang/Sema/SemaHLSL.h
@@ -178,18 +178,11 @@ class SemaHLSL : public SemaBase {
bool handleResourceTypeAttr(QualType T, const ParsedAttr &AL);
template <typename T>
- T *createSemanticAttr(const AttributeCommonInfo &ACI, NamedDecl *TargetDecl,
+ T *createSemanticAttr(const AttributeCommonInfo &ACI,
std::optional<unsigned> Location) {
- T *Attr =
- ::new (getASTContext()) T(getASTContext(), ACI, TargetDecl,
- Location.value_or(0), Location.has_value());
-
- if (!Attr->isSemanticIndexable() && Location.has_value()) {
- Diag(Attr->getLocation(), diag::err_hlsl_semantic_indexing_not_supported)
- << Attr->getAttrName()->getName();
- return nullptr;
- }
- return Attr;
+ return ::new (getASTContext())
+ T(getASTContext(), ACI, ACI.getAttrName()->getName(),
+ Location.value_or(0));
}
void diagnoseSystemSemanticAttr(Decl *D, const ParsedAttr &AL,
@@ -247,7 +240,7 @@ class SemaHLSL : public SemaBase {
IdentifierInfo *RootSigOverrideIdent = nullptr;
struct SemanticInfo {
- HLSLSemanticAttr *Semantic;
+ HLSLParsedSemanticAttr *Semantic;
std::optional<uint32_t> Index;
};
@@ -257,14 +250,14 @@ class SemaHLSL : public SemaBase {
const RecordType *RT);
void checkSemanticAnnotation(FunctionDecl *EntryPoint, const Decl *Param,
- const HLSLSemanticAttr *SemanticAttr);
- HLSLSemanticAttr *createSemantic(const SemanticInfo &Semantic,
- DeclaratorDecl *TargetDecl);
- bool determineActiveSemanticOnScalar(FunctionDecl *FD, DeclaratorDecl *D,
+ const HLSLAppliedSemanticAttr *SemanticAttr);
+ bool determineActiveSemanticOnScalar(FunctionDecl *FD,
+ DeclaratorDecl *OutputDecl,
+ DeclaratorDecl *D,
SemanticInfo &ActiveSemantic,
llvm::StringSet<> &ActiveInputSemantics);
- bool determineActiveSemantic(FunctionDecl *FD, DeclaratorDecl *D,
- SemanticInfo &ActiveSemantic,
+ bool determineActiveSemantic(FunctionDecl *FD, DeclaratorDecl *OutputDecl,
+ DeclaratorDecl *D, SemanticInfo &ActiveSemantic,
llvm::StringSet<> &ActiveInputSemantics);
void processExplicitBindingsOnDecl(VarDecl *D);
diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index e392a12044a39..d40ba2539cd00 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -587,7 +587,7 @@ static llvm::Value *createSPIRVLocationLoad(IRBuilder<> &B, llvm::Module &M,
llvm::Value *
CGHLSLRuntime::emitSPIRVUserSemanticLoad(llvm::IRBuilder<> &B, llvm::Type *Type,
- HLSLSemanticAttr *Semantic,
+ HLSLAppliedSemanticAttr *Semantic,
std::optional<unsigned> Index) {
Twine BaseName = Twine(Semantic->getAttrName()->getName());
Twine VariableName = BaseName.concat(Twine(Index.value_or(0)));
@@ -605,7 +605,7 @@ CGHLSLRuntime::emitSPIRVUserSemanticLoad(llvm::IRBuilder<> &B, llvm::Type *Type,
llvm::Value *
CGHLSLRuntime::emitDXILUserSemanticLoad(llvm::IRBuilder<> &B, llvm::Type *Type,
- HLSLSemanticAttr *Semantic,
+ HLSLAppliedSemanticAttr *Semantic,
std::optional<unsigned> Index) {
Twine BaseName = Twine(Semantic->getAttrName()->getName());
Twine VariableName = BaseName.concat(Twine(Index.value_or(0)));
@@ -625,7 +625,7 @@ CGHLSLRuntime::emitDXILUserSemanticLoad(llvm::IRBuilder<> &B, llvm::Type *Type,
llvm::Value *CGHLSLRuntime::emitUserSemanticLoad(
IRBuilder<> &B, llvm::Type *Type, const clang::DeclaratorDecl *Decl,
- HLSLSemanticAttr *Semantic, std::optional<unsigned> Index) {
+ HLSLAppliedSemanticAttr *Semantic, std::optional<unsigned> Index) {
if (CGM.getTarget().getTriple().isSPIRV())
return emitSPIRVUserSemanticLoad(B, Type, Semantic, Index);
@@ -637,14 +637,16 @@ llvm::Value *CGHLSLRuntime::emitUserSemanticLoad(
llvm::Value *CGHLSLRuntime::emitSystemSemanticLoad(
IRBuilder<> &B, llvm::Type *Type, const clang::DeclaratorDecl *Decl,
- Attr *Semantic, std::optional<unsigned> Index) {
- if (isa<HLSLSV_GroupIndexAttr>(Semantic)) {
+ HLSLSemanticBaseAttr *Semantic, std::optional<unsigned> Index) {
+
+ std::string SemanticName = Semantic->getAttrName()->getName().upper();
+ if (SemanticName == "SV_GROUPINDEX") {
llvm::Function *GroupIndex =
CGM.getIntrinsic(getFlattenedThreadIdInGroupIntrinsic());
return B.CreateCall(FunctionCallee(GroupIndex));
}
- if (isa<HLSLSV_DispatchThreadIDAttr>(Semantic)) {
+ if (SemanticName == "SV_DISPATCHTHREADID") {
llvm::Intrinsic::ID IntrinID = getThreadIdIntrinsic();
llvm::Function *ThreadIDIntrinsic =
llvm::Intrinsic::isOverloaded(IntrinID)
@@ -653,7 +655,7 @@ llvm::Value *CGHLSLRuntime::emitSystemSemanticLoad(
return buildVectorInput(B, ThreadIDIntrinsic, Type);
}
- if (isa<HLSLSV_GroupThreadIDAttr>(Semantic)) {
+ if (SemanticName == "SV_GROUPTHREADID") {
llvm::Intrinsic::ID IntrinID = getGroupThreadIdIntrinsic();
llvm::Function *GroupThreadIDIntrinsic =
llvm::Intrinsic::isOverloaded(IntrinID)
@@ -662,7 +664,7 @@ llvm::Value *CGHLSLRuntime::emitSystemSemanticLoad(
return buildVectorInput(B, GroupThreadIDIntrinsic, Type);
}
- if (isa<HLSLSV_GroupIDAttr>(Semantic)) {
+ if (SemanticName == "SV_GROUPID") {
llvm::Intrinsic::ID IntrinID = getGroupIdIntrinsic();
llvm::Function *GroupIDIntrinsic =
llvm::Intrinsic::isOverloaded(IntrinID)
@@ -671,44 +673,31 @@ llvm::Value *CGHLSLRuntime::emitSystemSemanticLoad(
return buildVectorInput(B, GroupIDIntrinsic, Type);
}
- if (HLSLSV_PositionAttr *S = dyn_cast<HLSLSV_PositionAttr>(Semantic)) {
+ if (SemanticName == "SV_POSITION") {
if (CGM.getTriple().getEnvironment() == Triple::EnvironmentType::Pixel)
return createSPIRVBuiltinLoad(B, CGM.getModule(), Type,
- S->getAttrName()->getName(),
+ Semantic->getAttrName()->getName(),
/* BuiltIn::FragCoord */ 15);
}
llvm_unreachable("non-handled system semantic. FIXME.");
}
-llvm::Value *
-CGHLSLRuntime::handleScalarSemanticLoad(IRBuilder<> &B, const FunctionDecl *FD,
- llvm::Type *Type,
- const clang::DeclaratorDecl *Decl) {
-
- HLSLSemanticAttr *Semantic = nullptr;
- for (HLSLSemanticAttr *Item : FD->specific_attrs<HLSLSemanticAttr>()) {
- if (Item->getTargetDecl() == Decl) {
- Semantic = Item;
- break;
- }
- }
- // Sema must create one attribute per scalar field.
- assert(Semantic);
-
- std::optional<unsigned> Index = std::nullopt;
- if (Semantic->isSemanticIndexExplicit())
- Index = Semantic->getSemanticIndex();
+llvm::Value *CGHLSLRuntime::handleScalarSemanticLoad(
+ IRBuilder<> &B, const FunctionDecl *FD, llvm::Type *Type,
+ const clang::DeclaratorDecl *Decl, HLSLAppliedSemanticAttr *Semantic) {
- if (isa<HLSLUserSemanticAttr>(Semantic))
- return emitUserSemanticLoad(B, Type, Decl, Semantic, Index);
- return emitSystemSemanticLoad(B, Type, Decl, Semantic, Index);
+ std::optional<unsigned> Index = Semantic->getSemanticIndex();
+ if (Semantic->getAttrName()->getName().starts_with_insensitive("SV_"))
+ return emitSystemSemanticLoad(B, Type, Decl, Semantic, Index);
+ return emitUserSemanticLoad(B, Type, Decl, Semantic, Index);
}
-llvm::Value *
-CGHLSLRuntime::handleStructSemanticLoad(IRBuilder<> &B, const FunctionDecl *FD,
- llvm::Type *Type,
- const clang::DeclaratorDecl *Decl) {
+llvm::Value *CGHLSLRuntime::handleStructSemanticLoad(
+ IRBuilder<> &B, const FunctionDecl *FD, llvm::Type *Type,
+ const clang::DeclaratorDecl *Decl,
+ specific_attr_iterator<HLSLAppliedSemanticAttr> &begin,
+ specific_attr_iterator<HLSLAppliedSemanticAttr> end) {
const llvm::StructType *ST = cast<StructType>(Type);
const clang::RecordDecl *RD = Decl->getType()->getAsRecordDecl();
@@ -718,8 +707,8 @@ CGHLSLRuntime::handleStructSemanticLoad(IRBuilder<> &B, const FunctionDecl *FD,
llvm::Value *Aggregate = llvm::PoisonValue::get(Type);
auto FieldDecl = RD->field_begin();
for (unsigned I = 0; I < ST->getNumElements(); ++I) {
- llvm::Value *ChildValue =
- handleSemanticLoad(B, FD, ST->getElementType(I), *FieldDecl);
+ llvm::Value *ChildValue = handleSemanticLoad(B, FD, ST->getElementType(I),
+ *FieldDecl, begin, end);
assert(ChildValue);
Aggregate = B.CreateInsertValue(Aggregate, ChildValue, I);
++FieldDecl;
@@ -728,13 +717,18 @@ CGHLSLRuntime::handleStructSemanticLoad(IRBuilder<> &B, const FunctionDecl *FD,
return Aggregate;
}
-llvm::Value *
-CGHLSLRuntime::handleSemanticLoad(IRBuilder<> &B, const FunctionDecl *FD,
- llvm::Type *Type,
- const clang::DeclaratorDecl *Decl) {
+llvm::Value *CGHLSLRuntime::handleSemanticLoad(
+ IRBuilder<> &B, const FunctionDecl *FD, llvm::Type *Type,
+ const clang::DeclaratorDecl *Decl,
+ specific_attr_iterator<HLSLAppliedSemanticAttr> &begin,
+ specific_attr_iterator<HLSLAppliedSemanticAttr> end) {
+ assert(end != begin);
if (Type->isStructTy())
- return handleStructSemanticLoad(B, FD, Type, Decl);
- return handleScalarSemanticLoad(B, FD, Type, Decl);
+ return handleStructSemanticLoad(B, FD, Type, Decl, begin, end);
+
+ HLSLAppliedSemanticAttr *Attr = *begin;
+ ++begin;
+ return handleScalarSemanticLoad(B, FD, Type, Decl, Attr);
}
void CGHLSLRuntime::emitEntryFunction(const FunctionDecl *FD,
@@ -781,6 +775,8 @@ void CGHLSLRuntime::emitEntryFunction(const FunctionDecl *FD,
}
const ParmVarDecl *PD = FD->getParamDecl(Param.getArgNo() - SRetOffset);
+ auto AttrBegin = PD->specific_attr_begin<HLSLAppliedSemanticAttr>();
+ auto AttrEnd = PD->specific_attr_end<HLSLAppliedSemanticAttr>();
llvm::Value *SemanticValue = nullptr;
if ([[maybe_unused]] HLSLParamModifierAttr *MA =
PD->getAttr<HLSLParamModifierAttr>()) {
@@ -788,7 +784,8 @@ void CGHLSLRuntime::emitEntryFunction(const FunctionDecl *FD,
} else {
llvm::Type *ParamType =
Param.hasByValAttr() ? Param.getParamByValType() : Param.getType();
- SemanticValue = handleSemanticLoad(B, FD, ParamType, PD);
+ SemanticValue =
+ handleSemanticLoad(B, FD, ParamType, PD, AttrBegin, AttrEnd);
if (!SemanticValue)
return;
if (Param.hasByValAttr()) {
diff --git a/clang/lib/CodeGen/CGHLSLRuntime.h b/clang/lib/CodeGen/CGHLSLRuntime.h
index 9d31714ab8606..1c844f8c218ef 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.h
+++ b/clang/lib/CodeGen/CGHLSLRuntime.h
@@ -146,22 +146,26 @@ class CGHLSLRuntime {
llvm::Value *emitSystemSemanticLoad(llvm::IRBuilder<> &B, llvm::Type *Type,
const clang::DeclaratorDecl *Decl,
- Attr *Semantic,
+ HLSLSemanticBaseAttr *Semantic,
std::optional<unsigned> Index);
llvm::Value *handleScalarSemanticLoad(llvm::IRBuilder<> &B,
const FunctionDecl *FD,
llvm::Type *Type,
- const clang::DeclaratorDecl *Decl);
+ const clang::DeclaratorDecl *Decl,
+ HLSLAppliedSemanticAttr *Semantic);
- llvm::Value *handleStructSemanticLoad(llvm::IRBuilder<> &B,
- const FunctionDecl *FD,
- llvm::Type *Type,
- ...
[truncated]
|
Previously, we had 2 level of attributes: - HLSLUnparsedSemantic - N attributes, one for each known system semantic. The first was assigned during parsing, and carried no other meaning than "there is a semantic token". It was then converted to one of the N attributes later during Sema. Those attributes also carried informations like "is indexable" or "is index explicit". This had a few issues: - there was no difference between a semantic attribute applied to a decl, and the effective semantic in the entrypoint use context. - having the indexable bit was not useful. - semantic constraints checks were split between .td files and sema. Also, existing implementation had effective attributes attached to the type decl or parameters, meaning struct decl reuse across entrypoints of in a nested type was not supported, even if legal in HLSL. This PR tried to simplifies semantic attribute by having 3 attributes: - HLSLUnpasedSemantic - HLSLParsedSemantic - HLSLAppliedSemantic Initial parsing emits an `HLSLUnparsedSemantic`. We simply say "here is an HLSL semantic token", but we don't do any semantic check. Then, Sema does initial validation and transforms an UnparseSemantic into a ParsedSemantic. This validates a system semantic is known, or that the associated type is valid (like uint3 for a ThreadIndex). Then, once we parse an actual shader entrypoint, we can know how semantics are used in a real context. This step emits a list of AppliedSemantic. Those are the actual semantic in use for this specific entrypoint. Those attributes are attached to each entrypoint parameter, as a flat list matching the semantic structure flattening HLSL defines. At this stage of sema, index collision or other stage compabitility checkes are carried. This allows codegen to simply iterate over this list and emit the proper DXIL or SPIR-V codegen.
Keenuts
force-pushed
the
hlsl-attr-refactoring
branch
from
296b978 to
0afad1c
Compare
s-perron
reviewed
Nov 10, 2025
Contributor
s-perron
left a comment
s-perron
left a comment
There was a problem hiding this comment.
A few minor style comments.
|
|
||
| def HLSLSV_GroupID : HLSLSemanticAttr</* Indexable= */ 0> { | ||
| let Documentation = [HLSLSV_GroupIDDocs]; | ||
| let Args = [StringArgument<"SemanticName">, IntArgument<"SemanticIndex">]; |
Contributor
There was a problem hiding this comment.
I don't know how the inheritance works, but could this be part of the base class? Both children of the base class have the same arguments.
| llvm::Value *emitSystemSemanticLoad(llvm::IRBuilder<> &B, llvm::Type *Type, | ||
| const clang::DeclaratorDecl *Decl, | ||
| Attr *Semantic, | ||
| HLSLSemanticBaseAttr *Semantic, |
Contributor
There was a problem hiding this comment.
Why use the base? Shouldn't this only be needed for an applied semantic?
Comment on lines
+778
to
+779
| auto AttrBegin = PD->specific_attr_begin<HLSLAppliedSemanticAttr>(); | ||
| auto AttrEnd = PD->specific_attr_end<HLSLAppliedSemanticAttr>(); |
Contributor
There was a problem hiding this comment.
Could you move these into the else block closer to their use?
Comment on lines
+710
to
+711
| llvm::Value *ChildValue = handleSemanticLoad(B, FD, ST->getElementType(I), | ||
| *FieldDecl, begin, end); |
Contributor
There was a problem hiding this comment.
At first glance, it looks like this is wrong because it is not obvious that begin is updated. If I have to go look at the declaration or definition of the handleSemanticLoad to understands what is happening, it is a sign that this could be better.
Off the top of my head I don't have a good suggestion other than have the function return one-past-the-last semantic that it consumed. Have it return a pair.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Previously, we had 2 level of attributes:
The first was assigned during parsing, and carried no other meaning than "there is a semantic token". It was then converted to one of the N attributes later during Sema.
Those attributes also carried informations like "is indexable" or "is index explicit".
This had a few issues:
Also, existing implementation had effective attributes attached to the type decl or parameters, meaning struct decl reuse across entrypoints of in a nested type was not supported, even if legal in HLSL.
This PR tried to simplifies semantic attribute by having 3 attributes:
Initial parsing emits an
HLSLUnparsedSemantic. We simply say "here is an HLSL semantic token", but we don't do any semantic check.Then, Sema does initial validation and transforms an UnparseSemantic into a ParsedSemantic. This validates a system semantic is known, or that the associated type is valid (like uint3 for a ThreadIndex).
Then, once we parse an actual shader entrypoint, we can know how semantics are used in a real context. This step emits a list of AppliedSemantic. Those are the actual semantic in use for this specific entrypoint.
Those attributes are attached to each entrypoint parameter, as a flat list matching the semantic structure flattening HLSL defines. At this stage of sema, index collision or other stage compabitility checkes are carried.
This allows codegen to simply iterate over this list and emit the proper DXIL or SPIR-V codegen.