Guarantee that symbol nodes get unique ids
The code is refactored so that symbol nodes can only be initialized
with an unique id object. This prevents accidentally forgetting to
create an id for a symbol node.
This opens up possibilities for future optimization: For example the
names and types of symbols could be stored in a central location
inside the SymbolTable, and TIntermSymbol nodes would only need to
store the symbol id. The symbol id could be used to look up the name
and type of the node.
BUG=angleproject:1490
TEST=angle_unittests
Change-Id: Ib8c8675d31493037a5a28c7b36bb9d1113cc10f6
Reviewed-on: https://chromium-review.googlesource.com/580955
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
diff --git a/src/compiler/translator/ArrayReturnValueToOutParameter.cpp b/src/compiler/translator/ArrayReturnValueToOutParameter.cpp
index db9bc54..17721fb 100644
--- a/src/compiler/translator/ArrayReturnValueToOutParameter.cpp
+++ b/src/compiler/translator/ArrayReturnValueToOutParameter.cpp
@@ -30,7 +30,7 @@
TIntermSymbol *CreateReturnValueSymbol(const TSymbolUniqueId &id, const TType &type)
{
- TIntermSymbol *node = new TIntermSymbol(id.get(), "angle_return", type);
+ TIntermSymbol *node = new TIntermSymbol(id, "angle_return", type);
node->setInternal(true);
node->getTypePointer()->setQualifier(EvqOut);
return node;
diff --git a/src/compiler/translator/DeclareAndInitBuiltinsForInstancedMultiview.cpp b/src/compiler/translator/DeclareAndInitBuiltinsForInstancedMultiview.cpp
index 7932a3e..ce9828f 100644
--- a/src/compiler/translator/DeclareAndInitBuiltinsForInstancedMultiview.cpp
+++ b/src/compiler/translator/DeclareAndInitBuiltinsForInstancedMultiview.cpp
@@ -132,7 +132,7 @@
new TIntermBinary(EOpAssign, viewportIndexSymbol, viewIDAsInt));
// Create a gl_Layer node.
- TIntermSymbol *layerSymbol = new TIntermSymbol(0, "gl_Layer", TType(EbtInt, EbpHigh, EvqLayer));
+ TIntermSymbol *layerSymbol = ReferenceBuiltInVariable("gl_Layer", symbolTable, 0);
// Create an int(ViewID_OVR) + multiviewBaseViewLayerIndex node
TIntermBinary *sumOfViewIDAndBaseViewIndex =
diff --git a/src/compiler/translator/Initialize.cpp b/src/compiler/translator/Initialize.cpp
index 769c93a..44bc662 100644
--- a/src/compiler/translator/Initialize.cpp
+++ b/src/compiler/translator/Initialize.cpp
@@ -964,6 +964,8 @@
// For internal use by ANGLE - not exposed to the parser.
symbolTable.insertVariable(GLSL_BUILTINS, "gl_ViewportIndex",
TType(EbtInt, EbpHigh, EvqViewportIndex));
+ // gl_Layer exists in other shader stages in ESSL, but not in vertex shader so far.
+ symbolTable.insertVariable(GLSL_BUILTINS, "gl_Layer", TType(EbtInt, EbpHigh, EvqLayer));
break;
}
case GL_COMPUTE_SHADER:
diff --git a/src/compiler/translator/IntermNode.h b/src/compiler/translator/IntermNode.h
index cee790b..1cb29f0 100644
--- a/src/compiler/translator/IntermNode.h
+++ b/src/compiler/translator/IntermNode.h
@@ -25,6 +25,7 @@
#include "compiler/translator/Common.h"
#include "compiler/translator/ConstantUnion.h"
#include "compiler/translator/Operator.h"
+#include "compiler/translator/SymbolUniqueId.h"
#include "compiler/translator/Types.h"
namespace sh
@@ -56,7 +57,6 @@
class TIntermBranch;
class TSymbolTable;
-class TSymbolUniqueId;
class TFunction;
// Encapsulate an identifier string and track whether it is coming from the original shader code
@@ -257,7 +257,7 @@
// if symbol is initialized as symbol(sym), the memory comes from the poolallocator of sym.
// If sym comes from per process globalpoolallocator, then it causes increased memory usage
// per compile it is essential to use "symbol = sym" to assign to symbol
- TIntermSymbol(int id, const TString &symbol, const TType &type)
+ TIntermSymbol(const TSymbolUniqueId &id, const TString &symbol, const TType &type)
: TIntermTyped(type), mId(id), mSymbol(symbol)
{
}
@@ -266,7 +266,7 @@
bool hasSideEffects() const override { return false; }
- int getId() const { return mId; }
+ int getId() const { return mId.get(); }
const TString &getSymbol() const { return mSymbol.getString(); }
const TName &getName() const { return mSymbol; }
TName &getName() { return mSymbol; }
@@ -278,7 +278,7 @@
bool replaceChildNode(TIntermNode *, TIntermNode *) override { return false; }
protected:
- const int mId;
+ const TSymbolUniqueId mId;
TName mSymbol;
private:
diff --git a/src/compiler/translator/IntermNode_util.cpp b/src/compiler/translator/IntermNode_util.cpp
index ed6a120..50c52f4 100644
--- a/src/compiler/translator/IntermNode_util.cpp
+++ b/src/compiler/translator/IntermNode_util.cpp
@@ -178,7 +178,7 @@
symbolNameOut << "s" << id.get();
TString symbolName = symbolNameOut.c_str();
- TIntermSymbol *node = new TIntermSymbol(id.get(), symbolName, type);
+ TIntermSymbol *node = new TIntermSymbol(id, symbolName, type);
node->setInternal(true);
ASSERT(qualifier == EvqTemporary || qualifier == EvqConst || qualifier == EvqGlobal);
diff --git a/src/compiler/translator/OutputHLSL.cpp b/src/compiler/translator/OutputHLSL.cpp
index 2fddbd3..b00ab78 100644
--- a/src/compiler/translator/OutputHLSL.cpp
+++ b/src/compiler/translator/OutputHLSL.cpp
@@ -119,8 +119,9 @@
ShShaderOutput outputType,
int numRenderTargets,
const std::vector<Uniform> &uniforms,
- ShCompileOptions compileOptions)
- : TIntermTraverser(true, true, true),
+ ShCompileOptions compileOptions,
+ TSymbolTable *symbolTable)
+ : TIntermTraverser(true, true, true, symbolTable),
mShaderType(shaderType),
mShaderVersion(shaderVersion),
mExtensionBehavior(extensionBehavior),
@@ -375,7 +376,7 @@
out << mStructureHLSL->structsHeader();
- mUniformHLSL->uniformsHeader(out, mOutputType, mReferencedUniforms);
+ mUniformHLSL->uniformsHeader(out, mOutputType, mReferencedUniforms, mSymbolTable);
out << mUniformHLSL->uniformBlocksHeader(mReferencedUniformBlocks);
if (!mEqualityFunctions.empty())
@@ -1895,8 +1896,8 @@
const TType &argType = typedArg->getType();
TVector<TIntermSymbol *> samplerSymbols;
TString structName = samplerNamePrefixFromStruct(typedArg);
- argType.createSamplerSymbols("angle_" + structName, "",
- &samplerSymbols, nullptr);
+ argType.createSamplerSymbols("angle_" + structName, "", &samplerSymbols,
+ nullptr, mSymbolTable);
for (const TIntermSymbol *sampler : samplerSymbols)
{
if (mOutputType == SH_HLSL_4_0_FL9_3_OUTPUT)
@@ -2626,7 +2627,7 @@
{
ASSERT(qualifier != EvqOut && qualifier != EvqInOut);
TVector<TIntermSymbol *> samplerSymbols;
- type.createSamplerSymbols("angle" + nameStr, "", &samplerSymbols, nullptr);
+ type.createSamplerSymbols("angle" + nameStr, "", &samplerSymbols, nullptr, mSymbolTable);
for (const TIntermSymbol *sampler : samplerSymbols)
{
const TType &samplerType = sampler->getType();
diff --git a/src/compiler/translator/OutputHLSL.h b/src/compiler/translator/OutputHLSL.h
index 3d1f978..e0e8cb8 100644
--- a/src/compiler/translator/OutputHLSL.h
+++ b/src/compiler/translator/OutputHLSL.h
@@ -22,6 +22,7 @@
{
class StructureHLSL;
class TextureFunctionHLSL;
+class TSymbolTable;
class UnfoldShortCircuit;
class UniformHLSL;
@@ -37,7 +38,8 @@
ShShaderOutput outputType,
int numRenderTargets,
const std::vector<Uniform> &uniforms,
- ShCompileOptions compileOptions);
+ ShCompileOptions compileOptions,
+ TSymbolTable *symbolTable);
~OutputHLSL();
diff --git a/src/compiler/translator/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp
index 8a76153..e5f9f59 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -2311,7 +2311,7 @@
// But if the empty declaration is declaring a struct type, the symbol node will store that.
if (type.getBasicType() == EbtStruct)
{
- symbol = new TIntermSymbol(0, "", type);
+ symbol = new TIntermSymbol(symbolTable.getEmptySymbolId(), "", type);
}
else if (IsAtomicCounter(publicType.getBasicType()))
{
@@ -3075,7 +3075,7 @@
{
// The parameter had no name or declaring the symbol failed - either way, add a nameless
// symbol.
- symbol = new TIntermSymbol(0, "", *param.type);
+ symbol = new TIntermSymbol(symbolTable.getEmptySymbolId(), "", *param.type);
}
symbol->setLine(location);
prototype->appendParameter(symbol);
@@ -3637,7 +3637,7 @@
}
TString symbolName = "";
- int symbolId = 0;
+ const TSymbolUniqueId *symbolId = nullptr;
if (!instanceName)
{
@@ -3662,6 +3662,7 @@
field->name().c_str());
}
}
+ symbolId = &symbolTable.getEmptySymbolId();
}
else
{
@@ -3672,7 +3673,7 @@
if (instanceTypeDef)
{
instanceTypeDef->setQualifier(typeQualifier.qualifier);
- symbolId = instanceTypeDef->getUniqueId();
+ symbolId = &instanceTypeDef->getUniqueId();
}
else
{
@@ -3682,11 +3683,16 @@
symbolName = *instanceName;
}
- TIntermSymbol *blockSymbol = new TIntermSymbol(symbolId, symbolName, interfaceBlockType);
- blockSymbol->setLine(typeQualifier.line);
- TIntermDeclaration *declaration = new TIntermDeclaration();
- declaration->appendDeclarator(blockSymbol);
- declaration->setLine(nameLine);
+ TIntermDeclaration *declaration = nullptr;
+
+ if (symbolId)
+ {
+ TIntermSymbol *blockSymbol = new TIntermSymbol(*symbolId, symbolName, interfaceBlockType);
+ blockSymbol->setLine(typeQualifier.line);
+ declaration = new TIntermDeclaration();
+ declaration->appendDeclarator(blockSymbol);
+ declaration->setLine(nameLine);
+ }
exitStructDeclaration();
return declaration;
diff --git a/src/compiler/translator/SymbolTable.cpp b/src/compiler/translator/SymbolTable.cpp
index 7885df7..b26acc8 100644
--- a/src/compiler/translator/SymbolTable.cpp
+++ b/src/compiler/translator/SymbolTable.cpp
@@ -29,19 +29,6 @@
} // anonymous namespace
-TSymbolUniqueId::TSymbolUniqueId(TSymbolTable *symbolTable) : mId(symbolTable->nextUniqueId())
-{
-}
-
-TSymbolUniqueId::TSymbolUniqueId(const TSymbol &symbol) : mId(symbol.getUniqueId())
-{
-}
-
-int TSymbolUniqueId::get() const
-{
- return mId;
-}
-
TSymbol::TSymbol(TSymbolTable *symbolTable, const TString *n)
: uniqueId(symbolTable->nextUniqueId()), name(n), extension(TExtension::UNDEFINED)
{
diff --git a/src/compiler/translator/SymbolTable.h b/src/compiler/translator/SymbolTable.h
index bcd51fd..21b2b15 100644
--- a/src/compiler/translator/SymbolTable.h
+++ b/src/compiler/translator/SymbolTable.h
@@ -38,26 +38,11 @@
#include "compiler/translator/ExtensionBehavior.h"
#include "compiler/translator/InfoSink.h"
#include "compiler/translator/IntermNode.h"
+#include "compiler/translator/SymbolUniqueId.h"
namespace sh
{
-// Encapsulates a unique id for a symbol.
-class TSymbolUniqueId
-{
- public:
- POOL_ALLOCATOR_NEW_DELETE();
- TSymbolUniqueId(TSymbolTable *symbolTable);
- TSymbolUniqueId(const TSymbol &symbol);
- TSymbolUniqueId(const TSymbolUniqueId &) = default;
- TSymbolUniqueId &operator=(const TSymbolUniqueId &) = default;
-
- int get() const;
-
- private:
- int mId;
-};
-
// Symbol base class. (Can build functions or variables out of these...)
class TSymbol : angle::NonCopyable
{
@@ -74,12 +59,12 @@
virtual const TString &getMangledName() const { return getName(); }
virtual bool isFunction() const { return false; }
virtual bool isVariable() const { return false; }
- int getUniqueId() const { return uniqueId; }
+ const TSymbolUniqueId &getUniqueId() const { return uniqueId; }
void relateToExtension(TExtension ext) { extension = ext; }
TExtension getExtension() const { return extension; }
private:
- const int uniqueId;
+ const TSymbolUniqueId uniqueId;
const TString *name;
TExtension extension;
};
@@ -302,7 +287,7 @@
class TSymbolTable : angle::NonCopyable
{
public:
- TSymbolTable() : mUniqueIdCounter(0)
+ TSymbolTable() : mUniqueIdCounter(0), mEmptySymbolId(this)
{
// The symbol table cannot be used until push() is called, but
// the lack of an initial call to push() can be used to detect
@@ -514,12 +499,20 @@
table[currentLevel()]->setGlobalInvariant(invariant);
}
- int nextUniqueId() { return ++mUniqueIdCounter; }
+ const TSymbolUniqueId nextUniqueId() { return TSymbolUniqueId(this); }
+
+ // The empty symbol id is shared between all empty string ("") symbols. They are used in the
+ // AST for unused function parameters and struct type declarations that don't declare a
+ // variable, for example.
+ const TSymbolUniqueId &getEmptySymbolId() { return mEmptySymbolId; }
// Checks whether there is a built-in accessible by a shader with the specified version.
bool hasUnmangledBuiltInForShaderVersion(const char *name, int shaderVersion);
private:
+ friend class TSymbolUniqueId;
+ int nextUniqueIdValue() { return ++mUniqueIdCounter; }
+
ESymbolLevel currentLevel() const { return static_cast<ESymbolLevel>(table.size() - 1); }
TVariable *insertVariable(ESymbolLevel level, const TString *name, const TType &type);
@@ -543,6 +536,8 @@
std::vector<PrecisionStackLevel *> precisionStack;
int mUniqueIdCounter;
+
+ const TSymbolUniqueId mEmptySymbolId;
};
} // namespace sh
diff --git a/src/compiler/translator/SymbolUniqueId.cpp b/src/compiler/translator/SymbolUniqueId.cpp
new file mode 100644
index 0000000..2a68ae3
--- /dev/null
+++ b/src/compiler/translator/SymbolUniqueId.cpp
@@ -0,0 +1,28 @@
+//
+// Copyright (c) 2017 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.
+//
+// SymbolUniqueId.cpp: Encapsulates a unique id for a symbol.
+
+#include "compiler/translator/SymbolUniqueId.h"
+
+#include "compiler/translator/SymbolTable.h"
+
+namespace sh
+{
+
+TSymbolUniqueId::TSymbolUniqueId(TSymbolTable *symbolTable) : mId(symbolTable->nextUniqueIdValue())
+{
+}
+
+TSymbolUniqueId::TSymbolUniqueId(const TSymbol &symbol) : mId(symbol.getUniqueId().get())
+{
+}
+
+int TSymbolUniqueId::get() const
+{
+ return mId;
+}
+
+} // namespace sh
diff --git a/src/compiler/translator/SymbolUniqueId.h b/src/compiler/translator/SymbolUniqueId.h
new file mode 100644
index 0000000..4bd5604
--- /dev/null
+++ b/src/compiler/translator/SymbolUniqueId.h
@@ -0,0 +1,36 @@
+//
+// Copyright (c) 2017 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.
+//
+// SymbolUniqueId.h: Encapsulates a unique id for a symbol.
+
+#ifndef COMPILER_TRANSLATOR_SYMBOLUNIQUEID_H_
+#define COMPILER_TRANSLATOR_SYMBOLUNIQUEID_H_
+
+#include "compiler/translator/Common.h"
+
+namespace sh
+{
+
+class TSymbolTable;
+class TSymbol;
+
+class TSymbolUniqueId
+{
+ public:
+ POOL_ALLOCATOR_NEW_DELETE();
+ explicit TSymbolUniqueId(TSymbolTable *symbolTable);
+ explicit TSymbolUniqueId(const TSymbol &symbol);
+ TSymbolUniqueId(const TSymbolUniqueId &) = default;
+ TSymbolUniqueId &operator=(const TSymbolUniqueId &) = default;
+
+ int get() const;
+
+ private:
+ int mId;
+};
+
+} // namespace sh
+
+#endif // COMPILER_TRANSLATOR_SYMBOLUNIQUEID_H_
diff --git a/src/compiler/translator/TranslatorHLSL.cpp b/src/compiler/translator/TranslatorHLSL.cpp
index 4b40ee0..ddf25be 100644
--- a/src/compiler/translator/TranslatorHLSL.cpp
+++ b/src/compiler/translator/TranslatorHLSL.cpp
@@ -126,7 +126,7 @@
sh::OutputHLSL outputHLSL(getShaderType(), getShaderVersion(), getExtensionBehavior(),
getSourcePath(), getOutputType(), numRenderTargets, getUniforms(),
- compileOptions);
+ compileOptions, &getSymbolTable());
outputHLSL.output(root, getInfoSink().obj);
diff --git a/src/compiler/translator/Types.cpp b/src/compiler/translator/Types.cpp
index b252512..c20e335 100644
--- a/src/compiler/translator/Types.cpp
+++ b/src/compiler/translator/Types.cpp
@@ -621,7 +621,8 @@
void TType::createSamplerSymbols(const TString &namePrefix,
const TString &apiNamePrefix,
TVector<TIntermSymbol *> *outputSymbols,
- TMap<TIntermSymbol *, TString> *outputSymbolsToAPINames) const
+ TMap<TIntermSymbol *, TString> *outputSymbolsToAPINames,
+ TSymbolTable *symbolTable) const
{
if (isStructureContainingSamplers())
{
@@ -636,18 +637,20 @@
TStringStream elementApiName;
elementApiName << apiNamePrefix << "[" << arrayIndex << "]";
elementType.createSamplerSymbols(elementName.str(), elementApiName.str(),
- outputSymbols, outputSymbolsToAPINames);
+ outputSymbols, outputSymbolsToAPINames,
+ symbolTable);
}
}
else
{
structure->createSamplerSymbols(namePrefix, apiNamePrefix, outputSymbols,
- outputSymbolsToAPINames);
+ outputSymbolsToAPINames, symbolTable);
}
return;
}
+
ASSERT(IsSampler(type));
- TIntermSymbol *symbol = new TIntermSymbol(0, namePrefix, *this);
+ TIntermSymbol *symbol = new TIntermSymbol(symbolTable->nextUniqueId(), namePrefix, *this);
outputSymbols->push_back(symbol);
if (outputSymbolsToAPINames)
{
@@ -658,7 +661,8 @@
void TStructure::createSamplerSymbols(const TString &namePrefix,
const TString &apiNamePrefix,
TVector<TIntermSymbol *> *outputSymbols,
- TMap<TIntermSymbol *, TString> *outputSymbolsToAPINames) const
+ TMap<TIntermSymbol *, TString> *outputSymbolsToAPINames,
+ TSymbolTable *symbolTable) const
{
ASSERT(containsSamplers());
for (auto &field : *mFields)
@@ -669,7 +673,7 @@
TString fieldName = namePrefix + "_" + field->name();
TString fieldApiName = apiNamePrefix + "." + field->name();
fieldType->createSamplerSymbols(fieldName, fieldApiName, outputSymbols,
- outputSymbolsToAPINames);
+ outputSymbolsToAPINames, symbolTable);
}
}
}
diff --git a/src/compiler/translator/Types.h b/src/compiler/translator/Types.h
index eaa5479..d90e926 100644
--- a/src/compiler/translator/Types.h
+++ b/src/compiler/translator/Types.h
@@ -12,6 +12,7 @@
#include "compiler/translator/BaseTypes.h"
#include "compiler/translator/Common.h"
+#include "compiler/translator/SymbolUniqueId.h"
namespace sh
{
@@ -103,15 +104,12 @@
void createSamplerSymbols(const TString &namePrefix,
const TString &apiNamePrefix,
TVector<TIntermSymbol *> *outputSymbols,
- TMap<TIntermSymbol *, TString> *outputSymbolsToAPINames) const;
+ TMap<TIntermSymbol *, TString> *outputSymbolsToAPINames,
+ TSymbolTable *symbolTable) const;
bool equals(const TStructure &other) const;
- int uniqueId() const
- {
- ASSERT(mUniqueId != 0);
- return mUniqueId;
- }
+ int uniqueId() const { return mUniqueId.get(); }
void setAtGlobalScope(bool atGlobalScope) { mAtGlobalScope = atGlobalScope; }
@@ -138,7 +136,7 @@
int calculateDeepestNesting() const;
mutable int mDeepestNesting;
- const int mUniqueId;
+ const TSymbolUniqueId mUniqueId;
bool mAtGlobalScope;
};
@@ -478,7 +476,8 @@
void createSamplerSymbols(const TString &namePrefix,
const TString &apiNamePrefix,
TVector<TIntermSymbol *> *outputSymbols,
- TMap<TIntermSymbol *, TString> *outputSymbolsToAPINames) const;
+ TMap<TIntermSymbol *, TString> *outputSymbolsToAPINames,
+ TSymbolTable *symbolTable) const;
// Initializes all lazily-initialized members.
void realize() { getMangledName(); }
diff --git a/src/compiler/translator/UniformHLSL.cpp b/src/compiler/translator/UniformHLSL.cpp
index 07badbb..f61fca2 100644
--- a/src/compiler/translator/UniformHLSL.cpp
+++ b/src/compiler/translator/UniformHLSL.cpp
@@ -278,7 +278,8 @@
void UniformHLSL::uniformsHeader(TInfoSinkBase &out,
ShShaderOutput outputType,
- const ReferencedSymbols &referencedUniforms)
+ const ReferencedSymbols &referencedUniforms,
+ TSymbolTable *symbolTable)
{
if (!referencedUniforms.empty())
{
@@ -313,7 +314,7 @@
TVector<TIntermSymbol *> samplerSymbols;
TMap<TIntermSymbol *, TString> symbolsToAPINames;
type.createSamplerSymbols("angle_" + name.getString(), name.getString(),
- &samplerSymbols, &symbolsToAPINames);
+ &samplerSymbols, &symbolsToAPINames, symbolTable);
for (TIntermSymbol *sampler : samplerSymbols)
{
const TType &samplerType = sampler->getType();
diff --git a/src/compiler/translator/UniformHLSL.h b/src/compiler/translator/UniformHLSL.h
index 05474c2..9c54f90 100644
--- a/src/compiler/translator/UniformHLSL.h
+++ b/src/compiler/translator/UniformHLSL.h
@@ -16,6 +16,7 @@
namespace sh
{
class StructureHLSL;
+class TSymbolTable;
class UniformHLSL : angle::NonCopyable
{
@@ -28,7 +29,8 @@
void reserveUniformBlockRegisters(unsigned int registerCount);
void uniformsHeader(TInfoSinkBase &out,
ShShaderOutput outputType,
- const ReferencedSymbols &referencedUniforms);
+ const ReferencedSymbols &referencedUniforms,
+ TSymbolTable *symbolTable);
// Must be called after uniformsHeader
void samplerMetadataUniforms(TInfoSinkBase &out, const char *reg);