Skip to content

Commit

Permalink
Fix issue for new unique id system. Add level bits to help verifying …
Browse files Browse the repository at this point in the history
…symbols and split symbol tables.

For intermediates rebuilding, now need manually amending level bits for redeclaring built-ins.
  • Loading branch information
ShchchowAMD committed Feb 15, 2021
1 parent 74e8f05 commit 93b400f
Show file tree
Hide file tree
Showing 17 changed files with 110 additions and 71 deletions.
10 changes: 5 additions & 5 deletions SPIRV/GlslangToSpv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -255,15 +255,15 @@ class TGlslangToSpvTraverser : public glslang::TIntermTraverser {
spv::Id nonSemanticDebugPrintf;
std::unordered_map<const char*, spv::Id> extBuiltinMap;

std::unordered_map<int, spv::Id> symbolValues;
std::unordered_set<int> rValueParameters; // set of formal function parameters passed as rValues,
std::unordered_map<long long, spv::Id> symbolValues;
std::unordered_set<long long> rValueParameters; // set of formal function parameters passed as rValues,
// rather than a pointer
std::unordered_map<std::string, spv::Function*> functionMap;
std::unordered_map<const glslang::TTypeList*, spv::Id> structMap[glslang::ElpCount][glslang::ElmCount];
// for mapping glslang block indices to spv indices (e.g., due to hidden members):
std::unordered_map<int, std::vector<int>> memberRemapper;
std::unordered_map<long long, std::vector<int>> memberRemapper;
// for mapping glslang symbol struct to symbol Id
std::unordered_map<const glslang::TTypeList*, int> glslangTypeToIdMap;
std::unordered_map<const glslang::TTypeList*, long long> glslangTypeToIdMap;
std::stack<bool> breakForLoop; // false means break for switch
std::unordered_map<std::string, const glslang::TIntermSymbol*> counterOriginator;
// Map pointee types for EbtReference to their forward pointers
Expand Down Expand Up @@ -1943,7 +1943,7 @@ bool TGlslangToSpvTraverser::visitBinary(glslang::TVisit /* visit */, glslang::T
{
// This may be, e.g., an anonymous block-member selection, which generally need
// index remapping due to hidden members in anonymous blocks.
int glslangId = glslangTypeToIdMap[node->getLeft()->getType().getStruct()];
long long glslangId = glslangTypeToIdMap[node->getLeft()->getType().getStruct()];
if (memberRemapper.find(glslangId) != memberRemapper.end()) {
std::vector<int>& remapper = memberRemapper[glslangId];
assert(remapper.size() > 0);
Expand Down
4 changes: 2 additions & 2 deletions Test/baseResults/link.vk.multiBlocksValid.1.0.geom.out
Original file line number Diff line number Diff line change
Expand Up @@ -328,14 +328,14 @@ output primitive = triangle_strip
Decorate 59(Vertex) Block
Decorate 61(oV) Location 0
Decorate 64(Vertex) Block
Decorate 68(iV) Location 1
Decorate 68(iV) Location 0
MemberDecorate 95(BufferBlock) 0 ColMajor
MemberDecorate 95(BufferBlock) 0 Offset 0
MemberDecorate 95(BufferBlock) 0 MatrixStride 16
Decorate 95(BufferBlock) BufferBlock
Decorate 97(uBuf) DescriptorSet 0
Decorate 97(uBuf) Binding 1
Decorate 100(P) Location 0
Decorate 100(P) Location 2
2: TypeVoid
3: TypeFunction 2
6: TypeFloat 32
Expand Down
6 changes: 3 additions & 3 deletions Test/baseResults/spv.specConstant.vert.out
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ spv.specConstant.vert
Source GLSL 400
Name 4 "main"
Name 9 "arraySize"
Name 14 "foo(vf4[s805310914];"
Name 14 "foo(vf4[s216172782];"
Name 13 "p"
Name 17 "builtin_spec_constant("
Name 20 "color"
Expand Down Expand Up @@ -106,10 +106,10 @@ spv.specConstant.vert
Store 20(color) 46
48: 10 Load 22(ucol)
Store 47(param) 48
49: 2 FunctionCall 14(foo(vf4[s805310914];) 47(param)
49: 2 FunctionCall 14(foo(vf4[s216172782];) 47(param)
Return
FunctionEnd
14(foo(vf4[s805310914];): 2 Function None 12
14(foo(vf4[s216172782];): 2 Function None 12
13(p): 11(ptr) FunctionParameter
15: Label
54: 24(ptr) AccessChain 53(dupUcol) 23
Expand Down
6 changes: 3 additions & 3 deletions glslang/HLSL/hlslParseHelper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1376,7 +1376,7 @@ TIntermTyped* HlslParseContext::flattenAccess(TIntermTyped* base, int member)

return flattened ? flattened : base;
}
TIntermTyped* HlslParseContext::flattenAccess(int uniqueId, int member, TStorageQualifier outerStorage,
TIntermTyped* HlslParseContext::flattenAccess(long long uniqueId, int member, TStorageQualifier outerStorage,
const TType& dereferencedType, int subset)
{
const auto flattenData = flattenMap.find(uniqueId);
Expand Down Expand Up @@ -1444,7 +1444,7 @@ int HlslParseContext::findSubtreeOffset(const TType& type, int subset, const TVe
};

// Find and return the split IO TVariable for id, or nullptr if none.
TVariable* HlslParseContext::getSplitNonIoVar(int id) const
TVariable* HlslParseContext::getSplitNonIoVar(long long id) const
{
const auto splitNonIoVar = splitNonIoVars.find(id);
if (splitNonIoVar == splitNonIoVars.end())
Expand Down Expand Up @@ -3256,7 +3256,7 @@ TIntermAggregate* HlslParseContext::handleSamplerTextureCombine(const TSourceLoc
// shadow state. This depends on downstream optimization to
// DCE one variant in [shadow, nonshadow] if both are present,
// or the SPIR-V module would be invalid.
int newId = texSymbol->getId();
long long newId = texSymbol->getId();

// Check to see if this texture has been given a shadow mode already.
// If so, look up the one we already have.
Expand Down
22 changes: 11 additions & 11 deletions glslang/HLSL/hlslParseHelper.h
Original file line number Diff line number Diff line change
Expand Up @@ -253,12 +253,12 @@ class HlslParseContext : public TParseContextBase {

// Array and struct flattening
TIntermTyped* flattenAccess(TIntermTyped* base, int member);
TIntermTyped* flattenAccess(int uniqueId, int member, TStorageQualifier outerStorage, const TType&, int subset = -1);
TIntermTyped* flattenAccess(long long uniqueId, int member, TStorageQualifier outerStorage, const TType&, int subset = -1);
int findSubtreeOffset(const TIntermNode&) const;
int findSubtreeOffset(const TType&, int subset, const TVector<int>& offsets) const;
bool shouldFlatten(const TType&, TStorageQualifier, bool topLevel) const;
bool wasFlattened(const TIntermTyped* node) const;
bool wasFlattened(int id) const { return flattenMap.find(id) != flattenMap.end(); }
bool wasFlattened(long long id) const { return flattenMap.find(id) != flattenMap.end(); }
int addFlattenedMember(const TVariable&, const TType&, TFlattenData&, const TString& name, bool linkage,
const TQualifier& outerQualifier, const TArraySizes* builtInArraySizes);

Expand All @@ -267,8 +267,8 @@ class HlslParseContext : public TParseContextBase {
void splitBuiltIn(const TString& baseName, const TType& memberType, const TArraySizes*, const TQualifier&);
const TType& split(const TType& type, const TString& name, const TQualifier&);
bool wasSplit(const TIntermTyped* node) const;
bool wasSplit(int id) const { return splitNonIoVars.find(id) != splitNonIoVars.end(); }
TVariable* getSplitNonIoVar(int id) const;
bool wasSplit(long long id) const { return splitNonIoVars.find(id) != splitNonIoVars.end(); }
TVariable* getSplitNonIoVar(long long id) const;
void addPatchConstantInvocation();
void fixTextureShadowModes();
void finalizeAppendMethods();
Expand Down Expand Up @@ -386,7 +386,7 @@ class HlslParseContext : public TParseContextBase {
//
TVector<TSymbol*> ioArraySymbolResizeList;

TMap<int, TFlattenData> flattenMap;
TMap<long long, TFlattenData> flattenMap;

// IO-type map. Maps a pure symbol-table form of a structure-member list into
// each of the (up to) three kinds of IO, as each as different allowed decorations,
Expand All @@ -399,7 +399,7 @@ class HlslParseContext : public TParseContextBase {
TMap<const TTypeList*, tIoKinds> ioTypeMap;

// Structure splitting data:
TMap<int, TVariable*> splitNonIoVars; // variables with the built-in interstage IO removed, indexed by unique ID.
TMap<long long, TVariable*> splitNonIoVars; // variables with the built-in interstage IO removed, indexed by unique ID.

// Structuredbuffer shared types. Typically there are only a few.
TVector<TType*> structBufferTypes;
Expand Down Expand Up @@ -488,18 +488,18 @@ class HlslParseContext : public TParseContextBase {
struct tShadowTextureSymbols {
tShadowTextureSymbols() { symId.fill(-1); }

void set(bool shadow, int id) { symId[int(shadow)] = id; }
int get(bool shadow) const { return symId[int(shadow)]; }
void set(bool shadow, long long id) { symId[int(shadow)] = id; }
long long get(bool shadow) const { return symId[int(shadow)]; }

// True if this texture has been seen with both shadow and non-shadow modes
bool overloaded() const { return symId[0] != -1 && symId[1] != -1; }
bool isShadowId(int id) const { return symId[1] == id; }
bool isShadowId(long long id) const { return symId[1] == id; }

private:
std::array<int, 2> symId;
std::array<long long, 2> symId;
};

TMap<int, tShadowTextureSymbols*> textureShadowVariant;
TMap<long long, tShadowTextureSymbols*> textureShadowVariant;
bool parsingEntrypointParameters;
};

Expand Down
10 changes: 5 additions & 5 deletions glslang/Include/intermediate.h
Original file line number Diff line number Diff line change
Expand Up @@ -1264,15 +1264,15 @@ class TIntermSymbol : public TIntermTyped {
// if symbol is initialized as symbol(sym), the memory comes from the pool allocator of sym. If sym comes from
// per process threadPoolAllocator, then it causes increased memory usage per compile
// it is essential to use "symbol = sym" to assign to symbol
TIntermSymbol(int i, const TString& n, const TType& t)
TIntermSymbol(long long i, const TString& n, const TType& t)
: TIntermTyped(t), id(i),
#ifndef GLSLANG_WEB
flattenSubset(-1),
#endif
constSubtree(nullptr)
{ name = n; }
virtual int getId() const { return id; }
virtual void changeId(int i) { id = i; }
virtual long long getId() const { return id; }
virtual void changeId(long long i) { id = i; }
virtual const TString& getName() const { return name; }
virtual void traverse(TIntermTraverser*);
virtual TIntermSymbol* getAsSymbolNode() { return this; }
Expand All @@ -1290,10 +1290,10 @@ class TIntermSymbol : public TIntermTyped {

// This is meant for cases where a node has already been constructed, and
// later on, it becomes necessary to switch to a different symbol.
virtual void switchId(int newId) { id = newId; }
virtual void switchId(long long newId) { id = newId; }

protected:
int id; // the unique id of the symbol this node represents
long long id; // the unique id of the symbol this node represents
#ifndef GLSLANG_WEB
int flattenSubset; // how deeply the flattened object rooted at id has been dereferenced
#endif
Expand Down
2 changes: 1 addition & 1 deletion glslang/MachineIndependent/Intermediate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ namespace glslang {
// Returns the added node.
//

TIntermSymbol* TIntermediate::addSymbol(int id, const TString& name, const TType& type, const TConstUnionArray& constArray,
TIntermSymbol* TIntermediate::addSymbol(long long id, const TString& name, const TType& type, const TConstUnionArray& constArray,
TIntermTyped* constSubtree, const TSourceLoc& loc)
{
TIntermSymbol* node = new TIntermSymbol(id, name, type);
Expand Down
6 changes: 4 additions & 2 deletions glslang/MachineIndependent/ParseHelper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4274,8 +4274,10 @@ TSymbol* TParseContext::redeclareBuiltinVariable(const TSourceLoc& loc, const TS
// If it wasn't at a built-in level, then it's already been redeclared;
// that is, this is a redeclaration of a redeclaration; reuse that initial
// redeclaration. Otherwise, make the new one.
if (builtIn)
if (builtIn) {
makeEditable(symbol);
symbolTable.amendSymbolIdLevel(*symbol);
}

// Now, modify the type of the copy, as per the type of the current redeclaration.

Expand Down Expand Up @@ -4804,7 +4806,7 @@ void TParseContext::inductiveLoopCheck(const TSourceLoc& loc, TIntermNode* init,
}

// get the unique id of the loop index
int loopIndex = binaryInit->getLeft()->getAsSymbolNode()->getId();
long long loopIndex = binaryInit->getLeft()->getAsSymbolNode()->getId();
inductiveLoopIds.insert(loopIndex);

// condition's form must be "loop-index relational-operator constant-expression"
Expand Down
4 changes: 2 additions & 2 deletions glslang/MachineIndependent/ParseHelper.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ struct TPragma {
class TScanContext;
class TPpContext;

typedef std::set<int> TIdSetType;
typedef std::set<long long> TIdSetType;
typedef std::map<const TTypeList*, std::map<size_t, const TTypeList*>> TStructRecord;

//
Expand Down Expand Up @@ -392,7 +392,7 @@ class TParseContext : public TParseContextBase {
void arrayLimitCheck(const TSourceLoc&, const TString&, int size);
void limitCheck(const TSourceLoc&, int value, const char* limit, const char* feature);

void inductiveLoopBodyCheck(TIntermNode*, int loopIndexId, TSymbolTable&);
void inductiveLoopBodyCheck(TIntermNode*, long long loopIndexId, TSymbolTable&);
void constantIndexExpressionCheck(TIntermNode*);

void setLayoutQualifier(const TSourceLoc&, TPublicType&, TString&);
Expand Down
9 changes: 9 additions & 0 deletions glslang/MachineIndependent/ShaderLang.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -949,6 +949,9 @@ bool ProcessDeferred(
if (cachedTable)
symbolTable->adoptLevels(*cachedTable);

if (intermediate.getUniqueId() != 0)
symbolTable->overwriteUniqueId(intermediate.getUniqueId());

// Add built-in symbols that are potentially context dependent;
// they get popped again further down.
if (! AddContextSpecificSymbols(resources, compiler->infoSink, *symbolTable, version, profile, spvVersion,
Expand Down Expand Up @@ -1011,6 +1014,7 @@ bool ProcessDeferred(
bool success = processingContext(*parseContext, ppContext, fullInput,
versionWillBeError, *symbolTable,
intermediate, optLevel, messages);
intermediate.setUniqueId(symbolTable->getMaxSymbolId());
return success;
}

Expand Down Expand Up @@ -1810,6 +1814,11 @@ void TShader::addProcesses(const std::vector<std::string>& p)
intermediate->addProcesses(p);
}

void TShader::setUniqueId(unsigned long long id)
{
intermediate->setUniqueId(id);
}

void TShader::setInvertY(bool invert) { intermediate->setInvertY(invert); }
void TShader::setNanMinMaxClamp(bool useNonNan) { intermediate->setNanMinMaxClamp(useNonNan); }

Expand Down
2 changes: 1 addition & 1 deletion glslang/MachineIndependent/SymbolTable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ void TType::buildMangledName(TString& mangledName) const
for (int i = 0; i < arraySizes->getNumDims(); ++i) {
if (arraySizes->getDimNode(i)) {
if (arraySizes->getDimNode(i)->getAsSymbolNode())
snprintf(buf, maxSize, "s%d", arraySizes->getDimNode(i)->getAsSymbolNode()->getId());
snprintf(buf, maxSize, "s%lld", arraySizes->getDimNode(i)->getAsSymbolNode()->getId());
else
snprintf(buf, maxSize, "s%p", arraySizes->getDimNode(i));
} else
Expand Down
38 changes: 28 additions & 10 deletions glslang/MachineIndependent/SymbolTable.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ class TSymbol {
virtual const TAnonMember* getAsAnonMember() const { return 0; }
virtual const TType& getType() const = 0;
virtual TType& getWritableType() = 0;
virtual void setUniqueId(int id) { uniqueId = id; }
virtual int getUniqueId() const { return uniqueId; }
virtual void setUniqueId(long long id) { uniqueId = id; }
virtual long long getUniqueId() const { return uniqueId; }
virtual void setExtensions(int numExts, const char* const exts[])
{
assert(extensions == 0);
Expand All @@ -130,7 +130,7 @@ class TSymbol {
TSymbol& operator=(const TSymbol&);

const TString *name;
unsigned int uniqueId; // For cross-scope comparing during code generation
unsigned long long uniqueId; // For cross-scope comparing during code generation

// For tracking what extensions must be present
// (don't use if correct version/profile is present).
Expand Down Expand Up @@ -612,6 +612,7 @@ class TSymbolTable {
// 3: user-shader globals
//
protected:
static const uint32_t LevelFlagBitOffset = 56;
static const int globalLevel = 3;
static bool isSharedLevel(int level) { return level <= 1; } // exclude all per-compile levels
static bool isBuiltInLevel(int level) { return level <= 2; } // exclude user globals
Expand All @@ -620,10 +621,12 @@ class TSymbolTable {
bool isEmpty() { return table.size() == 0; }
bool atBuiltInLevel() { return isBuiltInLevel(currentLevel()); }
bool atGlobalLevel() { return isGlobalLevel(currentLevel()); }
static bool isBuiltInSymbol(int uniqueId) {
int level = uniqueId >> LevelFlagBitOffset;
static bool isBuiltInSymbol(long long uniqueId) {
int level = static_cast<int>(uniqueId >> LevelFlagBitOffset);
return isBuiltInLevel(level);
}
static constexpr uint64_t uniqueIdMask = (1LL << LevelFlagBitOffset) - 1;
static const uint32_t MaxLevelInUniqueID = 127;
void setNoBuiltInRedeclarations() { noBuiltInRedeclarations = true; }
void setSeparateNameSpaces() { separateNameSpaces = true; }

Expand Down Expand Up @@ -691,6 +694,16 @@ class TSymbolTable {
return table[currentLevel()]->amend(symbol, firstNewMember);
}

// Update the level info in symbol's unique ID to current level
void amendSymbolIdLevel(TSymbol& symbol)
{
// clamp level to avoid overflow
uint64_t level = currentLevel() > MaxLevelInUniqueID ? MaxLevelInUniqueID : currentLevel();
uint64_t symbolId = symbol.getUniqueId();
symbolId &= uniqueIdMask;
symbolId |= (level << LevelFlagBitOffset);
symbol.setUniqueId(symbolId);
}
//
// To allocate an internal temporary, which will need to be uniquely
// identified by the consumer of the AST, but never need to
Expand Down Expand Up @@ -859,7 +872,7 @@ class TSymbolTable {
}
}

int getMaxSymbolId() { return uniqueId; }
long long getMaxSymbolId() { return uniqueId; }
#if !defined(GLSLANG_WEB) && !defined(GLSLANG_ANGLE)
void dump(TInfoSink& infoSink, bool complete = false) const;
#endif
Expand All @@ -876,19 +889,24 @@ class TSymbolTable {
// Add current level in the high-bits of unique id
void updateUniqueIdLevelFlag() {
// clamp level to avoid overflow
uint32_t level = currentLevel() > 7 ? 7 : currentLevel();
uniqueId &= ((1 << LevelFlagBitOffset) - 1);
uint64_t level = currentLevel() > MaxLevelInUniqueID ? MaxLevelInUniqueID : currentLevel();
uniqueId &= uniqueIdMask;
uniqueId |= (level << LevelFlagBitOffset);
}

void overwriteUniqueId(long long id)
{
uniqueId = id;
updateUniqueIdLevelFlag();
}

protected:
TSymbolTable(TSymbolTable&);
TSymbolTable& operator=(TSymbolTableLevel&);

int currentLevel() const { return static_cast<int>(table.size()) - 1; }
static const uint32_t LevelFlagBitOffset = 28;
std::vector<TSymbolTableLevel*> table;
int uniqueId; // for unique identification in code generation
long long uniqueId; // for unique identification in code generation
bool noBuiltInRedeclarations;
bool separateNameSpaces;
unsigned int adoptedLevels;
Expand Down
2 changes: 1 addition & 1 deletion glslang/MachineIndependent/iomapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ namespace glslang {

class TIntermediate;
struct TVarEntryInfo {
int id;
long long id;
TIntermSymbol* symbol;
bool live;
int newBinding;
Expand Down
Loading

0 comments on commit 93b400f

Please sign in to comment.