https://github.com/jzakharovnv created https://github.com/llvm/llvm-project/pull/231275
- **[HLSL][SPIRV] Implement vk::builtin attribute** - **[HLSL][SPIRV] Implement vk::builtin attribute** - **[HLSL][SPIRV] Implement vk::builtin(PointSize) validation and CodeGen** >From 85d55c3d7723a73eb6cd9b1c912c31a6c15ae2db Mon Sep 17 00:00:00 2001 From: Jonathan Zakharov <[email protected]> Date: Mon, 12 Oct 2026 11:13:10 -0700 Subject: [PATCH 1/3] [HLSL][SPIRV] Implement vk::builtin attribute --- clang/include/clang/Basic/Attr.td | 8 +++ clang/include/clang/Basic/AttrDocs.td | 22 ++++++ .../clang/Basic/DiagnosticSemaKinds.td | 3 + clang/include/clang/Sema/SemaHLSL.h | 1 + clang/lib/Sema/SemaDeclAttr.cpp | 3 + clang/lib/Sema/SemaHLSL.cpp | 52 +++++++++++++- clang/test/AST/HLSL/vk-builtin-attr.hlsl | 29 ++++++++ ...a-attribute-supported-attributes-list.test | 1 + ...antic.explicit-mix-vk-builtin-control.hlsl | 15 +++++ ...emantic.explicit-mix-vk-builtin-field.hlsl | 18 +++++ ...emantic.explicit-mix-vk-builtin-param.hlsl | 11 +++ ...mantic.explicit-mix-vk-builtin-return.hlsl | 14 ++++ .../SemaHLSL/vk-builtin-attr-non-spirv.hlsl | 16 +++++ clang/test/SemaHLSL/vk-builtin-attr.hlsl | 67 +++++++++++++++++++ 14 files changed, 259 insertions(+), 1 deletion(-) create mode 100644 clang/test/AST/HLSL/vk-builtin-attr.hlsl create mode 100644 clang/test/SemaHLSL/Semantics/semantic.explicit-mix-vk-builtin-control.hlsl create mode 100644 clang/test/SemaHLSL/Semantics/semantic.explicit-mix-vk-builtin-field.hlsl create mode 100644 clang/test/SemaHLSL/Semantics/semantic.explicit-mix-vk-builtin-param.hlsl create mode 100644 clang/test/SemaHLSL/Semantics/semantic.explicit-mix-vk-builtin-return.hlsl create mode 100644 clang/test/SemaHLSL/vk-builtin-attr-non-spirv.hlsl create mode 100644 clang/test/SemaHLSL/vk-builtin-attr.hlsl diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index b41c1c78ddd99..91af26ecaaa80 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -5462,6 +5462,14 @@ def HLSLVkLocation : HLSLAnnotationAttr { let Documentation = [HLSLVkLocationDocs]; } +def HLSLVkBuiltIn : HLSLAnnotationAttr, TargetSpecificAttr<TargetSPIRV> { + let Spellings = [CXX11<"vk", "builtin">]; + let Args = [StringArgument<"BuiltIn">]; + let Subjects = SubjectList<[ParmVar, Field, Function], ErrorDiag>; + let LangOpts = [HLSL]; + let Documentation = [HLSLVkBuiltInDocs]; +} + // `row_major` / `column_major` are HLSL keywords that select the in-memory // layout of a matrix-typed declaration. def HLSLRowMajor : TypeAttr { diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index e8ce3c3dea7f3..bc830670689e0 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -9741,6 +9741,28 @@ This attribute maps to the `Location` SPIR-V decoration. }]; } +def HLSLVkBuiltInDocs : Documentation { + let Category = DocCatVariable; + let Content = [{ +Some Vulkan built-in variables have no equivalent in native HLSL, so no +system-value semantic can express them. The `[[vk::builtin]]` attribute marks a +stage input/output variable as a SPIR-V built-in variable, identified by name. +Allowed on function parameters, function returns, and struct fields. This +attribute has no effect when used outside of an entrypoint parameter/parameter +field/return value. + +The attribute is only available when targeting SPIR-V. For any other target it +is ignored with an "unknown attribute ignored" warning. + +The attribute takes a single string literal argument naming the built-in. +Supported built-in names are added incrementally, each landing with its own +type/stage validation and CodeGen support. + +A declaration annotated with `[[vk::builtin]]` does not take part in +`vk::location` assignment, and cannot also be annotated with `[[vk::location]]`. + }]; +} + def HLSLMatrixLayoutDocs : Documentation { let Category = DocCatVariable; let Heading = "row_major, column_major"; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index cc87f556d50dc..50de8f772ace2 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -13788,6 +13788,9 @@ def err_hlsl_semantic_unsupported_iotype_for_stage def err_hlsl_semantic_partial_explicit_indexing : Error<"partial explicit stage input location assignment via " "vk::location(X) unsupported">; +def err_hlsl_vk_builtin_unsupported_value + : Error<"'%0' is not a supported value for the 'vk::builtin' attribute; " + "supported values are: %1">; def warn_hlsl_user_defined_type_missing_member: Warning<"binding type '%select{t|u|b|s|c}0' only applies to types containing %select{SRV resources|UAV resources|constant buffer resources|sampler state|numeric types}0">, InGroup<LegacyConstantRegisterBinding>; def err_hlsl_binding_type_mismatch: Error<"binding type '%select{t|u|b|s|c}0' only applies to %select{SRV resources|UAV resources|constant buffer resources|sampler state|numeric variables in the global scope}0">; diff --git a/clang/include/clang/Sema/SemaHLSL.h b/clang/include/clang/Sema/SemaHLSL.h index 3d62f47dfde69..160e3854d5e92 100644 --- a/clang/include/clang/Sema/SemaHLSL.h +++ b/clang/include/clang/Sema/SemaHLSL.h @@ -186,6 +186,7 @@ class SemaHLSL : public SemaBase { void handleWaveSizeAttr(Decl *D, const ParsedAttr &AL); void handleVkConstantIdAttr(Decl *D, const ParsedAttr &AL); void handleVkBindingAttr(Decl *D, const ParsedAttr &AL); + void handleVkBuiltInAttr(Decl *D, const ParsedAttr &AL); void handleVkLocationAttr(Decl *D, const ParsedAttr &AL); void handlePackOffsetAttr(Decl *D, const ParsedAttr &AL); void handleShaderAttr(Decl *D, const ParsedAttr &AL); diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 9e76c640fd339..b2bd95f11a659 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -8299,6 +8299,9 @@ ProcessDeclAttribute(Sema &S, Decl *D, const ParsedAttr &AL, case ParsedAttr::AT_HLSLVkExtBuiltinOutput: S.HLSL().handleVkExtBuiltinOutputAttr(D, AL); break; + case ParsedAttr::AT_HLSLVkBuiltIn: + S.HLSL().handleVkBuiltInAttr(D, AL); + break; case ParsedAttr::AT_HLSLVkPushConstant: S.HLSL().handleVkPushConstantAttr(D, AL); break; diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp index 4fe7ed151e918..7adeaab6c88f1 100644 --- a/clang/lib/Sema/SemaHLSL.cpp +++ b/clang/lib/Sema/SemaHLSL.cpp @@ -46,6 +46,7 @@ #include "llvm/Support/DXILABI.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/FormatVariadic.h" +#include "llvm/Support/raw_ostream.h" #include "llvm/TargetParser/Triple.h" #include <algorithm> #include <cmath> @@ -983,8 +984,13 @@ void SemaHLSL::ActOnTopLevelFunction(FunctionDecl *FD) { } static bool isVkPipelineBuiltin(const ASTContext &AstContext, FunctionDecl *FD, + const DeclaratorDecl *D, HLSLAppliedSemanticAttr *Semantic, bool IsInput) { + // Explicitly annotated built-ins skip location assignment + if (D->hasAttr<HLSLVkBuiltInAttr>()) + return true; + if (AstContext.getTargetInfo().getTriple().getOS() != llvm::Triple::Vulkan) return false; @@ -1048,7 +1054,7 @@ bool SemaHLSL::determineActiveSemanticOnScalar(FunctionDecl *FD, unsigned Location = ActiveSemantic.Index.value_or(0); - if (!isVkPipelineBuiltin(getASTContext(), FD, A, + if (!isVkPipelineBuiltin(getASTContext(), FD, D, A, any(SC.CurrentIOType & IOType::In))) { bool HasVkLocation = false; if (auto *A = D->getAttr<HLSLVkLocationAttr>()) { @@ -2157,10 +2163,54 @@ void SemaHLSL::handleVkLocationAttr(Decl *D, const ParsedAttr &AL) { if (!SemaRef.checkUInt32Argument(AL, AL.getArgAsExpr(0), Location)) return; + // vk::builtin and vk::location mutually exclusive + if (const auto *BuiltIn = D->getAttr<HLSLVkBuiltInAttr>()) { + Diag(AL.getLoc(), diag::err_attributes_are_not_compatible) + << AL << BuiltIn + << (AL.isRegularKeywordAttribute() || + BuiltIn->isRegularKeywordAttribute()); + Diag(BuiltIn->getLocation(), diag::note_conflicting_attribute); + } + D->addAttr(::new (getASTContext()) HLSLVkLocationAttr(getASTContext(), AL, Location)); } +void SemaHLSL::handleVkBuiltInAttr(Decl *D, const ParsedAttr &AL) { + StringRef BuiltIn; + SourceLocation ArgLoc; + if (!SemaRef.checkStringLiteralArgumentAttr(AL, 0, BuiltIn, &ArgLoc)) + return; + + static constexpr StringRef SupportedBuiltIns[] = { + "PointSize", // TODO: uncomment build-ins as they're fully implemented + // "HelperInvocation", "BaseVertex", "BaseInstance", + // "DrawIndex", "DeviceIndex", "ViewportMaskNV", + }; + if (!llvm::is_contained(SupportedBuiltIns, BuiltIn)) { + std::string Quoted; + llvm::raw_string_ostream OS(Quoted); + llvm::interleaveComma(SupportedBuiltIns, OS, + [&](StringRef S) { OS << "'" << S << "'"; }); + Diag(ArgLoc, diag::err_hlsl_vk_builtin_unsupported_value) + << BuiltIn << Quoted; + return; + } + + // `vk::builtin` and `vk::location` are mutually exclusive. Diagnose but + // still attach -- see the matching comment in handleVkLocationAttr. + if (const auto *Location = D->getAttr<HLSLVkLocationAttr>()) { + Diag(AL.getLoc(), diag::err_attributes_are_not_compatible) + << AL << Location + << (AL.isRegularKeywordAttribute() || + Location->isRegularKeywordAttribute()); + Diag(Location->getLoc(), diag::note_conflicting_attribute); + } + + D->addAttr(::new (getASTContext()) + HLSLVkBuiltInAttr(getASTContext(), AL, BuiltIn)); +} + void SemaHLSL::handleSemanticAttr(Decl *D, const ParsedAttr &AL) { uint32_t IndexValue(0), ExplicitIndex(0); if (!SemaRef.checkUInt32Argument(AL, AL.getArgAsExpr(0), IndexValue) || diff --git a/clang/test/AST/HLSL/vk-builtin-attr.hlsl b/clang/test/AST/HLSL/vk-builtin-attr.hlsl new file mode 100644 index 0000000000000..16d42a0b13637 --- /dev/null +++ b/clang/test/AST/HLSL/vk-builtin-attr.hlsl @@ -0,0 +1,29 @@ +// RUN: %clang_cc1 -triple spirv-unknown-vulkan1.3-library -finclude-default-header -ast-dump -o - %s | FileCheck %s -check-prefixes=CHECK,SPV +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.8-library -finclude-default-header -ast-dump -o - %s | FileCheck %s -check-prefixes=CHECK,DXIL + +// vk::builtin only exists when targeting SPIR-V. + +// CHECK: CXXRecordDecl {{.*}} struct S definition +// CHECK: FieldDecl {{.*}} a 'float' +// SPV-NEXT: HLSLVkBuiltInAttr {{.*}} "PointSize" +// DXIL-NOT: HLSLVkBuiltInAttr +// CHECK: HLSLParsedSemanticAttr {{.*}} "PSIZE" +// CHECK: FieldDecl {{.*}} b 'float' +// CHECK-NEXT: HLSLVkLocationAttr {{.*}} 1 +// CHECK-NEXT: HLSLParsedSemanticAttr {{.*}} "B" +struct S { + [[vk::builtin("PointSize")]] float a : PSIZE; + [[vk::location(1)]] float b : B; +}; + +// CHECK: FunctionDecl {{.*}} param 'float (float)' +// CHECK-NEXT: ParmVarDecl {{.*}} p 'float' +// SPV-NEXT: HLSLVkBuiltInAttr {{.*}} "PointSize" +// DXIL-NOT: HLSLVkBuiltInAttr +// CHECK: HLSLParsedSemanticAttr {{.*}} "PSIZE" +float param([[vk::builtin("PointSize")]] float p : PSIZE) : A { return p; } + +// CHECK: FunctionDecl {{.*}} func 'float ()' +// SPV: HLSLVkBuiltInAttr {{.*}} "PointSize" +// DXIL-NOT: HLSLVkBuiltInAttr +[[vk::builtin("PointSize")]] float func() : PSIZE { return 1.0f; } diff --git a/clang/test/Misc/pragma-attribute-supported-attributes-list.test b/clang/test/Misc/pragma-attribute-supported-attributes-list.test index 8f8c1aa3d3f9c..05e4e86f22f9d 100644 --- a/clang/test/Misc/pragma-attribute-supported-attributes-list.test +++ b/clang/test/Misc/pragma-attribute-supported-attributes-list.test @@ -90,6 +90,7 @@ // CHECK-NEXT: GCCStruct (SubjectMatchRule_record) // CHECK-NEXT: GNUInline (SubjectMatchRule_function) // CHECK-NEXT: HIPManaged (SubjectMatchRule_variable) +// CHECK-NEXT: HLSLVkBuiltIn (SubjectMatchRule_variable_is_parameter, SubjectMatchRule_field, SubjectMatchRule_function) // CHECK-NEXT: HLSLVkLocation (SubjectMatchRule_variable_is_parameter, SubjectMatchRule_field, SubjectMatchRule_function) // CHECK-NEXT: HLSLVkPushConstant (SubjectMatchRule_variable_is_global) // CHECK-NEXT: Hot (SubjectMatchRule_function) diff --git a/clang/test/SemaHLSL/Semantics/semantic.explicit-mix-vk-builtin-control.hlsl b/clang/test/SemaHLSL/Semantics/semantic.explicit-mix-vk-builtin-control.hlsl new file mode 100644 index 0000000000000..77079ae318970 --- /dev/null +++ b/clang/test/SemaHLSL/Semantics/semantic.explicit-mix-vk-builtin-control.hlsl @@ -0,0 +1,15 @@ +// RUN: %clang_cc1 -finclude-default-header -triple spirv-unknown-vulkan1.3-vertex -hlsl-entry main1 -fsyntax-only -verify -verify-ignore-unexpected=note %s + +// Control for semantic.explicit-mix-vk-builtin-field.hlsl: without the +// vk::builtin attribute, the same struct shape is still rejected. +struct VSOutput { + [[vk::location(0)]] float4 color : COLOR; + float size : PSIZE; + // expected-error@-1 {{partial explicit stage input location assignment via vk::location(X) unsupported}} +}; + +[shader("vertex")] +VSOutput main1() { + VSOutput output = (VSOutput)0; + return output; +} diff --git a/clang/test/SemaHLSL/Semantics/semantic.explicit-mix-vk-builtin-field.hlsl b/clang/test/SemaHLSL/Semantics/semantic.explicit-mix-vk-builtin-field.hlsl new file mode 100644 index 0000000000000..5b5b8acf373b2 --- /dev/null +++ b/clang/test/SemaHLSL/Semantics/semantic.explicit-mix-vk-builtin-field.hlsl @@ -0,0 +1,18 @@ +// RUN: %clang_cc1 -finclude-default-header -triple spirv-unknown-vulkan1.3-vertex -hlsl-entry main1 -fsyntax-only -verify %s + +// vk::builtin fields are exempt from the "all or nothing" explicit-location +// check, so they can be mixed with vk::location annotated fields. +struct VSOutput { + [[vk::location(0)]] float4 color : COLOR; + [[vk::location(1)]] float2 uv : TEXCOORD0; + [[vk::builtin("PointSize")]] float size : PSIZE; + [[vk::location(2)]] float other : TEXCOORD1; +}; + +[shader("vertex")] +VSOutput main1() { + VSOutput output = (VSOutput)0; + return output; +} + +// expected-no-diagnostics diff --git a/clang/test/SemaHLSL/Semantics/semantic.explicit-mix-vk-builtin-param.hlsl b/clang/test/SemaHLSL/Semantics/semantic.explicit-mix-vk-builtin-param.hlsl new file mode 100644 index 0000000000000..0bba0908d2433 --- /dev/null +++ b/clang/test/SemaHLSL/Semantics/semantic.explicit-mix-vk-builtin-param.hlsl @@ -0,0 +1,11 @@ +// RUN: %clang_cc1 -finclude-default-header -triple spirv-unknown-vulkan1.3-pixel -hlsl-entry main1 -fsyntax-only -verify %s + +// Same as semantic.explicit-mix-vk-builtin-field.hlsl, but for a vk::builtin +// annotated entry-point parameter instead of a struct field. +[shader("pixel")] +float4 main1([[vk::location(0)]] float4 a : A, + [[vk::builtin("PointSize")]] float size : PSIZE) : SV_Target { + return a * size; +} + +// expected-no-diagnostics diff --git a/clang/test/SemaHLSL/Semantics/semantic.explicit-mix-vk-builtin-return.hlsl b/clang/test/SemaHLSL/Semantics/semantic.explicit-mix-vk-builtin-return.hlsl new file mode 100644 index 0000000000000..d6dfdf793843d --- /dev/null +++ b/clang/test/SemaHLSL/Semantics/semantic.explicit-mix-vk-builtin-return.hlsl @@ -0,0 +1,14 @@ +// RUN: %clang_cc1 -finclude-default-header -triple spirv-unknown-vulkan1.3-vertex -hlsl-entry main1 -fsyntax-only -verify %s + +// Same as semantic.explicit-mix-vk-builtin-field.hlsl, but vk::builtin +// annotates the entry point's return value (the Function subject), mixed +// with an explicit output location on an `out` parameter. +[[vk::builtin("PointSize")]] +[shader("vertex")] +float main1( + [[vk::location(0)]] float4 a : A, [[vk::location(1)]] out float4 b : B) : PSIZE { + b = a; + return a.x; +} + +// expected-no-diagnostics diff --git a/clang/test/SemaHLSL/vk-builtin-attr-non-spirv.hlsl b/clang/test/SemaHLSL/vk-builtin-attr-non-spirv.hlsl new file mode 100644 index 0000000000000..51a3536891401 --- /dev/null +++ b/clang/test/SemaHLSL/vk-builtin-attr-non-spirv.hlsl @@ -0,0 +1,16 @@ +// RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.8-library -verify %s + +// vk::builtin only exists when targeting SPIR-V; elsewhere it is unknown. + +struct S { + // expected-warning@+1 {{unknown attribute 'vk::builtin' ignored}} + [[vk::builtin("PointSize")]] float size : PSIZE; +}; + +// expected-warning@+1 {{unknown attribute 'vk::builtin' ignored}} +float param([[vk::builtin("PointSize")]] float size : PSIZE) : A { + return size; +} + +// expected-warning@+1 {{unknown attribute 'vk::builtin' ignored}} +[[vk::builtin("PointSize")]] float func() : PSIZE { return 1.0f; } diff --git a/clang/test/SemaHLSL/vk-builtin-attr.hlsl b/clang/test/SemaHLSL/vk-builtin-attr.hlsl new file mode 100644 index 0000000000000..a330bf19e7a71 --- /dev/null +++ b/clang/test/SemaHLSL/vk-builtin-attr.hlsl @@ -0,0 +1,67 @@ +// RUN: %clang_cc1 -finclude-default-header -triple spirv-pc-vulkan1.3-compute -verify %s + +// Valid subjects: field, parameter, function. +struct Valid { + [[vk::builtin("PointSize")]] float size : PSIZE; +}; + +float param([[vk::builtin("PointSize")]] float size : PSIZE) : A { return size; } + +[[vk::builtin("PointSize")]] float func() : PSIZE { return 1.0f; } + +// Invalid subjects. +// expected-error@+1 {{'vk::builtin' attribute only applies to parameters, non-static data members, and functions}} +[[vk::builtin("PointSize")]] static float global_var; + +// expected-error@+1 {{'vk::builtin' attribute only applies to parameters, non-static data members, and functions}} +[[vk::builtin("PointSize")]] cbuffer InvalidCBuffer { + float cb; +} + +// Unsupported built-in name. +struct BadName { + // expected-error@+1 {{'Bogus' is not a supported value for the 'vk::builtin' attribute; supported values are: 'PointSize'}} + [[vk::builtin("Bogus")]] float a : A; + // The name is case sensitive. + // expected-error@+1 {{'pointsize' is not a supported value for the 'vk::builtin' attribute; supported values are: 'PointSize'}} + [[vk::builtin("pointsize")]] float b : B; + // expected-error@+1 {{'' is not a supported value for the 'vk::builtin' attribute; supported values are: 'PointSize'}} + [[vk::builtin("")]] float c : C; +}; + +// Wrong argument count and type. +struct BadArgs { + // expected-error@+1 {{'vk::builtin' attribute takes one argument}} + [[vk::builtin()]] float a : A; + // expected-error@+1 {{'vk::builtin' attribute takes one argument}} + [[vk::builtin("PointSize", "PointSize")]] float b : B; + // expected-error@+1 {{expected string literal as argument of 'builtin' attribute}} + [[vk::builtin(5)]] float c : C; +}; + +// Templates: the argument is a string literal, so it is validated when the +// template is declared, not when it is instantiated. +template <typename T> struct Templated { + // expected-error@+1 {{'Bogus' is not a supported value for the 'vk::builtin' attribute; supported values are: 'PointSize'}} + [[vk::builtin("Bogus")]] T a : A; + [[vk::builtin("PointSize")]] T b : PSIZE; +}; +Templated<float> TemplatedInstance; + +// Combining `vk::builtin` and `vk::location` is rejected, whatever the order. +struct Combined { + // expected-error@+2 {{'vk::location' and 'vk::builtin' attributes are not compatible}} + // expected-note@+1 {{conflicting attribute is here}} + [[vk::builtin("PointSize")]] [[vk::location(1)]] float a : A; + // expected-error@+2 {{'vk::builtin' and 'vk::location' attributes are not compatible}} + // expected-note@+1 {{conflicting attribute is here}} + [[vk::location(2)]] [[vk::builtin("PointSize")]] float b : B; +}; + +// expected-error@+2 {{'vk::location' and 'vk::builtin' attributes are not compatible}} +// expected-note@+1 {{conflicting attribute is here}} +[[vk::builtin("PointSize")]] [[vk::location(0)]] float func_combined() : A; + +[shader("compute")] +[numthreads(1, 1, 1)] +void main() {} >From a916aca7aeff687886147601e620afc806bbaa1f Mon Sep 17 00:00:00 2001 From: Jonathan Zakharov <[email protected]> Date: Mon, 12 Oct 2026 11:13:10 -0700 Subject: [PATCH 2/3] [HLSL][SPIRV] Implement vk::builtin attribute --- clang/lib/Sema/SemaHLSL.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp index 7adeaab6c88f1..214c659b992bb 100644 --- a/clang/lib/Sema/SemaHLSL.cpp +++ b/clang/lib/Sema/SemaHLSL.cpp @@ -2163,7 +2163,15 @@ void SemaHLSL::handleVkLocationAttr(Decl *D, const ParsedAttr &AL) { if (!SemaRef.checkUInt32Argument(AL, AL.getArgAsExpr(0), Location)) return; +<<<<<<< HEAD // vk::builtin and vk::location mutually exclusive +======= + // `vk::builtin` and `vk::location` are mutually exclusive. Diagnose but + // still attach: DXC accumulates diagnostics rather than short-circuiting + // (a single decl can simultaneously violate this, a builtin's required + // type, and its legal stage), so later checks must still be able to see + // this attribute rather than silently never running. +>>>>>>> 9ecd11ae9b5d ([HLSL][SPIRV] Implement vk::builtin attribute) if (const auto *BuiltIn = D->getAttr<HLSLVkBuiltInAttr>()) { Diag(AL.getLoc(), diag::err_attributes_are_not_compatible) << AL << BuiltIn >From ad9490a1f6e1f5c8fa77e54e8e8b005c5ee91077 Mon Sep 17 00:00:00 2001 From: Jonathan Zakharov <[email protected]> Date: Mon, 12 Oct 2026 11:26:51 -0700 Subject: [PATCH 3/3] [HLSL][SPIRV] Implement vk::builtin(PointSize) validation and CodeGen --- clang/include/clang/Basic/AttrDocs.td | 15 +++++- .../clang/Basic/DiagnosticSemaKinds.td | 4 ++ clang/lib/CodeGen/CGHLSLRuntime.cpp | 29 +++++++++++ clang/lib/Sema/SemaHLSL.cpp | 50 ++++++++++++++++++- .../vk-builtin-point-size-return.hlsl | 18 +++++++ .../CodeGenHLSL/vk-builtin-point-size.hlsl | 33 ++++++++++++ .../vk-builtin-point-size-inout-pixel.hlsl | 14 ++++++ .../vk-builtin-point-size-inout-vertex.hlsl | 14 ++++++ .../vk-builtin-point-size-invalid.hlsl | 12 +++++ .../test/SemaHLSL/vk-builtin-point-size.hlsl | 12 +++++ 10 files changed, 197 insertions(+), 4 deletions(-) create mode 100644 clang/test/CodeGenHLSL/vk-builtin-point-size-return.hlsl create mode 100644 clang/test/CodeGenHLSL/vk-builtin-point-size.hlsl create mode 100644 clang/test/SemaHLSL/vk-builtin-point-size-inout-pixel.hlsl create mode 100644 clang/test/SemaHLSL/vk-builtin-point-size-inout-vertex.hlsl create mode 100644 clang/test/SemaHLSL/vk-builtin-point-size-invalid.hlsl create mode 100644 clang/test/SemaHLSL/vk-builtin-point-size.hlsl diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index bc830670689e0..6a3d4a1ce01cf 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -9755,11 +9755,22 @@ The attribute is only available when targeting SPIR-V. For any other target it is ignored with an "unknown attribute ignored" warning. The attribute takes a single string literal argument naming the built-in. -Supported built-in names are added incrementally, each landing with its own -type/stage validation and CodeGen support. +Currently supported built-ins: + +* `PointSize`: the GLSL equivalent is `gl_PointSize`. Must be of floating-point + type. Can be used as a vertex shader output, a hull or domain shader control + point input or output, a geometry shader input or output, a pixel shader + input, or a mesh shader output. A declaration annotated with `[[vk::builtin]]` does not take part in `vk::location` assignment, and cannot also be annotated with `[[vk::location]]`. + +```c++ +struct VSOutput { + [[vk::location(0)]] float4 color : COLOR; + [[vk::builtin("PointSize")]] float size : PSIZE; +}; +``` }]; } diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 50de8f772ace2..12b66e5462c6e 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -13791,6 +13791,10 @@ def err_hlsl_semantic_partial_explicit_indexing def err_hlsl_vk_builtin_unsupported_value : Error<"'%0' is not a supported value for the 'vk::builtin' attribute; " "supported values are: %1">; +def err_hlsl_vk_builtin_wrong_type + : Error<"'%0' builtin must be of type %1">; +def err_hlsl_vk_builtin_wrong_stage + : Error<"'%0' builtin cannot be used as %1 shader %2">; def warn_hlsl_user_defined_type_missing_member: Warning<"binding type '%select{t|u|b|s|c}0' only applies to types containing %select{SRV resources|UAV resources|constant buffer resources|sampler state|numeric types}0">, InGroup<LegacyConstantRegisterBinding>; def err_hlsl_binding_type_mismatch: Error<"binding type '%select{t|u|b|s|c}0' only applies to %select{SRV resources|UAV resources|constant buffer resources|sampler state|numeric variables in the global scope}0">; diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp index f770981935952..8b3c5fbf42c3d 100644 --- a/clang/lib/CodeGen/CGHLSLRuntime.cpp +++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp @@ -36,6 +36,7 @@ #include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringSwitch.h" #include "llvm/Frontend/HLSL/HLSLResource.h" #include "llvm/Frontend/HLSL/RootSignatureMetadata.h" #include "llvm/IR/Constants.h" @@ -1570,6 +1571,18 @@ static void createSPIRVBuiltinStore(IRBuilder<> &B, llvm::Module &M, B.CreateStore(Source, GV); } +// Maps a [[vk::builtin(...)]] name to its SPIR-V spec `BuiltIn` decoration. +// Deliberately not creating dependency on llvm::SPIRV::BuiltIn in front end. +// Sema's handleVkBuiltInAttr only allows names handled here to reach CodeGen. +static unsigned getSPIRVVkBuiltinID(StringRef BuiltIn) { + unsigned ID = llvm::StringSwitch<unsigned>(BuiltIn) + .Case("PointSize", /* BuiltIn::PointSize */ 1) + .Default(-1); + assert(ID != static_cast<unsigned>(-1) && + "Sema should only allow builtin names implemented here"); + return ID; +} + void CGHLSLRuntime::emitSystemSemanticStore( IRBuilder<> &B, llvm::Value *Source, const clang::DeclaratorDecl *Decl, HLSLAppliedSemanticAttr *Semantic, @@ -1618,6 +1631,14 @@ llvm::Value *CGHLSLRuntime::handleScalarSemanticLoad( const clang::DeclaratorDecl *Decl, HLSLAppliedSemanticAttr *Semantic, SemanticSignatures &Signature) { + // [[vk::builtin(...)]] takes precedence over the recognized-semantic + // dispatch, so without this check they'd always be treated as an arbitrary + // user semantic. The attribute is only ever attached when targeting SPIR-V. + if (const auto *BuiltInAttr = Decl->getAttr<HLSLVkBuiltInAttr>()) + return createSPIRVBuiltinLoad( + B, CGM.getModule(), Type, Semantic->getAttrName()->getName(), + getSPIRVVkBuiltinID(BuiltInAttr->getBuiltIn())); + std::optional<unsigned> Index = Semantic->getSemanticIndex(); llvm::dxbc::PSV::SemanticKind SemanticKind = llvm::hlsl::getSemanticKind(Semantic->getAttrName()->getName()); @@ -1640,6 +1661,14 @@ void CGHLSLRuntime::handleScalarSemanticStore(IRBuilder<> &B, const clang::DeclaratorDecl *Decl, HLSLAppliedSemanticAttr *Semantic, SemanticSignatures &Signature) { + // See the matching comment in `handleScalarSemanticLoad` + if (const auto *BuiltInAttr = Decl->getAttr<HLSLVkBuiltInAttr>()) { + createSPIRVBuiltinStore(B, CGM.getModule(), Source, + Semantic->getAttrName()->getName(), + getSPIRVVkBuiltinID(BuiltInAttr->getBuiltIn())); + return; + } + std::optional<unsigned> Index = Semantic->getSemanticIndex(); llvm::dxbc::PSV::SemanticKind SemanticKind = llvm::hlsl::getSemanticKind(Semantic->getAttrName()->getName()); diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp index 214c659b992bb..0a8c3d9ee79c6 100644 --- a/clang/lib/Sema/SemaHLSL.cpp +++ b/clang/lib/Sema/SemaHLSL.cpp @@ -1015,6 +1015,46 @@ static bool isVkPipelineBuiltin(const ASTContext &AstContext, FunctionDecl *FD, } } +static void validateVkBuiltinUsage(Sema &S, FunctionDecl *FD, + const DeclaratorDecl *D, + const HLSLVkBuiltInAttr *Attr, + IOType CurrentIOType) { + const ASTContext &AstContext = S.getASTContext(); + if (AstContext.getTargetInfo().getTriple().getOS() != llvm::Triple::Vulkan) + return; + + StringRef BuiltIn = Attr->getBuiltIn(); + + QualType T = + (D == FD ? FD->getReturnType() : D->getType()).getNonReferenceType(); + const auto *ShaderAttr = FD->getAttr<HLSLShaderAttr>(); + assert(ShaderAttr && "Entry point has no shader attribute"); + llvm::Triple::EnvironmentType Stage = ShaderAttr->getType(); + bool IsIn = any(CurrentIOType & IOType::In); + bool IsOut = any(CurrentIOType & IOType::Out); + + if (BuiltIn == "PointSize") { + if (!T->isFloatingType()) + S.Diag(Attr->getLocation(), diag::err_hlsl_vk_builtin_wrong_type) + << BuiltIn << S.getASTContext().FloatTy; + + bool LegalStage = (Stage == llvm::Triple::Vertex && IsOut) || + Stage == llvm::Triple::Hull || + Stage == llvm::Triple::Domain || + Stage == llvm::Triple::Geometry || + (Stage == llvm::Triple::Pixel && IsIn) || + (Stage == llvm::Triple::Mesh && IsOut); + if (!LegalStage) { + StringRef IOTypeName = (IsIn && IsOut) ? "inputs and outputs" + : IsIn ? "inputs" + : "outputs"; + S.Diag(Attr->getLocation(), diag::err_hlsl_vk_builtin_wrong_stage) + << BuiltIn << llvm::Triple::getEnvironmentTypeName(Stage) + << IOTypeName; + } + } +} + bool SemaHLSL::determineActiveSemanticOnScalar(FunctionDecl *FD, DeclaratorDecl *OutputDecl, DeclaratorDecl *D, @@ -1054,8 +1094,14 @@ bool SemaHLSL::determineActiveSemanticOnScalar(FunctionDecl *FD, unsigned Location = ActiveSemantic.Index.value_or(0); - if (!isVkPipelineBuiltin(getASTContext(), FD, D, A, - any(SC.CurrentIOType & IOType::In))) { + bool IsBuiltin = isVkPipelineBuiltin(getASTContext(), FD, D, A, + any(SC.CurrentIOType & IOType::In)); + if (IsBuiltin) { + if (const auto *BuiltInAttr = D->getAttr<HLSLVkBuiltInAttr>()) + validateVkBuiltinUsage(SemaRef, FD, D, BuiltInAttr, SC.CurrentIOType); + } + + if (!IsBuiltin) { bool HasVkLocation = false; if (auto *A = D->getAttr<HLSLVkLocationAttr>()) { HasVkLocation = true; diff --git a/clang/test/CodeGenHLSL/vk-builtin-point-size-return.hlsl b/clang/test/CodeGenHLSL/vk-builtin-point-size-return.hlsl new file mode 100644 index 0000000000000..2e999f3226b31 --- /dev/null +++ b/clang/test/CodeGenHLSL/vk-builtin-point-size-return.hlsl @@ -0,0 +1,18 @@ +// RUN: %clang_cc1 -finclude-default-header -triple spirv-pc-vulkan1.3-vertex -hlsl-entry main -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s + +// Same as vk-builtin-point-size.hlsl, but [[vk::builtin("PointSize")]] +// annotates the entry point's return value (the Function subject) rather +// than a struct field. + +[[vk::builtin("PointSize")]] +[shader("vertex")] +float main([[vk::location(0)]] float4 a : A) : PSIZE { + return a.x; +} + +// CHECK: @PSIZE = {{.*}} addrspace(8) global float, !spirv.Decorations [[PSIZE_DECO:![0-9]+]] +// CHECK: define void @main() +// CHECK: [[PT_SIZE:%[0-9]+]] = call spir_func float @{{.*}}main{{.*}} +// CHECK-NEXT: store float [[PT_SIZE]], ptr addrspace(8) @PSIZE +// CHECK-DAG: [[PSIZE_DECO]] = !{[[PSIZE_OP:![0-9]+]]} +// CHECK-DAG: [[PSIZE_OP]] = !{i32 11, i32 1} diff --git a/clang/test/CodeGenHLSL/vk-builtin-point-size.hlsl b/clang/test/CodeGenHLSL/vk-builtin-point-size.hlsl new file mode 100644 index 0000000000000..a1f42192c9b91 --- /dev/null +++ b/clang/test/CodeGenHLSL/vk-builtin-point-size.hlsl @@ -0,0 +1,33 @@ +// RUN: %clang_cc1 -finclude-default-header -triple spirv-pc-vulkan1.3-vertex -hlsl-entry main -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s +// RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.8-vertex -hlsl-entry main -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s --check-prefix=DXIL + +// On SPIR-V, [[vk::builtin("PointSize")]] lowers to a BuiltIn PointSize +// decoration instead of an ordinary Location-based user semantic. The +// attribute doesn't exist on DXIL, so there PSIZE is an ordinary semantic. + +struct VSOutput { + float4 pos : SV_Position; + float4 color : COLOR; + [[vk::builtin("PointSize")]] float size : PSIZE; +}; + +[shader("vertex")] +VSOutput main() { + VSOutput output = (VSOutput)0; + return output; +} + +// CHECK: @COLOR0 = {{.*}}!spirv.Decorations ![[COLOR_MD:[0-9]+]] +// CHECK: @PSIZE = {{.*}}addrspace(8){{.*}}!spirv.Decorations ![[PSIZE_MD:[0-9]+]] +// CHECK: define void @main() +// CHECK: store float %{{.*}}, ptr addrspace(8) @PSIZE + +// BuiltIn decoration (11) with value 1 (PointSize), vs. COLOR0's Location (30). +// CHECK: ![[COLOR_MD]] = !{![[COLOR_DEC:[0-9]+]]} +// CHECK: ![[COLOR_DEC]] = !{i32 30, +// CHECK: ![[PSIZE_MD]] = !{![[PSIZE_DEC:[0-9]+]]} +// CHECK: ![[PSIZE_DEC]] = !{i32 11, i32 1} + +// DXIL: define void @main() +// DXIL-NOT: PointSize +// DXIL: !{{[0-9]+}} = !{i32 {{[0-9]+}}, !"PSIZE", diff --git a/clang/test/SemaHLSL/vk-builtin-point-size-inout-pixel.hlsl b/clang/test/SemaHLSL/vk-builtin-point-size-inout-pixel.hlsl new file mode 100644 index 0000000000000..3fc454105e90d --- /dev/null +++ b/clang/test/SemaHLSL/vk-builtin-point-size-inout-pixel.hlsl @@ -0,0 +1,14 @@ +// RUN: %clang_cc1 -finclude-default-header -triple spirv-pc-vulkan1.3-pixel -hlsl-entry ps -fsyntax-only -verify %s + +// PointSize is only legal as a pixel shader input, never an output. An +// `inout` parameter is currently treated as output-only here, so this is +// rejected (see the sibling vk-builtin-point-size-inout-vertex.hlsl, where +// `inout` on a stage where PointSize is a legal output is accepted). + +[shader("pixel")] +float4 ps([[vk::builtin("PointSize")]] inout float size : PSIZE) : SV_Target { + // expected-error@-1 {{semantic 'PSIZE' is not supported in pixel shader outputs}} + // expected-error@-2 {{'PointSize' builtin cannot be used as pixel shader outputs}} + size *= 2.0; + return float4(size, 0, 0, 1); +} diff --git a/clang/test/SemaHLSL/vk-builtin-point-size-inout-vertex.hlsl b/clang/test/SemaHLSL/vk-builtin-point-size-inout-vertex.hlsl new file mode 100644 index 0000000000000..f0a541c48e829 --- /dev/null +++ b/clang/test/SemaHLSL/vk-builtin-point-size-inout-vertex.hlsl @@ -0,0 +1,14 @@ +// RUN: %clang_cc1 -finclude-default-header -triple spirv-pc-vulkan1.3-vertex -hlsl-entry vs -fsyntax-only -verify %s + +// An `inout` PointSize parameter on a vertex shader is accepted, same as +// plain `out` (see the sibling vk-builtin-point-size-inout-pixel.hlsl for a +// stage where `inout`'s current treatment as output-only changes the +// outcome). + +[shader("vertex")] +float4 vs([[vk::builtin("PointSize")]] inout float size : PSIZE) : SV_Position { + size *= 2.0; + return float4(0, 0, 0, 1); +} + +// expected-no-diagnostics diff --git a/clang/test/SemaHLSL/vk-builtin-point-size-invalid.hlsl b/clang/test/SemaHLSL/vk-builtin-point-size-invalid.hlsl new file mode 100644 index 0000000000000..cc7d5cbd1b485 --- /dev/null +++ b/clang/test/SemaHLSL/vk-builtin-point-size-invalid.hlsl @@ -0,0 +1,12 @@ +// RUN: %clang_cc1 -finclude-default-header -triple spirv-pc-vulkan1.3-vertex -hlsl-entry main -fsyntax-only -verify %s + +// A single decl can violate multiple PointSize rules at once -- combined +// with vk::location, wrong type, and wrong stage -- and all diagnostics +// fire together rather than short-circuiting on the first. +float main([[vk::location(5), vk::builtin("PointSize")]] int input : A) : B { + // expected-error@-1 {{'vk::builtin' and 'vk::location' attributes are not compatible}} + // expected-note@-2 {{conflicting attribute is here}} + // expected-error@-3 {{'PointSize' builtin must be of type 'float'}} + // expected-error@-4 {{'PointSize' builtin cannot be used as vertex shader inputs}} + return (float)input; +} diff --git a/clang/test/SemaHLSL/vk-builtin-point-size.hlsl b/clang/test/SemaHLSL/vk-builtin-point-size.hlsl new file mode 100644 index 0000000000000..0fcf7130ddd2a --- /dev/null +++ b/clang/test/SemaHLSL/vk-builtin-point-size.hlsl @@ -0,0 +1,12 @@ +// RUN: %clang_cc1 -finclude-default-header -triple spirv-pc-vulkan1.3-vertex -hlsl-entry legal -fsyntax-only -verify %s + +// PointSize must be a float, and is only legal as a vertex shader output +// (plus other stages not exercised here). +[shader("vertex")] +float legal([[vk::location(0)]] float4 a : A, + [[vk::builtin("PointSize")]] out float size : PSIZE) : B { + size = a.x; + return a.y; +} + +// expected-no-diagnostics _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
