llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-hlsl Author: Divyansh Yadav (schizophrenicmaniac) <details> <summary>Changes</summary> Variable templates are part of HLSL 202x (proposal 0010), but until now HLSL always hit the C++14 compat diagnostic for them. This adds a "variable templates are a HLSL 202x extension" warning under a new `-Whlsl-202x-extensions` group (part of `-Whlsl-extensions`). It fires only for HLSL 2021 and earlier, the same way the 202y `auto` warning is handled. Testing it showed that instantiated specializations never went through `SemaHLSL::ActOnVariableDeclarator`, so they skipped the usual handling for HLSL globals. A `static` variable template ended up in the default address space, so on SPIR-V the global was in addrspace(0) instead of addrspace(10). A non-static one became a plain mutable global instead of a `$Globals` constant. Meanwhile the template pattern itself was added to `$Globals` when its type didn't depend on the template parameters. Now: - instantiated specializations go through `ActOnVariableDeclarator` right after their attributes are instantiated, as normal declarations do; - the pattern and partial specializations are skipped there; - `CompleteVarTemplateSpecializationDecl` keeps the address space when it re-substitutes the type for the definition; - an explicit specialization of a `static` variable template is treated as static (explicit specializations can't spell a storage class themselves). Open question: I made non-static variable template specializations behave like any other non-static global, so each one used becomes a member of `$Globals`. If the proposal intends something else for those, such as requiring `static` or treating them differently, I'm happy to change it. Fixes #<!-- -->235123 --- Full diff: https://github.com/llvm/llvm-project/pull/235273.diff 7 Files Affected: - (modified) clang/include/clang/Basic/DiagnosticGroups.td (+2-1) - (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+2) - (modified) clang/lib/Sema/SemaDecl.cpp (+5-2) - (modified) clang/lib/Sema/SemaHLSL.cpp (+13) - (modified) clang/lib/Sema/SemaTemplateInstantiateDecl.cpp (+11-2) - (added) clang/test/CodeGenHLSL/variable-templates.hlsl (+51) - (added) clang/test/SemaHLSL/Language/VariableTemplates.hlsl (+39) ``````````diff diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index b3bb2fb1b53fd..6f900d26e3a9b 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -1974,10 +1974,11 @@ def PedanticMacros : DiagGroup<"pedantic-macros", def BranchProtection : DiagGroup<"branch-protection">; // HLSL diagnostic groups +def HLSL202x : DiagGroup<"hlsl-202x-extensions">; def HLSL202y : DiagGroup<"hlsl-202y-extensions">; // Warnings for HLSL Clang extensions -def HLSLExtension : DiagGroup<"hlsl-extensions", [HLSL202y]>; +def HLSLExtension : DiagGroup<"hlsl-extensions", [HLSL202x, HLSL202y]>; // Warning for mix packoffset and non-packoffset. def HLSLMixPackOffset : DiagGroup<"mix-packoffset">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index dcb5732c3c19e..a2438a2b78718 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -13855,6 +13855,8 @@ def err_hlsl_rootsignature_entry: Error< def err_hlsl_operator_unsupported : Error< "the '%select{&|*|->}0' operator is unsupported in HLSL">; +def ext_hlsl_variable_template : ExtWarn< + "variable templates are a HLSL 202x extension">, InGroup<HLSL202x>; def warn_hlsl_groupshared_202x : Warning< "support for groupshared parameter annotation not added until HLSL 202x">, InGroup<HLSLAvailability>; diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index fba96fe7abb2b..46a454e6e8bb9 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -8015,8 +8015,11 @@ NamedDecl *Sema::ActOnVariableDeclarator( // This is a template declaration. IsVariableTemplate = true; - // Only C++1y supports variable templates (N3651). - DiagCompat(D.getIdentifierLoc(), diag_compat::variable_template); + // Only C++1y and HLSL 202x support variable templates (N3651). + if (!getLangOpts().HLSL) + DiagCompat(D.getIdentifierLoc(), diag_compat::variable_template); + else if (getLangOpts().getHLSLVersion() < LangOptions::HLSL_202x) + Diag(D.getIdentifierLoc(), diag::ext_hlsl_variable_template); } } } else { diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp index f3fd1fb68c072..f119edb6ebdc4 100644 --- a/clang/lib/Sema/SemaHLSL.cpp +++ b/clang/lib/Sema/SemaHLSL.cpp @@ -6238,6 +6238,19 @@ void SemaHLSL::handleGlobalStructOrArrayOfWithResources(VarDecl *VD) { } void SemaHLSL::ActOnVariableDeclarator(VarDecl *VD) { + // Variable templates are handled when their specializations are + // instantiated. + if (VD->getDescribedVarTemplate() || + isa<VarTemplatePartialSpecializationDecl>(VD)) + return; + + // An explicit specialization cannot have a storage class of its own, so it is + // static if the variable template it specializes is. + if (auto *VTSD = dyn_cast<VarTemplateSpecializationDecl>(VD)) + if (VTSD->getSpecializedTemplate()->getTemplatedDecl()->getStorageClass() == + SC_Static) + VD->setStorageClass(SC_Static); + if (VD->hasGlobalStorage()) { // make sure the declaration has a complete type if (SemaRef.RequireCompleteType( diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp index b4881d0c6db5c..6f1b3e37ef286 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -6405,8 +6405,13 @@ VarTemplateSpecializationDecl *Sema::CompleteVarTemplateSpecializationDecl( if (!TSI) return nullptr; - // Update the type of this variable template specialization. - VarSpec->setType(TSI->getType()); + // Update the type of this variable template specialization. HLSL assigns + // the address space when the specialization is declared, so keep it. + QualType T = TSI->getType(); + if (getLangOpts().HLSL && VarSpec->getType().hasAddressSpace() && + !T.hasAddressSpace()) + T = Context.getAddrSpaceQualType(T, VarSpec->getType().getAddressSpace()); + VarSpec->setType(T); // Convert the declaration into a definition now. VarSpec->setCompleteDefinition(); @@ -6467,6 +6472,10 @@ void Sema::BuildVariableInstantiation( InstantiateAttrs(TemplateArgs, OldVar, NewVar, LateAttrs, StartingScope); + if (getLangOpts().HLSL && isa<VarTemplateSpecializationDecl>(NewVar) && + !isa<VarTemplatePartialSpecializationDecl>(NewVar)) + HLSL().ActOnVariableDeclarator(NewVar); + LookupResult Previous( *this, NewVar->getDeclName(), NewVar->getLocation(), NewVar->isLocalExternDecl() ? Sema::LookupRedeclarationWithLinkage diff --git a/clang/test/CodeGenHLSL/variable-templates.hlsl b/clang/test/CodeGenHLSL/variable-templates.hlsl new file mode 100644 index 0000000000000..6bb81c035b75c --- /dev/null +++ b/clang/test/CodeGenHLSL/variable-templates.hlsl @@ -0,0 +1,51 @@ +// RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.3-library -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s --check-prefixes=CHECK,DXIL +// RUN: %clang_cc1 -finclude-default-header -triple spirv-unknown-vulkan1.3-library -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s --check-prefixes=CHECK,SPIRV + +template <typename T> static T Counter = T(1); +template <> int Counter<int> = 7; + +template <typename T> static const T Scale[2] = {T(2), T(3)}; + +template <typename T> T Global; +template <typename T> uint Index; + +template <typename T> groupshared T Shared[4]; + +struct S { + template <typename T> static T Member; +}; +template <typename T> T S::Member = T(5); + +// DXIL-DAG: @_ZL7CounterIfE = internal global float 1.000000e+00, align 4 +// DXIL-DAG: @_ZL7CounterIiE = internal global i32 7, align 4 +// DXIL-DAG: @_ZL5ScaleIfE = internal constant [2 x float] [float 2.000000e+00, float 3.000000e+00], align 4 +// DXIL-DAG: @_Z6GlobalIfE = external hidden addrspace(2) global float, align 4 +// DXIL-DAG: @_Z5IndexIfE = external hidden addrspace(2) global i32, align 4 +// DXIL-DAG: @_Z6SharedIfE = external hidden addrspace(3) global [4 x float], align 4 +// DXIL-DAG: @_ZN1S6MemberIfEE = linkonce_odr hidden global float 5.000000e+00, align 4 + +// SPIRV-DAG: @_ZL7CounterIfE = internal addrspace(10) global float 1.000000e+00, align 4 +// SPIRV-DAG: @_ZL7CounterIiE = internal addrspace(10) global i32 7, align 4 +// SPIRV-DAG: @_ZL5ScaleIfE = internal addrspace(10) constant [2 x float] [float 2.000000e+00, float 3.000000e+00], align 4 +// SPIRV-DAG: @_Z6GlobalIfE = external hidden addrspace(12) global float, align 4 +// SPIRV-DAG: @_Z5IndexIfE = external hidden addrspace(12) global i32, align 4 +// SPIRV-DAG: @_Z6SharedIfE = external hidden addrspace(3) global [4 x float], align 4 +// SPIRV-DAG: @_ZN1S6MemberIfEE = linkonce_odr hidden addrspace(10) global float 5.000000e+00, comdat, align 4 + +// CHECK-LABEL: define {{.*}}float @_Z2fnj( +export float fn(uint I) { +// DXIL: store float {{.*}}, ptr @_ZL7CounterIfE, align 4 +// SPIRV: store float {{.*}}, ptr addrspace(10) @_ZL7CounterIfE, align 4 + Counter<float> += 1.0; +// DXIL: load float, ptr addrspace(2) @_Z6GlobalIfE, align 4 +// SPIRV: load float, ptr addrspace(12) @_Z6GlobalIfE, align 4 + Shared<float>[I] = Global<float>; +// DXIL: load i32, ptr @_ZL7CounterIiE, align 4 +// SPIRV: load i32, ptr addrspace(10) @_ZL7CounterIiE, align 4 + return Counter<float> + Counter<int> + Scale<float>[I] + Shared<float>[I] + + S::Member<float> + Index<float>; +} + +// CHECK: !hlsl.cbs = !{![[CB:.*]]} +// DXIL: ![[CB]] = !{ptr @"$Globals.cb", ptr addrspace(2) @_Z6GlobalIfE, ptr addrspace(2) @_Z5IndexIfE} +// SPIRV: ![[CB]] = !{ptr @"$Globals.cb", ptr addrspace(12) @_Z6GlobalIfE, ptr addrspace(12) @_Z5IndexIfE} diff --git a/clang/test/SemaHLSL/Language/VariableTemplates.hlsl b/clang/test/SemaHLSL/Language/VariableTemplates.hlsl new file mode 100644 index 0000000000000..89c370dfc7709 --- /dev/null +++ b/clang/test/SemaHLSL/Language/VariableTemplates.hlsl @@ -0,0 +1,39 @@ +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -finclude-default-header -std=hlsl2021 -fsyntax-only -verify %s +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -finclude-default-header -std=hlsl202x -fsyntax-only -verify %s +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -finclude-default-header -std=hlsl202y -fsyntax-only -verify %s + +#if __HLSL_VERSION < 2028 +// expected-warning@*{{support for HLSL language version hlsl2021 is incomplete, recommend using hlsl202x instead}} +// expected-warning@#pi{{variable templates are a HLSL 202x extension}} +// expected-warning@#zero{{variable templates are a HLSL 202x extension}} +// expected-warning@#arr{{variable templates are a HLSL 202x extension}} +// expected-warning@#max{{variable templates are a HLSL 202x extension}} +#endif + +template <typename T> static const T Pi = T(3.14159265); // #pi +template <> const int Pi<int> = 3; + +template <typename T> static T Zero = T(0); // #zero + +template <typename T, uint N> static const T Arr[N] = {T(1), T(2)}; // #arr + +struct S { + template <typename T> static const T Max = T(100); // #max +}; + +_Static_assert(Pi<int> == 3, "explicit specialization"); +_Static_assert(Pi<uint> == 3, "implicit instantiation"); +_Static_assert(S::Max<int> == 100, "static data member template"); + +template <typename T> T twice() { return Pi<T> * 2; } + +export float fn(uint I) { + Zero<float> += 1.0; + return twice<float>() + Zero<float> + Arr<float, 2>[I]; +} + +export float bad() { + // expected-error@+1 {{use of variable template 'Pi' requires template arguments}} + return Pi; + // expected-note@#pi {{template is declared here}} +} `````````` </details> https://github.com/llvm/llvm-project/pull/235273 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
