Selaa lähdekoodia

Make assert consistent with the code (#5382)

There is an assert that checks if a variable will be placed in the
Function or Private storage class in the spir-v. However, the condition
checked in that assert is inconsistent with the part of the code that
determines which storage class a variable belongs to.

The fix is to make the function `isExternalVar` available outside of
SprivEmitter.cpp, and use that in the assert. Now the same check is used
everywhere.

Fixes #5273
Steven Perron 2 vuotta sitten
vanhempi
commit
e931c944ce

+ 1 - 1
tools/clang/lib/SPIRV/DeclResultIdMapper.cpp

@@ -4106,7 +4106,7 @@ QualType DeclResultIdMapper::getTypeAndCreateCounterForPotentialAliasVar(
   if (const auto *varDecl = dyn_cast<VarDecl>(decl)) {
     // This method is only intended to be used to create SPIR-V variables in the
     // Function or Private storage class.
-    assert(!varDecl->isExternallyVisible() || varDecl->isStaticDataMember());
+    assert(!SpirvEmitter::isExternalVar(varDecl));
   }
 
   const QualType type = getTypeOrFnRetType(decl);

+ 1 - 13
tools/clang/lib/SPIRV/SpirvEmitter.cpp

@@ -119,18 +119,6 @@ const Expr *isStructuredBufferLoad(const Expr *expr, const Expr **index) {
   return nullptr;
 }
 
-/// Returns true if the given VarDecl will be translated into a SPIR-V variable
-/// not in the Private or Function storage class.
-inline bool isExternalVar(const VarDecl *var) {
-  // Class static variables should be put in the Private storage class.
-  // groupshared variables are allowed to be declared as "static". But we still
-  // need to put them in the Workgroup storage class. That is, when seeing
-  // "static groupshared", ignore "static".
-  return var->hasExternalFormalLinkage()
-             ? !var->isStaticDataMember()
-             : (var->getAttr<HLSLGroupSharedAttr>() != nullptr);
-}
-
 /// Returns the referenced variable's DeclContext if the given expr is
 /// a DeclRefExpr referencing a ConstantBuffer/TextureBuffer. Otherwise,
 /// returns nullptr.
@@ -159,7 +147,7 @@ bool isReferencingNonAliasStructuredOrByteBuffer(const Expr *expr) {
   if (const auto *declRefExpr = dyn_cast<DeclRefExpr>(expr)) {
     if (const auto *varDecl = dyn_cast<VarDecl>(declRefExpr->getFoundDecl()))
       if (isAKindOfStructuredOrByteBuffer(varDecl->getType()))
-        return isExternalVar(varDecl);
+        return SpirvEmitter::isExternalVar(varDecl);
   } else if (const auto *callExpr = dyn_cast<CallExpr>(expr)) {
     if (isAKindOfStructuredOrByteBuffer(callExpr->getType()))
       return true;

+ 12 - 0
tools/clang/lib/SPIRV/SpirvEmitter.h

@@ -92,6 +92,18 @@ public:
                                QualType toType, SourceLocation,
                                SourceRange range = {});
 
+  /// Returns true if the given VarDecl will be translated into a SPIR-V
+  /// variable not in the Private or Function storage class.
+  static inline bool isExternalVar(const VarDecl *var) {
+    // Class static variables should be put in the Private storage class.
+    // groupshared variables are allowed to be declared as "static". But we
+    // still need to put them in the Workgroup storage class. That is, when
+    // seeing "static groupshared", ignore "static".
+    return var->hasExternalFormalLinkage()
+               ? !var->isStaticDataMember()
+               : (var->getAttr<HLSLGroupSharedAttr>() != nullptr);
+  }
+
 private:
   void doFunctionDecl(const FunctionDecl *decl);
   void doVarDecl(const VarDecl *decl);

+ 24 - 0
tools/clang/test/CodeGenSPIRV/template.static.var.hlsl

@@ -0,0 +1,24 @@
+// RUN: %dxc -HV 2021 -T cs_6_7 -E main
+
+// CHECK: [[static_var:%\w+]] = OpVariable %_ptr_Private_int Private
+// CHECK: [[init_var:%\w+]] = OpVariable %_ptr_Private_bool Private %false
+// CHECK: %test = OpFunction %int None
+// CHECK: [[init:%\w+]] = OpLoad %bool [[init_var]]
+// CHECK: OpBranchConditional [[init]] [[init_done_bb:%\w+]] [[do_init_bb:%\w+]]
+// CHECK: [[do_init_bb]] = OpLabel
+// CHECK: OpStore [[static_var]] %int_20
+// CHECK: OpStore [[init_var]] %true
+// CHECK: OpBranch [[init_done_bb]]
+// CHECK: [[init_done_bb]] = OpLabel
+// CHECK: OpLoad %int [[static_var]]
+
+
+template <typename R> R test(R x) {
+    static R v = 20;
+    return v * x;
+}
+
+[numthreads(32, 32, 1)] void main(uint2 threadId: SV_DispatchThreadID) {
+    float x = test(10);
+}
+

+ 1 - 0
tools/clang/unittests/SPIRV/CodeGenSpirvTest.cpp

@@ -230,6 +230,7 @@ TEST_F(FileTest, VarInitCrossStorageClass) {
 }
 TEST_F(FileTest, VarInitVec1) { runFileTest("var.init.vec.size.1.hlsl"); }
 TEST_F(FileTest, StaticVar) { runFileTest("var.static.hlsl"); }
+TEST_F(FileTest, TemplateStaticVar) { runFileTest("template.static.var.hlsl"); }
 TEST_F(FileTest, UninitStaticResourceVar) {
   runFileTest("var.static.resource.hlsl");
 }