From 7c4d91e7819a1d27213aa3499953d54ae1a00e8f Mon Sep 17 00:00:00 2001
From: dTry <92609548+D7ry@users.noreply.github.com>
Date: Thu, 8 Aug 2024 07:21:00 -0700
Subject: [PATCH] Add type checks for hitObjectNV (#3689)

`VK_NV_ray_tracing_invocation_reorder` extension introduces `hitObjectNV`, a special opaque type
that can only be declared without storage qualifiers in either global or function scope.
Added checks/tests to enforce this constraint.

References:
https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_NV_ray_tracing_invocation_reorder.html
https://github.com/KhronosGroup/GLSL/blob/3e0d9a3b3f54651ef53d533392a7401a3b19e16d/extensions/nv/GLSL_NV_shader_invocation_reorder.txt#L180

Co-authored-by: Tianyun <tianyuny@nvidia.com>
---
 .../spv.nv.hitobject-errors.rgen.out          |  9 +++++++
 Test/spv.nv.hitobject-errors.rgen             | 24 +++++++++++++++++++
 glslang/MachineIndependent/ParseHelper.cpp    | 13 ++++++++++
 glslang/MachineIndependent/ParseHelper.h      |  1 +
 gtests/Spv.FromFile.cpp                       |  1 +
 5 files changed, 48 insertions(+)
 create mode 100644 Test/baseResults/spv.nv.hitobject-errors.rgen.out
 create mode 100644 Test/spv.nv.hitobject-errors.rgen

diff --git a/Test/baseResults/spv.nv.hitobject-errors.rgen.out b/Test/baseResults/spv.nv.hitobject-errors.rgen.out
new file mode 100644
index 000000000..139482752
--- /dev/null
+++ b/Test/baseResults/spv.nv.hitobject-errors.rgen.out
@@ -0,0 +1,9 @@
+spv.nv.hitobject-errors.rgen
+ERROR: 0:7: 'hitObjectNV' : hitObjectNV can only be declared in global or function scope with no storage qualifier: uHitObj
+ERROR: 0:9: 'hitObjectNV' : hitObjectNV can only be declared in global or function scope with no storage qualifier: hobjIn
+ERROR: 0:10: 'hitObjectNV' : hitObjectNV can only be declared in global or function scope with no storage qualifier: hobjOut
+ERROR: 0:21: 'hObjWrapper' : struct is not allowed to contain hitObjectNV: wrapper
+ERROR: 4 compilation errors.  No code generated.
+
+
+SPIR-V is not generated for failed compile or link
diff --git a/Test/spv.nv.hitobject-errors.rgen b/Test/spv.nv.hitobject-errors.rgen
new file mode 100644
index 000000000..9744ccf4c
--- /dev/null
+++ b/Test/spv.nv.hitobject-errors.rgen
@@ -0,0 +1,24 @@
+#version 460
+#extension GL_EXT_ray_tracing : enable
+#extension GL_NV_shader_invocation_reorder : enable
+ 
+
+hitObjectNV hObjGlob;                     // OK
+uniform hitObjectNV uHitObj;              // ERROR
+
+layout(location=0) in hitObjectNV hobjIn; // ERROR
+out hitObjectNV hobjOut;                  // ERROR
+
+struct hObjWrapper{
+    hitObjectNV objField;
+    vec3 v;
+};
+
+void foo(hitObjectNV hObjArg) {}           // OK
+
+void main()
+{
+    hObjWrapper wrapper;                   // ERROR
+    hitObjectNV localHitObj;               // OK
+    foo(localHitObj);                      // OK
+}
diff --git a/glslang/MachineIndependent/ParseHelper.cpp b/glslang/MachineIndependent/ParseHelper.cpp
index 724e4cde5..e6c97a26f 100644
--- a/glslang/MachineIndependent/ParseHelper.cpp
+++ b/glslang/MachineIndependent/ParseHelper.cpp
@@ -3936,6 +3936,18 @@ void TParseContext::accStructCheck(const TSourceLoc& loc, const TType& type, con
 
 }
 
+void TParseContext::hitObjectNVCheck(const TSourceLoc & loc, const TType & type, const TString & identifier)
+{
+    if (type.getBasicType() == EbtStruct && containsFieldWithBasicType(type, EbtHitObjectNV)) {
+        error(loc, "struct is not allowed to contain hitObjectNV:", type.getTypeName().c_str(), identifier.c_str());
+    } else if (type.getBasicType() == EbtHitObjectNV) {
+        TStorageQualifier qualifier = type.getQualifier().storage;
+        if (qualifier != EvqGlobal && qualifier != EvqTemporary) {
+            error(loc, "hitObjectNV can only be declared in global or function scope with no storage qualifier:", "hitObjectNV", identifier.c_str());
+        }
+    }
+}
+
 void TParseContext::transparentOpaqueCheck(const TSourceLoc& loc, const TType& type, const TString& identifier)
 {
     if (parsingBuiltins)
@@ -7875,6 +7887,7 @@ TIntermNode* TParseContext::declareVariable(const TSourceLoc& loc, TString& iden
     transparentOpaqueCheck(loc, type, identifier);
     atomicUintCheck(loc, type, identifier);
     accStructCheck(loc, type, identifier);
+    hitObjectNVCheck(loc, type, identifier);
     checkAndResizeMeshViewDim(loc, type, /*isBlockMember*/ false);
     if (type.getQualifier().storage == EvqConst && type.containsReference()) {
         error(loc, "variables with reference type can't have qualifier 'const'", "qualifier", "");
diff --git a/glslang/MachineIndependent/ParseHelper.h b/glslang/MachineIndependent/ParseHelper.h
index 16902aefe..67ba7dbd1 100644
--- a/glslang/MachineIndependent/ParseHelper.h
+++ b/glslang/MachineIndependent/ParseHelper.h
@@ -397,6 +397,7 @@ public:
     void samplerCheck(const TSourceLoc&, const TType&, const TString& identifier, TIntermTyped* initializer);
     void atomicUintCheck(const TSourceLoc&, const TType&, const TString& identifier);
     void accStructCheck(const TSourceLoc & loc, const TType & type, const TString & identifier);
+    void hitObjectNVCheck(const TSourceLoc & loc, const TType & type, const TString & identifier);
     void transparentOpaqueCheck(const TSourceLoc&, const TType&, const TString& identifier);
     void memberQualifierCheck(glslang::TPublicType&);
     void globalQualifierFixCheck(const TSourceLoc&, TQualifier&, bool isMemberCheck = false, const TPublicType* publicType = nullptr);
diff --git a/gtests/Spv.FromFile.cpp b/gtests/Spv.FromFile.cpp
index 21f12d020..4f820c162 100644
--- a/gtests/Spv.FromFile.cpp
+++ b/gtests/Spv.FromFile.cpp
@@ -710,6 +710,7 @@ INSTANTIATE_TEST_SUITE_P(
 
         // SPV_NV_shader_execution_reorder
 
+        "spv.nv.hitobject-errors.rgen",
         "spv.nv.hitobject-allops.rgen",
         "spv.nv.hitobject-allops.rchit",
         "spv.nv.hitobject-allops.rmiss",