Enable template dependent globallycoherent types (#4718)

This change alters how globallycoherent attributes are validated. We
will need similar changes for other HLSL type attributes as well.

Without this change, globallycoherent attributes are only allowed to
apply to declarations of builtin objects during parsing. Additionally
if applied to a non-UAV we catch that late (during code generation) and
report a different error there.

This patch reworks all of that. Instead, during parsing we allow verify
the attribute is applying to a UAV builtin type and issue a diagnostic
if applied to a non-UAV type. We also allow the attribute to be applied
to any dependent type.

If the attribute is applied to a dependent type, we verify that the
dependent type is resolved to a valid UAV type during instantation.

This change also reworks the existing test case for globallycoherent
diagnostics and deletes duplicated tests that were not adding meanigful
value.

Fixes 4583.

tools/clang/include/clang/Sema/Sema.h

@@ -3793,6 +3793,7 @@ public:
   // HLSL Change Begins
   bool CheckHLSLUnaryExprOrTypeTraitOperand(QualType ExprType, SourceLocation Loc,
                                             UnaryExprOrTypeTrait ExprKind);
+  void DiagnoseHLSLDeclAttr(const Decl *D, const Attr *A);
   // HLSL Change Ends
 
   bool CheckUnaryExprOrTypeTraitOperand(Expr *E, UnaryExprOrTypeTrait ExprKind);

tools/clang/lib/CodeGen/CGHLSLMS.cpp

@@ -3312,15 +3312,6 @@ bool CGMSHLSLRuntime::SetUAVSRV(SourceLocation loc,
     hlslRes->SetGloballyCoherent(true);
   }
   if (resClass == hlsl::DxilResourceBase::Class::SRV) {
-    if (hlslRes->IsGloballyCoherent()) {
-      DiagnosticsEngine &Diags = CGM.getDiags();
-      unsigned DiagID = Diags.getCustomDiagID(
-          DiagnosticsEngine::Error, "globallycoherent can only be used with "
-                                    "Unordered Access View buffers.");
-      Diags.Report(loc, DiagID);
-      return false;
-    }
-
     hlslRes->SetRW(false);
     hlslRes->SetID(m_pHLModule->GetSRVs().size());
   } else {

tools/clang/lib/Sema/SemaHLSL.cpp

@@ -10,6 +10,7 @@
 //                                                                           //
 ///////////////////////////////////////////////////////////////////////////////
 
+#include "clang/AST/DeclBase.h"
 #include "clang/Basic/Diagnostic.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/DenseMap.h"
@@ -12131,6 +12132,26 @@ bool ValidateAttributeTargetIsFunction(Sema& S, Decl* D, const AttributeList &A)
   return false;
 }
 
+void Sema::DiagnoseHLSLDeclAttr(const Decl *D, const Attr *A) {
+  HLSLExternalSource *ExtSource = HLSLExternalSource::FromSema(this);
+  if (const HLSLGloballyCoherentAttr *HLSLGCAttr =
+          dyn_cast<HLSLGloballyCoherentAttr>(A)) {
+    const ValueDecl *TD = cast<ValueDecl>(D);
+    if (!TD->getType()->isDependentType()) {
+      QualType DeclType = TD->getType();
+      while (DeclType->isArrayType())
+        DeclType = QualType(DeclType->getArrayElementTypeNoTypeQual(), 0);
+      if (ExtSource->GetTypeObjectKind(DeclType) != AR_TOBJ_OBJECT ||
+          hlsl::GetResourceClassForType(getASTContext(), DeclType) !=
+              hlsl::DXIL::ResourceClass::UAV) {
+        Diag(A->getLocation(), diag::err_hlsl_varmodifierna)
+            << A << "non-UAV type";
+      }
+    }
+    return;
+  }
+}
+
 void hlsl::HandleDeclAttributeForHLSL(Sema &S, Decl *D, const AttributeList &A, bool& Handled)
 {
   DXASSERT_NOMSG(D != nullptr);
@@ -12296,6 +12317,7 @@ void hlsl::HandleDeclAttributeForHLSL(Sema &S, Decl *D, const AttributeList &A,
 
   if (declAttr != nullptr)
   {
+    S.DiagnoseHLSLDeclAttr(D, declAttr);
     DXASSERT_NOMSG(Handled);
     D->addAttr(declAttr);
     return;
@@ -13127,12 +13149,7 @@ bool Sema::DiagnoseHLSLDecl(Declarator &D, DeclContext *DC, Expr *BitWidth,
         result = false;
       }
       break;
-    case AttributeList::AT_HLSLGloballyCoherent:
-      if (!bIsObject) {
-        Diag(pAttr->getLoc(), diag::err_hlsl_varmodifierna)
-            << pAttr->getName() << "non-UAV type";
-        result = false;
-      }
+    case AttributeList::AT_HLSLGloballyCoherent: // Handled elsewhere
       break;
     case AttributeList::AT_HLSLUniform:
       if (!(isGlobal || isParameter)) {

tools/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

@@ -22,6 +22,7 @@
 #include "clang/AST/TypeLoc.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/PrettyDeclStackTrace.h"
+#include "clang/Sema/SemaHLSL.h" // HLSL Change
 #include "clang/Sema/Template.h"
 
 using namespace clang;
@@ -266,6 +267,10 @@ void Sema::InstantiateAttrs(const MultiLevelTemplateArgumentList &TemplateArgs,
       continue;
     }
 
+    // HLSL Change Begin - Validate post-instantiation attributes
+    DiagnoseHLSLDeclAttr(New, TmplAttr);
+    // HLSL Change End
+
     // Existing DLL attribute on the instantiation takes precedence.
     if (TmplAttr->getKind() == attr::DLLExport ||
         TmplAttr->getKind() == attr::DLLImport) {

tools/clang/lib/Sema/SemaType.cpp

@@ -5797,11 +5797,6 @@ static bool handleHLSLTypeAttr(TypeProcessingState &State,
               !hlsl::GetOriginalElementType(&S, Type)->isFloatingType()) {
     S.Diag(Attr.getLoc(), diag::err_hlsl_norm_float_only) << Attr.getRange();
     return true;
-  } else if (Kind == AttributeList::AT_HLSLGloballyCoherent &&
-             !hlsl::IsObjectType(&S, Type)) {
-    S.Diag(Attr.getLoc(), diag::err_hlsl_varmodifierna) <<
-        Attr.getName() << "non-UAV type";
-    return true;
   }
 
   const AttributedType *pMatrixOrientation = nullptr;

tools/clang/test/CodeGenHLSL/globallycoherent2.hlsl

@@ -1,17 +0,0 @@
-// RUN: %dxc -E main -T ps_6_0 %s | FileCheck %s
-
-// CHECK: globallycoherent can only be used with Unordered Access View buffers
-
-globallycoherent RWTexture1D<float4> uav1 : register(u3);
-RWBuffer<float4> uav2;
-globallycoherent Buffer<float4> srv;
-
- float4 main(uint2 a : A, uint2 b : B) : SV_Target
-{
-  globallycoherent  RWTexture1D<float4> uav3 = uav1;
-  float x = 3;
-  uav3[0] = srv[0];
-  uav1[0] = 2;
-  uav2[1] = 3;
-  return 0;
-}

tools/clang/test/CodeGenHLSL/globallycoherent3.hlsl

@@ -1,16 +0,0 @@
-// RUN: %dxc -E main -T ps_6_0 %s | FileCheck %s
-
-// CHECK: 'globallycoherent' is not a valid modifier for a non-UAV type
-
-globallycoherent RWTexture1D<float4> uav1 : register(u3);
-RWBuffer<float4> uav2;
-globallycoherent float m;
-float4 main(uint2 a : A, uint2 b : B) : SV_Target
-{
-  globallycoherent  RWTexture1D<float4> uav3 = uav1;
-  globallycoherent float x = 3;
-  uav3[0] = 0;
-  uav1[0] = 2;
-  uav2[1] = 3;
-  return 0;
-}

tools/clang/test/DXILValidation/globallycoherent2.hlsl

@@ -1,17 +0,0 @@
-// RUN: %dxc -E main -T ps_6_0 %s | FileCheck %s
-
-// CHECK: globallycoherent can only be used with Unordered Access View buffers
-
-globallycoherent RWTexture1D<float4> uav1 : register(u3);
-RWBuffer<float4> uav2;
-globallycoherent Buffer<float4> srv;
-
- float4 main(uint2 a : A, uint2 b : B) : SV_Target
-{
-  globallycoherent  RWTexture1D<float4> uav3 = uav1;
-  float x = 3;
-  uav3[0] = srv[0];
-  uav1[0] = 2;
-  uav2[1] = 3;
-  return 0;
-}

tools/clang/test/DXILValidation/globallycoherent3.hlsl

@@ -1,16 +0,0 @@
-// RUN: %dxc -E main -T ps_6_0 %s | FileCheck %s
-
-// CHECK: 'globallycoherent' is not a valid modifier for a non-UAV type
-
-globallycoherent RWTexture1D<float4> uav1 : register(u3);
-RWBuffer<float4> uav2;
-globallycoherent float m;
-float4 main(uint2 a : A, uint2 b : B) : SV_Target
-{
-  globallycoherent  RWTexture1D<float4> uav3 = uav1;
-  globallycoherent float x = 3;
-  uav3[0] = 0;
-  uav1[0] = 2;
-  uav2[1] = 3;
-  return 0;
-}

tools/clang/test/HLSL/globallycoherent-errors.hlsl

@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -fsyntax-only -ffreestanding -verify %s
+
+globallycoherent RWTexture1D<float4> uav1 : register(u3);
+RWBuffer<float4> uav2;
+globallycoherent Buffer<float4> srv; // expected-error {{'globallycoherent' is not a valid modifier for a non-UAV type}}
+globallycoherent float m; // expected-error {{'globallycoherent' is not a valid modifier for a non-UAV type}}
+
+globallycoherent RWTexture2D<float> tex[12];
+globallycoherent RWTexture2D<float> texMD[12][12];
+
+ float4 main(uint2 a : A, uint2 b : B) : SV_Target
+{
+  globallycoherent  RWTexture1D<float4> uav3 = uav1;
+  globallycoherent float x = 3; // expected-error {{'globallycoherent' is not a valid modifier for a non-UAV type}}
+  uav3[0] = srv[0];
+  uav1[0] = 2;
+  uav2[1] = 3;
+  return 0;
+}

tools/clang/test/HLSL/globallycoherent-template-errors.hlsl

@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -HV 2021 -fsyntax-only -ffreestanding -verify %s
+
+template <typename T> void doSomething(uint pos) {
+  globallycoherent RWTexture2D<T> output;
+  globallycoherent Buffer<T> nonUAV; // expected-error {{'globallycoherent' is not a valid modifier for a non-UAV type}}
+  globallycoherent T ThisShouldBreak = 2.0; // expected-error {{'globallycoherent' is not a valid modifier for a non-UAV type}}
+  output[uint2(pos, pos)] = 0;
+}
+
+void doSomething2(uint pos) {
+  globallycoherent RWTexture2D<float> output;
+  globallycoherent float ThisShouldBreak = 2.0; // expected-error {{'globallycoherent' is not a valid modifier for a non-UAV type}}
+}
+
+[numthreads(8, 8, 1)] void main(uint threadId
+                                : SV_DispatchThreadID) {
+  doSomething<float>(threadId); // expected-note {{in instantiation of function template specialization 'doSomething<float>' requested here}}
+  doSomething2(threadId);
+}

tools/clang/test/HLSLFileCheck/hlsl/types/modifiers/globallycoherent/globallycoherent2.hlsl

@@ -1,17 +0,0 @@
-// RUN: %dxc -E main -T ps_6_0 %s | FileCheck %s
-
-// CHECK: globallycoherent can only be used with Unordered Access View buffers
-
-globallycoherent RWTexture1D<float4> uav1 : register(u3);
-RWBuffer<float4> uav2;
-globallycoherent Buffer<float4> srv;
-
- float4 main(uint2 a : A, uint2 b : B) : SV_Target
-{
-  globallycoherent  RWTexture1D<float4> uav3 = uav1;
-  float x = 3;
-  uav3[0] = srv[0];
-  uav1[0] = 2;
-  uav2[1] = 3;
-  return 0;
-}

tools/clang/unittests/HLSL/ValidationTest.cpp

@@ -136,8 +136,7 @@ public:
   TEST_METHOD(I8Type)
   TEST_METHOD(EmptyStructInBuffer)
   TEST_METHOD(BigStructInBuffer)
-  TEST_METHOD(GloballyCoherent2)
-  TEST_METHOD(GloballyCoherent3)
+
   // TODO: enable this.
   //TEST_METHOD(TGSMRaceCond)
   //TEST_METHOD(TGSMRaceCond2)
@@ -1614,14 +1613,6 @@ TEST_F(ValidationTest, BigStructInBuffer) {
   TestCheck(L"..\\CodeGenHLSL\\BigStructInBuffer.hlsl");
 }
 
-TEST_F(ValidationTest, GloballyCoherent2) {
-  TestCheck(L"..\\CodeGenHLSL\\globallycoherent2.hlsl");
-}
-
-TEST_F(ValidationTest, GloballyCoherent3) {
-  TestCheck(L"..\\CodeGenHLSL\\globallycoherent3.hlsl");
-}
-
 // TODO: enable this.
 //TEST_F(ValidationTest, TGSMRaceCond) {
 //  TestCheck(L"..\\CodeGenHLSL\\RaceCond.hlsl");

tools/clang/unittests/HLSL/VerifierTest.cpp

@@ -100,6 +100,8 @@ public:
   TEST_METHOD(RunWriteConstArrays)
   TEST_METHOD(RunAtomicsOnBitfields)
   TEST_METHOD(RunUnboundedResourceArrays)
+  TEST_METHOD(GloballyCoherentErrors)
+  TEST_METHOD(GloballyCoherentTemplateErrors)
   TEST_METHOD(RunBitFieldAnnotations)
   void CheckVerifies(const wchar_t* path) {
     WEX::TestExecution::SetVerifyOutput verifySettings(WEX::TestExecution::VerifyOutputSettings::LogOnlyFailures);
@@ -435,6 +437,14 @@ TEST_F(VerifierTest, RunUnboundedResourceArrays) {
   CheckVerifiesHLSL(L"invalid-unbounded-resource-arrays.hlsl");
 }
 
+TEST_F(VerifierTest, GloballyCoherentErrors) {
+  CheckVerifiesHLSL(L"globallycoherent-errors.hlsl");
+}
+
+TEST_F(VerifierTest, GloballyCoherentTemplateErrors) {
+  CheckVerifiesHLSL(L"globallycoherent-template-errors.hlsl");
+}
+
 TEST_F(VerifierTest, RunBitFieldAnnotations) {
   CheckVerifiesHLSL(L"bitfields-and-annotations.hlsl");
 }