[ser] Declare HitObject type regardless of SM, use availability attributes to check SM version for SER

Author: simoll

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

@@ -7662,8 +7662,8 @@ def warn_hlsl_rayquery_flags_conflict : Warning<
   " must have RAYQUERY_FLAG_ALLOW_OPACITY_MICROMAPS set.">, DefaultError, InGroup<HLSLRayQueryFlags>;
 def err_hlsl_unsupported_builtin_op: Error<
   "operator cannot be used with built-in type %0">;
-def warn_hlsl_builtin_constant_unavailable: Warning<
-  "potential misuse of built-in constant %0 in shader model %1; introduced"
+def warn_hlsl_builtin_unavailable: Warning<
+  "potential misuse of built-in %select{constant|function}3 '%0' in shader model %1; introduced"
   " in shader model %2">, InGroup<HLSLAvailabilityConstant>;
 def err_hlsl_unsupported_char_literal : Error<
   "unsupported style of char literal - use a single-character char-based literal">;

tools/clang/lib/AST/ASTContextHLSL.cpp

@@ -1243,6 +1243,10 @@ CXXRecordDecl *hlsl::DeclareHitObjectType(NamespaceDecl &NSDecl) {
       static_cast<int>(hlsl::IntrinsicOp::MOP_DxHitObject_MakeNop)));
   pConstructorDecl->addAttr(HLSLCXXOverloadAttr::CreateImplicit(Context));
 
+  // Shader Execution Reordering requires SM6.9
+  VersionTuple VT69 = VersionTuple(6, 9);
+  pConstructorDecl->addAttr(ConstructAvailabilityAttribute(Context, VT69));
+
   // Add the implicit HLSLHitObjectAttr attribute to unambiguously recognize the
   // builtin HitObject type (SM6.9+). This distinguishes it from any
   // user-defined type named 'HitObject' pre SM6.9.

tools/clang/lib/Sema/SemaHLSL.cpp

@@ -1941,15 +1941,14 @@ static void AddHLSLIntrinsicAttr(FunctionDecl *FD, ASTContext &context,
     FD->addAttr(PureAttr::CreateImplicit(context));
   if (pIntrinsic->Flags & INTRIN_FLAG_IS_WAVE)
     FD->addAttr(HLSLWaveSensitiveAttr::CreateImplicit(context));
-  // TBD: Add availability attribute if MinShaderModel is set.
-  // if (pIntrinsic->MinShaderModel) {
-  //  unsigned Major = pIntrinsic->MinShaderModel >> 4;
-  //  unsigned Minor = pIntrinsic->MinShaderModel & 0xF;
-  //  FD->addAttr(AvailabilityAttr::CreateImplicit(
-  //      context, &context.Idents.get(""), clang::VersionTuple(Major, Minor),
-  //      clang::VersionTuple(), clang::VersionTuple(), false,
-  //      "HLSL Intrinsic availability limited by shader model."));
-  //}
+  if (pIntrinsic->MinShaderModel) {
+    unsigned Major = pIntrinsic->MinShaderModel >> 4;
+    unsigned Minor = pIntrinsic->MinShaderModel & 0xF;
+    FD->addAttr(AvailabilityAttr::CreateImplicit(
+        context, &context.Idents.get(""), clang::VersionTuple(Major, Minor),
+        clang::VersionTuple(), clang::VersionTuple(), false,
+        "HLSL Intrinsic availability limited by shader model."));
+  }
 }
 
 static FunctionDecl *
