[ser] Diagnose implied 'reordercoherent' in type attributes

Author: simoll

tools/clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -7769,6 +7769,8 @@ def warn_hlsl_impcast_rdc_mismatch : Warning<
 def warn_hlsl_impcast_rdc_glc_mismatch : Warning<
   "implicit conversion from %0 to %1 %select{demotes globallycoherent to reordercoherent|promotes reordercoherent to globallycoherent}2 annotation">,
   InGroup<Conversion>, DefaultWarn;
+def warn_hlsl_glc_implies_rdc : Warning<
+  "attribute 'globallycoherent' implies 'reordercoherent'">, InGroup<IgnoredAttributes>;
 def warn_hlsl_narrowing : Warning<
   "conversion from larger type %0 to smaller type %1, possible loss of data">,
   InGroup<Conversion>, DefaultWarn;

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

@@ -203,7 +203,8 @@ void Indent(unsigned int Indentation, llvm::raw_ostream &Out);
 void GetHLSLAttributedTypes(clang::Sema *self, clang::QualType type,
                             const clang::AttributedType **ppMatrixOrientation,
                             const clang::AttributedType **ppNorm,
-                            const clang::AttributedType **ppGLC);
+                            const clang::AttributedType **ppGLC,
+                            const clang::AttributedType **ppRDC);
 
 bool IsMatrixType(clang::Sema *self, clang::QualType type);
 bool IsVectorType(clang::Sema *self, clang::QualType type);

tools/clang/lib/Sema/SemaHLSL.cpp

