Reapply "[HLSL] Rework semantic handling as attributes" #167862
Merged
Reapply "[HLSL] Rework semantic handling as attributes" #167862
Conversation
Re-land of llvm#166796 following CI failures. This reverts commit 1d2429b.
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-hlsl @llvm/pr-subscribers-clang Author: Nathan Gauër (Keenuts) ChangesLast PR had asan failures due to bad use of a Twine instead of an std::string. Patch is 55.33 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/167862.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 aac8c1f550cb2..8dfe4bc08c48e 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -783,18 +783,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> {
@@ -5021,28 +5009,28 @@ def HLSLUnparsedSemantic : HLSLAnnotationAttr {
let Documentation = [InternalOnly];
}
-def HLSLUserSemantic : HLSLSemanticAttr</* Indexable= */ 1> {
- let Documentation = [InternalOnly];
-}
-
-def HLSLSV_Position : HLSLSemanticAttr</* Indexable= */ 1> {
- let Documentation = [HLSLSV_PositionDocs];
-}
+class HLSLSemanticBaseAttr : HLSLAnnotationAttr {
+ int SemanticIndex = 0;
-def HLSLSV_GroupThreadID : HLSLSemanticAttr</* Indexable= */ 0> {
- let Documentation = [HLSLSV_GroupThreadIDDocs];
-}
+ let Spellings = [];
+ let Subjects = SubjectList<[ParmVar, Field, Function]>;
+ let LangOpts = [HLSL];
-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 HLSLParsedSemantic : 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];
+def HLSLAppliedSemantic : HLSLSemanticBaseAttr {
+ let Spellings = [];
+ let Subjects = SubjectList<[ParmVar, Field, Function]>;
+ let LangOpts = [HLSL];
+ let Documentation = [InternalOnly];
}
def HLSLPackOffset: HLSLAnnotationAttr {
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index f1dbd8af6093a..4813191d2d602 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 4bdba9b3da502..ec02096787c7a 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -573,7 +573,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)));
@@ -591,7 +591,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)));
@@ -611,7 +611,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);
@@ -623,14 +623,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)) {
+ HLSLAppliedSemanticAttr *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)
@@ -639,7 +641,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)
@@ -648,7 +650,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)
@@ -657,44 +659,32 @@ 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) {
+std::pair<llvm::Value *, specific_attr_iterator<HLSLAppliedSemanticAttr>>
+CGHLSLRuntime::handleStructSemanticLoad(
+ IRBuilder<> &B, const FunctionDecl *FD, llvm::Type *Type,
+ const clang::DeclaratorDecl *Decl,
+ specific_attr_iterator<HLSLAppliedSemanticAttr> AttrBegin,
+ specific_attr_iterator<HLSLAppliedSemanticAttr> AttrEnd) {
const llvm::StructType *ST = cast<StructType>(Type);
const clang::RecordDecl *RD = Decl->getType()->getAsRecordDecl();
@@ -704,23 +694,31 @@ 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);
+ auto [ChildValue, NextAttr] = handleSemanticLoad(
+ B, FD, ST->getElementType(I), *FieldDecl, AttrBegin, AttrEnd);
+ AttrBegin = NextAttr;
assert(ChildValue);
Aggregate = B.CreateInsertValue(Aggregate, ChildValue, I);
++FieldDecl;
}
- return Aggregate;
+ return std::make_pair(Aggregate, AttrBegin);
}
-llvm::Value *
-CGHLSLRuntime::handleSemanticLoad(IRBuilder<> &B, const FunctionDecl *FD,
- llvm::Type *Type,
- const clang::DeclaratorDecl *Decl) {
+std::pair<llvm::Value *, specific_attr_iterator<HLSLAppliedSemanticAttr>>
+CGHLSLRuntime::handleSemanticLoad(
+ IRBuilder<> &B, const FunctionDecl *FD, llvm::Type *Type,
+ const clang::DeclaratorDecl *Decl,
+ specific_attr_iterator<HLSLAppliedSemanticAttr> AttrBegin,
+ specific_attr_iterator<HLSLAppliedSemanticAttr> AttrEnd) {
+ assert(AttrBegin != AttrEnd);
if (Type->isStructTy())
- return handleStructSemanticLoad(B, FD, Type, Decl);
- return handleScalarSemanticLoad(B, FD, Type, Decl);
+ return handleStructSemanticLoad(B, FD, Type, Decl, AttrBegin, AttrEnd);
+
+ HLSLAppliedSemanticAttr *Attr = *AttrBegin;
+ ++AttrBegin;
+ return std::make_pair(handleScalarSemanticLoad(B, FD, Type, Decl, Attr),
+ AttrBegin);
}
void CGHLSLRuntime::emitEntryFunction(const FunctionDecl *FD,
@@ -774,7 +772,11 @@ void CGHLSLRuntime::emitEntryFunction(const FunctionDecl *FD,
} else {
llvm::Type *ParamType =
Param.hasByValAttr() ? Param.getParamByValType() : Param.getType();
- SemanticValue = handleSemanticLoad(B, FD, ParamType, PD);
+ auto AttrBegin = PD->specific_attr_begin<HLSLAppliedSemanticAttr>();
+ auto AttrEnd = PD->specific_attr_end<HLSLAppliedSemanticAttr>();
+ auto Result =
+ handleSemanticLoad(B, FD, ParamType, PD, AttrBegin, AttrEnd);
+ SemanticValue = Result.first;
if (!SemanticValue)
return;
if (Param.hasByValAttr()) {
diff --git a/clang/lib/CodeGen/CGHLSLRuntime.h b/clang/lib/CodeGen/CGHLSLRuntime.h
index 488a322ca7569..48935584f28a2 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.h
+++ b/clang/lib/CodeGen/CGHLSLRuntime.h
@@ -173,22 +173,27 @@ class CGHLSLRuntime {
llvm::Value *emitSystemSemanticLoad(llvm::IRBuilder<> &B, llvm::Type *Type,
const clang::DeclaratorDecl *Decl,
- Attr *Semantic,
+ HLSLAppliedSemanticAttr *Semantic,
std::optional<unsigned> Index);
llvm::Value *handleScalarSemanticLoad(llvm::IRBuilder<> &B,
const FunctionDecl *FD,
llvm::Type *Type,
- const clang::DeclaratorDecl *Decl);
-
- llvm::Value *handleStructSemanticLoad(llvm::IRBuilder<> &B,
- const FunctionDecl *FD,
- llvm::Type *Type,
- const clang::DeclaratorDecl *Decl);
-
- llvm::Value *handleSemanticLoad(llvm::IRBuilder<> &B, const FunctionDecl *FD,
- llvm::Type *Type,
- const clang::DeclaratorDecl *Decl);
+ const clang::DeclaratorDecl *Decl,...
[truncated]
|
Member
|
@llvm/pr-subscribers-clang-codegen Author: Nathan Gauër (Keenuts) ChangesLast PR had asan failures due to bad use of a Twine instead of an std::string. Patch is 55.33 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/167862.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 aac8c1f550cb2..8dfe4bc08c48e 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -783,18 +783,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> {
@@ -5021,28 +5009,28 @@ def HLSLUnparsedSemantic : HLSLAnnotationAttr {
let Documentation = [InternalOnly];
}
-def HLSLUserSemantic : HLSLSemanticAttr</* Indexable= */ 1> {
- let Documentation = [InternalOnly];
-}
-
-def HLSLSV_Position : HLSLSemanticAttr</* Indexable= */ 1> {
- let Documentation = [HLSLSV_PositionDocs];
-}
+class HLSLSemanticBaseAttr : HLSLAnnotationAttr {
+ int SemanticIndex = 0;
-def HLSLSV_GroupThreadID : HLSLSemanticAttr</* Indexable= */ 0> {
- let Documentation = [HLSLSV_GroupThreadIDDocs];
-}
+ let Spellings = [];
+ let Subjects = SubjectList<[ParmVar, Field, Function]>;
+ let LangOpts = [HLSL];
-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 HLSLParsedSemantic : 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];
+def HLSLAppliedSemantic : HLSLSemanticBaseAttr {
+ let Spellings = [];
+ let Subjects = SubjectList<[ParmVar, Field, Function]>;
+ let LangOpts = [HLSL];
+ let Documentation = [InternalOnly];
}
def HLSLPackOffset: HLSLAnnotationAttr {
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index f1dbd8af6093a..4813191d2d602 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 4bdba9b3da502..ec02096787c7a 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -573,7 +573,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)));
@@ -591,7 +591,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)));
@@ -611,7 +611,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);
@@ -623,14 +623,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)) {
+ HLSLAppliedSemanticAttr *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)
@@ -639,7 +641,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)
@@ -648,7 +650,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)
@@ -657,44 +659,32 @@ 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) {
+std::pair<llvm::Value *, specific_attr_iterator<HLSLAppliedSemanticAttr>>
+CGHLSLRuntime::handleStructSemanticLoad(
+ IRBuilder<> &B, const FunctionDecl *FD, llvm::Type *Type,
+ const clang::DeclaratorDecl *Decl,
+ specific_attr_iterator<HLSLAppliedSemanticAttr> AttrBegin,
+ specific_attr_iterator<HLSLAppliedSemanticAttr> AttrEnd) {
const llvm::StructType *ST = cast<StructType>(Type);
const clang::RecordDecl *RD = Decl->getType()->getAsRecordDecl();
@@ -704,23 +694,31 @@ 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);
+ auto [ChildValue, NextAttr] = handleSemanticLoad(
+ B, FD, ST->getElementType(I), *FieldDecl, AttrBegin, AttrEnd);
+ AttrBegin = NextAttr;
assert(ChildValue);
Aggregate = B.CreateInsertValue(Aggregate, ChildValue, I);
++FieldDecl;
}
- return Aggregate;
+ return std::make_pair(Aggregate, AttrBegin);
}
-llvm::Value *
-CGHLSLRuntime::handleSemanticLoad(IRBuilder<> &B, const FunctionDecl *FD,
- llvm::Type *Type,
- const clang::DeclaratorDecl *Decl) {
+std::pair<llvm::Value *, specific_attr_iterator<HLSLAppliedSemanticAttr>>
+CGHLSLRuntime::handleSemanticLoad(
+ IRBuilder<> &B, const FunctionDecl *FD, llvm::Type *Type,
+ const clang::DeclaratorDecl *Decl,
+ specific_attr_iterator<HLSLAppliedSemanticAttr> AttrBegin,
+ specific_attr_iterator<HLSLAppliedSemanticAttr> AttrEnd) {
+ assert(AttrBegin != AttrEnd);
if (Type->isStructTy())
- return handleStructSemanticLoad(B, FD, Type, Decl);
- return handleScalarSemanticLoad(B, FD, Type, Decl);
+ return handleStructSemanticLoad(B, FD, Type, Decl, AttrBegin, AttrEnd);
+
+ HLSLAppliedSemanticAttr *Attr = *AttrBegin;
+ ++AttrBegin;
+ return std::make_pair(handleScalarSemanticLoad(B, FD, Type, Decl, Attr),
+ AttrBegin);
}
void CGHLSLRuntime::emitEntryFunction(const FunctionDecl *FD,
@@ -774,7 +772,11 @@ void CGHLSLRuntime::emitEntryFunction(const FunctionDecl *FD,
} else {
llvm::Type *ParamType =
Param.hasByValAttr() ? Param.getParamByValType() : Param.getType();
- SemanticValue = handleSemanticLoad(B, FD, ParamType, PD);
+ auto AttrBegin = PD->specific_attr_begin<HLSLAppliedSemanticAttr>();
+ auto AttrEnd = PD->specific_attr_end<HLSLAppliedSemanticAttr>();
+ auto Result =
+ handleSemanticLoad(B, FD, ParamType, PD, AttrBegin, AttrEnd);
+ SemanticValue = Result.first;
if (!SemanticValue)
return;
if (Param.hasByValAttr()) {
diff --git a/clang/lib/CodeGen/CGHLSLRuntime.h b/clang/lib/CodeGen/CGHLSLRuntime.h
index 488a322ca7569..48935584f28a2 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.h
+++ b/clang/lib/CodeGen/CGHLSLRuntime.h
@@ -173,22 +173,27 @@ class CGHLSLRuntime {
llvm::Value *emitSystemSemanticLoad(llvm::IRBuilder<> &B, llvm::Type *Type,
const clang::DeclaratorDecl *Decl,
- Attr *Semantic,
+ HLSLAppliedSemanticAttr *Semantic,
std::optional<unsigned> Index);
llvm::Value *handleScalarSemanticLoad(llvm::IRBuilder<> &B,
const FunctionDecl *FD,
llvm::Type *Type,
- const clang::DeclaratorDecl *Decl);
-
- llvm::Value *handleStructSemanticLoad(llvm::IRBuilder<> &B,
- const FunctionDecl *FD,
- llvm::Type *Type,
- const clang::DeclaratorDecl *Decl);
-
- llvm::Value *handleSemanticLoad(llvm::IRBuilder<> &B, const FunctionDecl *FD,
- llvm::Type *Type,
- const clang::DeclaratorDecl *Decl);
+ const clang::DeclaratorDecl *Decl,...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Contributor
Author
|
CI is passing, let's see if the buildbots are passing this time. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/10/builds/17243 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/51/builds/26939 Here is the relevant piece of the build log for the reference |
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
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.
Last PR had asan failures due to bad use of a Twine instead of an std::string.