@@ -3781,10 +3780,8 @@ class HLSLExternalSource : public ExternalSemaSource {
         recordDecl = DeclareRayQueryType(*m_context);
       } else if (kind == AR_OBJECT_HIT_OBJECT) {
         // Declare 'HitObject' in '::dx' extension namespace.
-        if (SM->IsSM69Plus()) {
-          DXASSERT(m_dxNSDecl, "namespace ::dx must be declared in SM6.9+");
-          recordDecl = DeclareHitObjectType(*m_dxNSDecl);
-        }
+        DXASSERT(m_dxNSDecl, "namespace ::dx must be declared in SM6.9+");
+        recordDecl = DeclareHitObjectType(*m_dxNSDecl);
       } else if (kind == AR_OBJECT_HEAP_RESOURCE) {
         recordDecl = DeclareResourceType(*m_context, /*bSampler*/ false);
         if (SM->IsSM66Plus()) {
@@ -4063,8 +4060,6 @@ class HLSLExternalSource : public ExternalSemaSource {
     S.addExternalSource(this);
 
     // Namespace ::dx only introduced with SM6.9
-    const auto *SM =
-        hlsl::ShaderModel::GetByName(m_sema->getLangOpts().HLSLProfile.c_str());
     m_dxNSDecl =
         NamespaceDecl::Create(context, context.getTranslationUnitDecl(),
                               /*Inline*/ false, SourceLocation(),
@@ -4091,8 +4086,7 @@ class HLSLExternalSource : public ExternalSemaSource {
       AddIntrinsicTableMethods(intrinsic);
     }
 
-    if (SM->IsSM69Plus())
-      AddDxIntrinsicFunctions();
+    AddDxIntrinsicFunctions();
 
 #ifdef ENABLE_SPIRV_CODEGEN
     if (m_sema->getLangOpts().SPIRV) {
@@ -5083,17 +5077,6 @@ class HLSLExternalSource : public ExternalSemaSource {
                                            nameIdentifier, argumentCount));
   }
 
-  bool IsEnabledIntrinsic(const HLSL_INTRINSIC *pIntrinsic,
-                          const ShaderModel &SM) {
-    switch ((IntrinsicOp)pIntrinsic->Op) {
-    default:
-      return true;
-    case IntrinsicOp::MOP_DxHitObject_MakeNop:
-    case IntrinsicOp::IOP_DxMaybeReorderThread:
-      return SM.IsSM69Plus();
-    }
-  }
-
   bool AddOverloadedCallCandidates(UnresolvedLookupExpr *ULE,
                                    ArrayRef<Expr *> Args,
                                    OverloadCandidateSet &CandidateSet,
@@ -5145,9 +5128,6 @@ class HLSLExternalSource : public ExternalSemaSource {
     }
 #endif // ENABLE_SPIRV_CODEGEN
 
-    const auto *SM =
-        hlsl::ShaderModel::GetByName(m_sema->getLangOpts().HLSLProfile.c_str());
-
     IntrinsicDefIter cursor = FindIntrinsicByNameAndArgCount(
         table, tableCount, StringRef(), nameIdentifier, Args.size());
     IntrinsicDefIter end = IntrinsicDefIter::CreateEnd(
@@ -5156,13 +5136,6 @@ class HLSLExternalSource : public ExternalSemaSource {
     for (; cursor != end; ++cursor) {
       const HLSL_INTRINSIC *pIntrinsic = *cursor;
 
-      // Check whether this extension intrinsic is available in the current
-      // configuration. Note that this not necessary for member functions:
-      // if the builtin type is never declared, neither will any type refer
-      // to the member intrinsics table.
-      if (!IsEnabledIntrinsic(*cursor, *SM))
-        continue;
-
       // If this is the intrinsic we're interested in, build up a representation
       // of the types we need.
       LPCSTR tableName = cursor.GetTableName();
@@ -11935,11 +11908,6 @@ void Sema::DiagnoseReachableCallForSER(CallExpr *CE, DXIL::ShaderKind EntrySK,
         << DiagValidShaderKinds;
     Diag(EntryFD->getLocation(), diag::note_hlsl_entry_defined_here);
   }
-
-  if (!SM->IsSM69Plus()) {
-    // Legal entry, but version not supported
-    Diag(Loc, diag::err_hlsl_ser_invalid_version) << SM->GetName();
-  }
 }
 
 // Check HLSL member call constraints for used functions.

tools/clang/lib/Sema/SemaHLSLDiagnoseTU.cpp

@@ -24,6 +24,7 @@
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/Support/Debug.h"
+#include "llvm/Support/raw_ostream.h"
 #include <optional>
 
 using namespace clang;
@@ -432,13 +433,27 @@ class HLSLCallDiagnoseVisitor // Could rename to HLSLReachableDiagnoseVisitor
     // if the current shader model is lower than what
     // is stated in the availability attribute, emit
     // the availability warning.
-
-    if (SMVT < AAttrVT) {
-      // TBD: Determine best way to distinguish between builtin constant decls
-      // and other decls.
-      sema->Diag(Loc, diag::warn_hlsl_builtin_constant_unavailable)
-          << ND << SM->GetName() << AAttrVT.getAsString();
+    if (SMVT >= AAttrVT)
+      return;
+
+    // TBD: Determine best way to distinguish between builtin constant decls
+    // and other decls.
+    enum class DiagKind { Constant = 0, Function = 1 };
+    DiagKind Kind;
+    switch (ND->getKind()) {
+    case Decl::Function:
+    case Decl::FunctionTemplate:
+    case Decl::CXXMethod:
+      Kind = DiagKind::Function;
+      break;
+    default:
+      Kind = DiagKind::Constant;
+      break;
     }
+
+    sema->Diag(Loc, diag::warn_hlsl_builtin_unavailable)
+        << ND->getQualifiedNameAsString() << SM->GetName()
+        << AAttrVT.getAsString() << (int)Kind;
   }
 
   clang::Sema *getSema() { return sema; }
@@ -453,29 +468,6 @@ class HLSLCallDiagnoseVisitor // Could rename to HLSLReachableDiagnoseVisitor
   llvm::SmallPtrSetImpl<DeclRefExpr *> &DeclAvailabilityChecked;
 };
 
-class HLSLDisallowSERDiagnoseVisitor
-    : public RecursiveASTVisitor<HLSLDisallowSERDiagnoseVisitor> {
-public:
-  explicit HLSLDisallowSERDiagnoseVisitor(Sema &S, DXIL::ShaderKind EntrySK,
-                                          const FunctionDecl *EntryDecl)
-      : S(S), EntrySK(EntrySK), EntryDecl(EntryDecl) {}
-
-  bool VisitTypeLoc(TypeLoc TL) {
-    if (!hlsl::IsHLSLHitObjectType(TL.getType()))
-      return true;
-
-    S.Diag(TL.getLocStart(), diag::err_hlsl_ser_unsupported)
-        << 0 << ShaderModel::FullNameFromKind(EntrySK) << 1;
-    S.Diag(EntryDecl->getLocation(), diag::note_hlsl_entry_defined_here);
-    return false;
-  }
-
-private:
-  clang::Sema &S;
-  DXIL::ShaderKind EntrySK;
-  const FunctionDecl *EntryDecl;
-};
-
 std::optional<uint32_t>
 getFunctionInputPatchCount(const FunctionDecl *function) {
   for (const auto *param : function->params()) {
@@ -697,18 +689,6 @@ void hlsl::DiagnoseTranslationUnit(clang::Sema *self) {
       }
     }
 
-    // Shader Execution Reordering
-    // MaybeReorderThread(..) handled elsewhere.
-    const bool AllowHitObject = (EntrySK == DXIL::ShaderKind::ClosestHit ||
-                                 EntrySK == DXIL::ShaderKind::Miss ||
-                                 EntrySK == DXIL::ShaderKind::RayGeneration);
-    if (!AllowHitObject) {
-      HLSLDisallowSERDiagnoseVisitor Visitor(*self, EntrySK, FDecl);
-      for (FunctionDecl *FD : callGraph.GetVisitedFunctions()) {
-        Visitor.TraverseDecl(FD);
-      }
-    }
-
     // Visit all visited functions in call graph to collect illegal intrinsic
     // calls.
     HLSLCallDiagnoseVisitor Visitor(self, shaderModel, EntrySK, NodeLaunchTy,

…reorder/reorder-defineable-pre-sm69.hlsl …eorder/reorder-unavailable-pre-sm69.hlsltools/clang/test/SemaHLSL/hlsl/intrinsics/reorder/reorder-defineable-pre-sm69.hlsl renamed to tools/clang/test/SemaHLSL/hlsl/intrinsics/reorder/reorder-unavailable-pre-sm69.hlsl tools/clang/test/SemaHLSL/hlsl/intrinsics/reorder/reorder-defineable-pre-sm69.hlsl renamed to tools/clang/test/SemaHLSL/hlsl/intrinsics/reorder/reorder-unavailable-pre-sm69.hlsl

@@ -1,15 +1,9 @@
-// RUN: %dxc -T lib_6_6 %s -verify
 // RUN: %dxc -T lib_6_8 %s -verify
 
 // Check that intrinsic names of Shader Execution Reordering are unclaimed pre SM 6.9.
-// expected-no-diagnostics
-
-namespace dx {
-void MaybeReorderThread(uint CoherenceHint, uint NumCoherenceHintBitsFromLSB) {
-}
-}
 
 [shader("raygeneration")]
 void main() {
+  // expected-warning@+1{{potential misuse of built-in function 'dx::MaybeReorderThread' in shader model lib_6_8; introduced in shader model 6.9}}
   dx::MaybeReorderThread(15u, 4u);
 }

tools/clang/test/SemaHLSL/hlsl/objects/HitObject/hitobject-entry-errors.hlsl

@@ -9,8 +9,8 @@ struct [raypayload] Payload
 
 struct Attribs { float2 barys; };
 
-void UseHitObject() {
-  dx::HitObject hit;
+dx::HitObject UseHitObject() {
+  return dx::HitObject::MakeNop();
 }
 
 // expected-note@+3{{entry function defined here}}

tools/clang/test/SemaHLSL/hlsl/objects/HitObject/hitobject-target-profile-error.hlsl

@@ -4,6 +4,6 @@ namespace dx {}
 
 [shader("raygeneration")]
 void main() {
-// expected-error@+1{{no type named 'HitObject' in namespace 'dx'}}
-  dx::HitObject hit;
+  // expected-warning@+1{{potential misuse of built-in function 'dx::HitObject::MakeNop' in shader model lib_6_8; introduced in shader model 6.9}}
+  dx::HitObject::MakeNop();
 }

…bject/hitobject-defineable-pre-sm69.hlsl …ject/hitobject-unavailable-pre-sm69.hlsltools/clang/test/SemaHLSL/hlsl/objects/HitObject/hitobject-defineable-pre-sm69.hlsl renamed to tools/clang/test/SemaHLSL/hlsl/objects/HitObject/hitobject-unavailable-pre-sm69.hlsl tools/clang/test/SemaHLSL/hlsl/objects/HitObject/hitobject-defineable-pre-sm69.hlsl renamed to tools/clang/test/SemaHLSL/hlsl/objects/HitObject/hitobject-unavailable-pre-sm69.hlsl

@@ -1,20 +1,9 @@
 // RUN: %dxc -T lib_6_8 %s -verify
 
 // Check that the HitObject type name of Shader Execution Reordering is unclaimed pre SM 6.9.
-// expected-no-diagnostics
-
-namespace dx {
-struct HitObject {
-  int notTheSM69HitObject;
-  static HitObject MakeNop() {
-    HitObject hit;
-    hit.notTheSM69HitObject = 1;
-    return hit;
-  }
-};
-}
 
 [shader("raygeneration")]
 void main() {
+  // expected-warning@+1{{potential misuse of built-in function 'dx::HitObject::MakeNop' in shader model lib_6_8; introduced in shader model 6.9}}
   dx::HitObject hit = dx::HitObject::MakeNop();
 }

tools/clang/test/SemaHLSL/hlsl/objects/HitObject/hitobject-undeclared-pre-sm69.hlsl

@@ -3,6 +3,6 @@
 // expected-note@+1{{entry function defined here}}
 float main(RayDesc rayDesc: RAYDESC) : OUT {
 // expected-error@+1{{dx::HitObject is unavailable in shader stage 'vertex' (requires 'raygeneration', 'closesthit' or 'miss')}}
-  dx::HitObject hit;
+  dx::HitObject::MakeNop();
   return 0.f;
 }