kc3-lang/angle/src/compiler/translator/tree_ops/SeparateDeclarations.cpp

• Show log

  Commit

• Hash : 4397ff2f
  Author :
  Date : 2024-10-25T16:55:30
  

  Metal: SeparateCompoundStructDeclarations fails validation
  
  Consider GLSL:
    struct S { int i; } s;
    s=s;
  
  SeparateCompoundStructDeclarations would rewrite this to:
    struct S { int i; };
    S s';
    s=s;
  
  The interm rewrite would rewrite the specification and declaration of s,
  but not the use sites. The use sites would use the old type, and thus
  something that was not in the tree anymore. This would fail the
  validation.
  
  This kind of bug was previously fixed for SeparateDeclarations
  in commit 18fa02bebf901dd8501de3176f6052ae4ce984be.
  
  Fix by adding the logic to SeparateDeclarations, as it is already
  doing almost the exact task, separating `struct S { ..} a, b`.
  
  The separation is tested in GLSLTests.StructInShader and various
  other draw tests. These pass with MSL, but these would also fail
  validation if that was enabled.
  
  Bug: angleproject:375523825
  Change-Id: I1697103d0ba47616dbd3159f36f9e71cb2831c4b
  Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5964899
  Auto-Submit: Kimmo Kinnunen <kkinnunen@apple.com>
  Commit-Queue: Kimmo Kinnunen <kkinnunen@apple.com>
  Reviewed-by: Geoff Lang <geofflang@chromium.org>
  

Download

• src/compiler/translator/tree_ops/SeparateDeclarations.cpp

• //
  // Copyright 2002 The ANGLE Project Authors. All rights reserved.
  // Use of this source code is governed by a BSD-style license that can be
  // found in the LICENSE file.
  //
  #include "compiler/translator/tree_ops/SeparateDeclarations.h"
  
  #include <unordered_map>
  #include "compiler/translator/SymbolTable.h"
  
  #include "compiler/translator/IntermRebuild.h"
  #include "compiler/translator/util.h"
  
  namespace sh
  {
  namespace
  {
  
  class Separator final : private TIntermRebuild
  {
    public:
      Separator(TCompiler &compiler, bool separateCompoundStructDeclarations)
          : TIntermRebuild(compiler, true, true),
            mSeparateCompoundStructDeclarations(separateCompoundStructDeclarations)
      {}
      using TIntermRebuild::rebuildRoot;
  
    private:
      void recordModifiedStructVariables(TIntermDeclaration &node)
      {
          ASSERT(!mNewStructure);  // No nested struct declarations.
          TIntermSequence &sequence = *node.getSequence();
          if (sequence.size() <= 1 && !mSeparateCompoundStructDeclarations)
          {
              return;
          }
          TIntermTyped *declarator    = sequence.at(0)->getAsTyped();
          const TType &declaratorType = declarator->getType();
          const TStructure *structure = declaratorType.getStruct();
          // Rewrite variable declarations that specify structs AND variable(s) at the same
          // time.
          if (!structure || !declaratorType.isStructSpecifier())
          {
              return;
          }
          if (mSeparateCompoundStructDeclarations && sequence.size() == 1)
          {
              if (TIntermSymbol *symbol = declarator->getAsSymbolNode(); symbol != nullptr)
              {
                  if (symbol->variable().symbolType() == SymbolType::Empty)
                  {
                      return;
                  }
              }
          }
          // By default, struct specifier changes for all variables except the first one.
          uint32_t index = 1;
          if (structure->symbolType() == SymbolType::Empty)
          {
              TStructure *newStructure =
                  new TStructure(&mSymbolTable, kEmptyImmutableString, &structure->fields(),
                                 SymbolType::AngleInternal);
              newStructure->setAtGlobalScope(structure->atGlobalScope());
              structure     = newStructure;
              // Adding name causes the struct type to change, so also the first variable variable
              // needs rewriting.
              index = 0;
          }
          if (mSeparateCompoundStructDeclarations)
          {
              mNewStructure = structure;
              // Separating struct and variable declaration causes the variable type to change
              // from specifying to not-specifying, so also the first variable needs rewriting.
              index = 0;
          }
  
          for (; index < sequence.size(); ++index)
          {
              Declaration decl              = ViewDeclaration(node, index);
              const TVariable &var          = decl.symbol.variable();
              const TType &varType          = var.getType();
              const bool newTypeIsSpecifier = index == 0 && !mSeparateCompoundStructDeclarations;
              TType *newType                = new TType(structure, newTypeIsSpecifier);
              newType->setQualifier(varType.getQualifier());
              newType->makeArrays(varType.getArraySizes());
              TVariable *newVar = new TVariable(&mSymbolTable, var.name(), newType, var.symbolType());
              mStructVariables.insert(std::make_pair(&var, newVar));
          }
      }
  
      PreResult visitDeclarationPre(TIntermDeclaration &node) override
      {
          recordModifiedStructVariables(node);
          return node;
      }
  
      PostResult visitDeclarationPost(TIntermDeclaration &node) override
      {
          TIntermSequence &sequence = *node.getSequence();
          if (sequence.size() <= 1 && !mNewStructure)
          {
              return node;
          }
          std::vector<TIntermNode *> replacements;
          if (mNewStructure)
          {
              TType *newType = new TType(mNewStructure, true);
              if (mNewStructure->atGlobalScope())
              {
                  newType->setQualifier(EvqGlobal);
              }
              TVariable *structVar =
                  new TVariable(&mSymbolTable, kEmptyImmutableString, newType, SymbolType::Empty);
              TIntermDeclaration *replacement = new TIntermDeclaration({structVar});
              replacement->setLine(node.getLine());
              replacements.push_back(replacement);
              mNewStructure = nullptr;
          }
          for (uint32_t index = 0; index < sequence.size(); ++index)
          {
              TIntermTyped *declarator        = sequence.at(index)->getAsTyped();
              TIntermDeclaration *replacement = new TIntermDeclaration({declarator});
              replacement->setLine(declarator->getLine());
              replacements.push_back(replacement);
          }
          return PostResult::Multi(std::move(replacements));
      }
  
      PreResult visitSymbolPre(TIntermSymbol &symbolNode) override
      {
          auto it = mStructVariables.find(&symbolNode.variable());
          if (it == mStructVariables.end())
          {
              return symbolNode;
          }
          return *new TIntermSymbol(it->second);
      }
  
      const TStructure *mNewStructure = nullptr;
      // Old struct variable to new struct variable mapping.
      std::unordered_map<const TVariable *, TVariable *> mStructVariables;
      const bool mSeparateCompoundStructDeclarations;
  };
  
  }  // namespace
  
  bool SeparateDeclarations(TCompiler &compiler,
                            TIntermBlock &root,
                            bool separateCompoundStructDeclarations)
  {
      Separator separator(compiler, separateCompoundStructDeclarations);
      return separator.rebuildRoot(root);
  }
  
  }  // namespace sh
  

Download