@@ -15326,15 +15326,17 @@ static QualType getUnderlyingType(QualType Type) {
 void hlsl::GetHLSLAttributedTypes(
     clang::Sema *self, clang::QualType type,
     const clang::AttributedType **ppMatrixOrientation,
-    const clang::AttributedType **ppNorm, const clang::AttributedType **ppGLC) {
+    const clang::AttributedType **ppNorm, const clang::AttributedType **ppGLC,
+    const clang::AttributedType **ppRDC) {
   AssignOpt<const clang::AttributedType *>(nullptr, ppMatrixOrientation);
   AssignOpt<const clang::AttributedType *>(nullptr, ppNorm);
   AssignOpt<const clang::AttributedType *>(nullptr, ppGLC);
+  AssignOpt<const clang::AttributedType *>(nullptr, ppRDC);
 
   // Note: we clear output pointers once set so we can stop searching
   QualType Desugared = getUnderlyingType(type);
   const AttributedType *AT = dyn_cast<AttributedType>(Desugared);
-  while (AT && (ppMatrixOrientation || ppNorm || ppGLC)) {
+  while (AT && (ppMatrixOrientation || ppNorm || ppGLC || ppRDC)) {
     AttributedType::Kind Kind = AT->getAttrKind();
 
     if (Kind == AttributedType::attr_hlsl_row_major ||
@@ -15354,6 +15356,11 @@ void hlsl::GetHLSLAttributedTypes(
         *ppGLC = AT;
         ppGLC = nullptr;
       }
+    } else if (Kind == AttributedType::attr_hlsl_reordercoherent) {
+      if (ppRDC) {
+        *ppRDC = AT;
+        ppRDC = nullptr;
+      }
     }
 
     Desugared = getUnderlyingType(AT->getEquivalentType());

tools/clang/lib/Sema/SemaType.cpp

@@ -5805,7 +5805,9 @@ static bool handleHLSLTypeAttr(TypeProcessingState &State,
   const AttributedType *pMatrixOrientation = nullptr;
   const AttributedType *pNorm = nullptr;
   const AttributedType *pGLC = nullptr;
-  hlsl::GetHLSLAttributedTypes(&S, Type, &pMatrixOrientation, &pNorm, &pGLC);
+  const AttributedType *pRDC = nullptr;
+  hlsl::GetHLSLAttributedTypes(&S, Type, &pMatrixOrientation, &pNorm, &pGLC,
+                               &pRDC);
 
   if (pMatrixOrientation &&
     (Kind == AttributeList::AT_HLSLColumnMajor ||
@@ -5839,14 +5841,18 @@ static bool handleHLSLTypeAttr(TypeProcessingState &State,
     return true;
   }
 
-  if (pGLC && (Kind == AttributeList::AT_HLSLGloballyCoherent ||
-               Kind == AttributeList::AT_HLSLReorderCoherent)) {
-    AttributedType::Kind CurAttrKind = pGLC->getAttrKind();
-    if (Kind == getAttrListKind(CurAttrKind)) {
-      S.Diag(Attr.getLoc(), diag::warn_duplicate_attribute_exact)
-          << Attr.getName() << Attr.getRange();
-    }
-  }
+  const bool hasGLC = pGLC;
+  const bool addsGLC = Kind == AttributeList::AT_HLSLGloballyCoherent;
+  const bool hasRDC = pRDC;
+  const bool addsRDC = Kind == AttributeList::AT_HLSLReorderCoherent;
+
+  const bool hasMismatchingAttrs = hasGLC && hasRDC;
+  const bool addsMismatchingAttr = (hasGLC && addsRDC) || (hasRDC && addsGLC);
+  if ((hasGLC && addsGLC) || (hasRDC && addsRDC))
+    S.Diag(Attr.getLoc(), diag::warn_duplicate_attribute_exact)
+        << Attr.getName() << Attr.getRange();
+  else if (!hasMismatchingAttrs && addsMismatchingAttr)
+    S.Diag(Attr.getLoc(), diag::warn_hlsl_glc_implies_rdc) << Attr.getRange();
 
   AttributedType::Kind TAK;
   switch (Kind) {

tools/clang/test/SemaHLSL/reordercoherent-implied.hlsl

@@ -0,0 +1,41 @@
+// RUN: %dxc -E main -T lib_6_9 -verify %s
+// REQUIRES: dxil-1-9
+
+using Ty = RWTexture1D<float4>;
+
+using GTy = globallycoherent Ty;
+using RTy = reordercoherent Ty;
+
+// expected-warning@+1{{attribute 'globallycoherent' is already applied}}
+using GGTy = globallycoherent GTy;
+// expected-warning@+1{{attribute 'reordercoherent' is already applied}}
+using RRTy = reordercoherent RTy;
+
+// expected-warning@+1{{attribute 'globallycoherent' implies 'reordercoherent'}}
+using GRTy = globallycoherent RTy;
+// expected-warning@+1{{attribute 'globallycoherent' implies 'reordercoherent'}}
+using RGTy = reordercoherent GTy;
+
+// expected-warning@+1{{attribute 'globallycoherent' is already applied}}
+using GGRTy = globallycoherent GRTy;
+// expected-warning@+1{{attribute 'reordercoherent' is already applied}}
+using RRGTy = reordercoherent RGTy;
+
+// expected-warning@+1{{attribute 'globallycoherent' implies 'reordercoherent'}}
+using GRTy2 = globallycoherent reordercoherent Ty;
+// expected-warning@+1{{attribute 'globallycoherent' implies 'reordercoherent'}}
+using RGTy2 = reordercoherent globallycoherent Ty;
+
+// expected-warning@+2{{attribute 'globallycoherent' implies 'reordercoherent'}}
+// expected-warning@+1{{attribute 'globallycoherent' is already applied}}
+using GGRTy2 = globallycoherent globallycoherent reordercoherent Ty;
+// expected-warning@+2{{attribute 'globallycoherent' implies 'reordercoherent'}}
+// expected-warning@+1{{attribute 'globallycoherent' is already applied}}
+using GRGTy2 = globallycoherent reordercoherent globallycoherent Ty;
+
+// expected-warning@+2{{attribute 'globallycoherent' implies 'reordercoherent'}}
+// expected-warning@+1{{attribute 'reordercoherent' is already applied}}
+using RGRTy2 = reordercoherent globallycoherent reordercoherent Ty;
+// expected-warning@+2{{attribute 'globallycoherent' implies 'reordercoherent'}}
+// expected-warning@+1{{attribute 'reordercoherent' is already applied}}
+using RRGTy2 = reordercoherent reordercoherent globallycoherent Ty;