Browse Source

[spirv] Unify checks for skipping struct fields in layout (#1164)

Lei Zhang 7 years ago
parent
commit
01fb45c865

+ 2 - 12
tools/clang/lib/SPIRV/DeclResultIdMapper.cpp

@@ -491,8 +491,7 @@ uint32_t DeclResultIdMapper::createVarOfExplicitLayoutStruct(
   const auto *blockDec = forTBuffer ? Decoration::getBufferBlock(context)
   const auto *blockDec = forTBuffer ? Decoration::getBufferBlock(context)
                                     : Decoration::getBlock(context);
                                     : Decoration::getBlock(context);
 
 
-  auto decorations =
-      typeTranslator.getLayoutDecorations(decl, layoutRule, forGlobals);
+  auto decorations = typeTranslator.getLayoutDecorations(decl, layoutRule);
   decorations.push_back(blockDec);
   decorations.push_back(blockDec);
 
 
   // Collect the type and name for each field
   // Collect the type and name for each field
@@ -508,12 +507,6 @@ uint32_t DeclResultIdMapper::createVarOfExplicitLayoutStruct(
     assert(isa<VarDecl>(subDecl) || isa<FieldDecl>(subDecl));
     assert(isa<VarDecl>(subDecl) || isa<FieldDecl>(subDecl));
     const auto *declDecl = cast<DeclaratorDecl>(subDecl);
     const auto *declDecl = cast<DeclaratorDecl>(subDecl);
 
 
-    // If we are creating the $Globals cbuffer, we only care about
-    // externally-visiable non-resource-type variables.
-    if (forGlobals && (!declDecl->hasExternalFormalLinkage() ||
-                       TypeTranslator::isResourceType(declDecl)))
-      continue;
-
     // All fields are qualified with const. It will affect the debug name.
     // All fields are qualified with const. It will affect the debug name.
     // We don't need it here.
     // We don't need it here.
     auto varType = declDecl->getType();
     auto varType = declDecl->getType();
@@ -638,10 +631,7 @@ void DeclResultIdMapper::createGlobalsCBuffer(const VarDecl *var) {
   uint32_t index = 0;
   uint32_t index = 0;
   for (const auto *decl : context->decls())
   for (const auto *decl : context->decls())
     if (const auto *varDecl = dyn_cast<VarDecl>(decl)) {
     if (const auto *varDecl = dyn_cast<VarDecl>(decl)) {
-      // We are only interested in explicitly-defined externally-visible
-      // variables here.
-      if (varDecl->isImplicit() || !varDecl->hasExternalFormalLinkage() ||
-          TypeTranslator::isResourceType(varDecl))
+      if (TypeTranslator::shouldSkipInStructLayout(varDecl))
         continue;
         continue;
 
 
       if (const auto *attr = varDecl->getAttr<VKBindingAttr>()) {
       if (const auto *attr = varDecl->getAttr<VKBindingAttr>()) {

+ 29 - 10
tools/clang/lib/SPIRV/TypeTranslator.cpp

@@ -1130,15 +1130,40 @@ bool TypeTranslator::shouldSkipInStructLayout(const Decl *decl) {
 
 
   // For $Globals (whose "struct" is the TranslationUnit)
   // For $Globals (whose "struct" is the TranslationUnit)
   // Ignore resources in the TranslationUnit "struct"
   // Ignore resources in the TranslationUnit "struct"
-  if (decl->getDeclContext() == decl->getTranslationUnitDecl())
-    return isa<HLSLBufferDecl>(decl);
+
+  // For the $Globals cbuffer, we only care about externally-visiable
+  // non-resource-type variables. The rest should be filtered out.
+
+  // Special check for ConstantBuffer/TextureBuffer, whose DeclContext is a
+  // HLSLBufferDecl. So that we need to check the HLSLBufferDecl's parent decl
+  // to check whether this is a ConstantBuffer/TextureBuffer defined in the
+  // global namespace.
+  if (isConstantTextureBuffer(decl) &&
+      decl->getDeclContext()->getLexicalParent()->isTranslationUnit())
+    return true;
+
+  // For others we can check their DeclContext directly.
+  if (decl->getDeclContext()->isTranslationUnit()) {
+    // External visibility
+    if (const auto *declDecl = dyn_cast<DeclaratorDecl>(decl))
+      if (!declDecl->hasExternalFormalLinkage())
+        return true;
+
+    // cbuffer/tbuffer
+    if (isa<HLSLBufferDecl>(decl))
+      return true;
+
+    // Other resource types
+    if (const auto *valueDecl = dyn_cast<ValueDecl>(decl))
+      if (isResourceType(valueDecl))
+        return true;
+  }
 
 
   return false;
   return false;
 }
 }
 
 
 llvm::SmallVector<const Decoration *, 4>
 llvm::SmallVector<const Decoration *, 4>
-TypeTranslator::getLayoutDecorations(const DeclContext *decl, LayoutRule rule,
-                                     bool forGlobals) {
+TypeTranslator::getLayoutDecorations(const DeclContext *decl, LayoutRule rule) {
   const auto spirvContext = theBuilder.getSPIRVContext();
   const auto spirvContext = theBuilder.getSPIRVContext();
   llvm::SmallVector<const Decoration *, 4> decorations;
   llvm::SmallVector<const Decoration *, 4> decorations;
   uint32_t offset = 0, index = 0;
   uint32_t offset = 0, index = 0;
@@ -1152,12 +1177,6 @@ TypeTranslator::getLayoutDecorations(const DeclContext *decl, LayoutRule rule,
     const auto *declDecl = cast<DeclaratorDecl>(field);
     const auto *declDecl = cast<DeclaratorDecl>(field);
     auto fieldType = declDecl->getType();
     auto fieldType = declDecl->getType();
 
 
-    // If we are creating the $Globals cbuffer, we only care about
-    // externally-visiable non-resource-type variables.
-    if (forGlobals &&
-        (!declDecl->hasExternalFormalLinkage() || isResourceType(declDecl)))
-      continue;
-
     uint32_t memberAlignment = 0, memberSize = 0, stride = 0;
     uint32_t memberAlignment = 0, memberSize = 0, stride = 0;
     std::tie(memberAlignment, memberSize) =
     std::tie(memberAlignment, memberSize) =
         getAlignmentAndSize(fieldType, rule, &stride);
         getAlignmentAndSize(fieldType, rule, &stride);

+ 7 - 2
tools/clang/lib/SPIRV/TypeTranslator.h

@@ -192,6 +192,12 @@ public:
   static bool isConstantTextureBuffer(const Decl *decl);
   static bool isConstantTextureBuffer(const Decl *decl);
 
 
   /// \brief Returns true if the decl will have a SPIR-V resource type.
   /// \brief Returns true if the decl will have a SPIR-V resource type.
+  ///
+  /// Note that this function covers the following HLSL types:
+  /// * ConstantBuffer/TextureBuffer
+  /// * Various structured buffers
+  /// * (RW)ByteAddressBuffer
+  /// * SubpassInput(MS)
   static bool isResourceType(const ValueDecl *decl);
   static bool isResourceType(const ValueDecl *decl);
 
 
   /// \brief Returns true if the two types are the same scalar or vector type,
   /// \brief Returns true if the two types are the same scalar or vector type,
@@ -249,8 +255,7 @@ public:
   /// according to the spec, must be attached to the array type itself instead
   /// according to the spec, must be attached to the array type itself instead
   /// of a struct member.
   /// of a struct member.
   llvm::SmallVector<const Decoration *, 4>
   llvm::SmallVector<const Decoration *, 4>
-  getLayoutDecorations(const DeclContext *decl, LayoutRule rule,
-                       bool forGlobals = false);
+  getLayoutDecorations(const DeclContext *decl, LayoutRule rule);
 
 
   /// \brief Returns how many sequential locations are consumed by a given type.
   /// \brief Returns how many sequential locations are consumed by a given type.
   uint32_t getLocationCount(QualType type);
   uint32_t getLocationCount(QualType type);