Skip to content

A
angle
Commit 5c0e023c by Olli Etuaho
Options

Qualify stored constant union data with const

This prevents accidentally changing data that may be shared between multiple TIntermConstantUnion nodes. Besides making the code less prone to bugs in general, this will make it easier to implement constant folding of array constructors. BUG=541551 TEST=angle_unittests, WebGL conformance tests Change-Id: I4f3059f70b841d9dd0cf20fea4d37684da9cd47e Reviewed-on: https://chromium-review.googlesource.com/312440Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Tryjob-Request: Jamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarCorentin Wallez <cwallez@chromium.org> Tested-by: 's avatarOlli Etuaho <oetuaho@nvidia.com>
parent 36b0514a
Showing with 45 additions and 56 deletions
src/compiler/translator/IntermNode.cpp
......@@ -169,7 +169,7 @@ void UndefinedConstantFoldingError(const TSourceLoc &loc, TOperator op, TBasicTy
}
}
float VectorLength(TConstantUnion *paramArray, size_t paramArraySize)
float VectorLength(const TConstantUnion *paramArray, size_t paramArraySize)
{
float result = 0.0f;
for (size_t i = 0; i < paramArraySize; i++)
......@@ -180,7 +180,9 @@ float VectorLength(TConstantUnion *paramArray, size_t paramArraySize)
return sqrtf(result);
}
float VectorDotProduct(TConstantUnion *paramArray1, TConstantUnion *paramArray2, size_t paramArraySize)
float VectorDotProduct(const TConstantUnion *paramArray1,
const TConstantUnion *paramArray2,
size_t paramArraySize)
{
float result = 0.0f;
for (size_t i = 0; i < paramArraySize; i++)
......@@ -202,7 +204,9 @@ TIntermTyped *CreateFoldedNode(TConstantUnion *constArray,
return folded;
}
angle::Matrix<float> GetMatrix(TConstantUnion *paramArray, const unsigned int &rows, const unsigned int &cols)
angle::Matrix<float> GetMatrix(const TConstantUnion *paramArray,
const unsigned int &rows,
const unsigned int &cols)
{
std::vector<float> elements;
for (size_t i = 0; i < rows * cols; i++)
......@@ -212,7 +216,7 @@ angle::Matrix<float> GetMatrix(TConstantUnion *paramArray, const unsigned int &r
return angle::Matrix<float>(elements, rows, cols).transpose();
}
angle::Matrix<float> GetMatrix(TConstantUnion *paramArray, const unsigned int &size)
angle::Matrix<float> GetMatrix(const TConstantUnion *paramArray, const unsigned int &size)
{
std::vector<float> elements;
for (size_t i = 0; i < size * size; i++)
......@@ -416,12 +420,7 @@ TIntermTyped::TIntermTyped(const TIntermTyped &node) : TIntermNode(), mType(node
TIntermConstantUnion::TIntermConstantUnion(const TIntermConstantUnion &node) : TIntermTyped(node)
{
size_t arraySize = mType.getObjectSize();
mUnionArrayPointer = new TConstantUnion[arraySize];
for (size_t i = 0u; i < arraySize; ++i)
{
mUnionArrayPointer[i] = node.mUnionArrayPointer[i];
}
mUnionArrayPointer = node.mUnionArrayPointer;
}
TIntermAggregate::TIntermAggregate(const TIntermAggregate &node)
......@@ -969,8 +968,8 @@ TIntermTyped *TIntermAggregate::fold(TInfoSink &infoSink)
//
TConstantUnion *TIntermConstantUnion::foldBinary(TOperator op, TIntermConstantUnion *rightNode, TInfoSink &infoSink)
{
TConstantUnion *leftArray = getUnionArrayPointer();
TConstantUnion *rightArray = rightNode->getUnionArrayPointer();
const TConstantUnion *leftArray = getUnionArrayPointer();
const TConstantUnion *rightArray = rightNode->getUnionArrayPointer();
if (!leftArray)
return nullptr;
......@@ -1317,7 +1316,7 @@ TConstantUnion *TIntermConstantUnion::foldUnaryWithDifferentReturnType(TOperator
// Do operations where the return type has a different number of components compared to the operand type.
//
TConstantUnion *operandArray = getUnionArrayPointer();
const TConstantUnion *operandArray = getUnionArrayPointer();
if (!operandArray)
return nullptr;
......@@ -1530,7 +1529,7 @@ TConstantUnion *TIntermConstantUnion::foldUnaryWithSameReturnType(TOperator op,
// Do unary operations where the return type is the same as operand type.
//
TConstantUnion *operandArray = getUnionArrayPointer();
const TConstantUnion *operandArray = getUnionArrayPointer();
if (!operandArray)
return nullptr;
......@@ -2082,7 +2081,7 @@ TConstantUnion *TIntermConstantUnion::FoldAggregateBuiltIn(TIntermAggregate *agg
TOperator op = aggregate->getOp();
TIntermSequence *sequence = aggregate->getSequence();
unsigned int paramsCount = static_cast<unsigned int>(sequence->size());
std::vector<TConstantUnion *> unionArrays(paramsCount);
std::vector<const TConstantUnion *> unionArrays(paramsCount);
std::vector<size_t> objectSizes(paramsCount);
size_t maxObjectSize = 0;
TBasicType basicType = EbtVoid;
......
src/compiler/translator/IntermNode.h
......@@ -302,16 +302,16 @@ class TIntermRaw : public TIntermTyped
class TIntermConstantUnion : public TIntermTyped
{
public:
TIntermConstantUnion(TConstantUnion *unionPointer, const TType &type)
: TIntermTyped(type),
mUnionArrayPointer(unionPointer) { }
TIntermConstantUnion(const TConstantUnion *unionPointer, const TType &type)
: TIntermTyped(type), mUnionArrayPointer(unionPointer)
{
}
TIntermTyped *deepCopy() const override { return new TIntermConstantUnion(*this); }
bool hasSideEffects() const override { return false; }
const TConstantUnion *getUnionArrayPointer() const { return mUnionArrayPointer; }
TConstantUnion *getUnionArrayPointer() { return mUnionArrayPointer; }
int getIConst(size_t index) const
{
......@@ -330,7 +330,7 @@ class TIntermConstantUnion : public TIntermTyped
return mUnionArrayPointer ? mUnionArrayPointer[index].getBConst() : false;
}
void replaceConstantUnion(TConstantUnion *safeConstantUnion)
void replaceConstantUnion(const TConstantUnion *safeConstantUnion)
{
// Previous union pointer freed on pool deallocation.
mUnionArrayPointer = safeConstantUnion;
......@@ -349,7 +349,8 @@ class TIntermConstantUnion : public TIntermTyped
static TConstantUnion *FoldAggregateBuiltIn(TIntermAggregate *aggregate, TInfoSink &infoSink);
protected:
TConstantUnion *mUnionArrayPointer;
// Same data may be shared between multiple constant unions, so it can't be modified.
const TConstantUnion *mUnionArrayPointer;
private:
typedef float(*FloatTypeUnaryFunc) (float);
......
src/compiler/translator/Intermediate.cpp
......@@ -375,8 +375,9 @@ TIntermCase *TIntermediate::addCase(
// Returns the constant union node created.
//
TIntermConstantUnion *TIntermediate::addConstantUnion(
TConstantUnion *constantUnion, const TType &type, const TSourceLoc &line)
TIntermConstantUnion *TIntermediate::addConstantUnion(const TConstantUnion *constantUnion,
const TType &type,
const TSourceLoc &line)
{
TIntermConstantUnion *node = new TIntermConstantUnion(constantUnion, type);
node->setLine(line);
......
src/compiler/translator/Intermediate.h
......@@ -52,8 +52,9 @@ class TIntermediate
TIntermTyped *right,
const TSourceLoc &line,
int shaderVersion);
TIntermConstantUnion *addConstantUnion(
TConstantUnion *constantUnion, const TType &type, const TSourceLoc &line);
TIntermConstantUnion *addConstantUnion(const TConstantUnion *constantUnion,
const TType &type,
const TSourceLoc &line);
TIntermNode *addLoop(TLoopType, TIntermNode *, TIntermTyped *, TIntermTyped *,
TIntermNode *, const TSourceLoc &);
TIntermBranch *addBranch(TOperator, const TSourceLoc &);
......
src/compiler/translator/ParseContext.cpp
......@@ -1216,7 +1216,7 @@ TIntermTyped *TParseContext::parseVariableIdentifier(const TSourceLoc &location,
if (variable->getConstPointer())
{
TConstantUnion *constArray = variable->getConstPointer();
const TConstantUnion *constArray = variable->getConstPointer();
return intermediate.addConstantUnion(constArray, variable->getType(), location);
}
else
......@@ -1353,7 +1353,7 @@ bool TParseContext::executeInitializer(const TSourceLoc &line,
symbolTable.find(initializer->getAsSymbolNode()->getSymbol(), 0);
const TVariable *tVar = static_cast<const TVariable *>(symbol);
TConstantUnion *constArray = tVar->getConstPointer();
const TConstantUnion *constArray = tVar->getConstPointer();
if (constArray)
{
variable->shareConstPointer(constArray);
......@@ -2373,7 +2373,7 @@ TIntermTyped *TParseContext::addConstMatrixNode(int index,
index = node->getType().getCols() - 1;
}
TConstantUnion *unionArray = node->getUnionArrayPointer();
const TConstantUnion *unionArray = node->getUnionArrayPointer();
int size = node->getType().getCols();
return intermediate.addConstantUnion(&unionArray[size * index], node->getType(), line);
}
......@@ -2401,8 +2401,8 @@ TIntermTyped *TParseContext::addConstArrayNode(int index,
outOfRangeError(outOfRangeIndexIsError, line, "", "[", extraInfo.c_str());
index = node->getType().getArraySize() - 1;
}
size_t arrayElementSize = arrayElementType.getObjectSize();
TConstantUnion *unionArray = node->getUnionArrayPointer();
size_t arrayElementSize = arrayElementType.getObjectSize();
const TConstantUnion *unionArray = node->getUnionArrayPointer();
return intermediate.addConstantUnion(&unionArray[arrayElementSize * index], node->getType(),
line);
}
......@@ -2437,7 +2437,7 @@ TIntermTyped *TParseContext::addConstStruct(const TString &identifier,
TIntermConstantUnion *tempConstantNode = node->getAsConstantUnion();
if (tempConstantNode)
{
TConstantUnion *constArray = tempConstantNode->getUnionArrayPointer();
const TConstantUnion *constArray = tempConstantNode->getUnionArrayPointer();
// type will be changed in the calling function
typedNode = intermediate.addConstantUnion(constArray + instanceSize,
......@@ -2796,8 +2796,9 @@ TIntermTyped *TParseContext::addIndexExpression(TIntermTyped *baseExpression,
safeIndex = baseExpression->getType().getNominalSize() - 1;
}
// Don't modify the data of the previous constant union, because it can point
// to builtins, like gl_MaxDrawBuffers. Instead use a new sanitized object.
// Data of constant unions can't be changed, because it may be shared with other
// constant unions or even builtins, like gl_MaxDrawBuffers. Instead use a new
// sanitized object.
if (safeIndex != -1)
{
TConstantUnion *safeConstantUnion = new TConstantUnion();
......
src/compiler/translator/SymbolTable.h
......@@ -128,34 +128,16 @@ class TVariable : public TSymbol
type.setQualifier(qualifier);
}
TConstantUnion *getConstPointer()
{
if (!unionArray)
unionArray = new TConstantUnion[type.getObjectSize()];
const TConstantUnion *getConstPointer() const { return unionArray; }
return unionArray;
}
TConstantUnion *getConstPointer() const
{
return unionArray;
}
void shareConstPointer(TConstantUnion *constArray)
{
if (unionArray == constArray)
return;
delete[] unionArray;
unionArray = constArray;
}
void shareConstPointer(const TConstantUnion *constArray) { unionArray = constArray; }
private:
TType type;
bool userType;
// we are assuming that Pool Allocator will free the memory
// allocated to unionArray when this object is destroyed.
TConstantUnion *unionArray;
const TConstantUnion *unionArray;
};
// Immutable version of TParameter.
......@@ -400,7 +382,9 @@ class TSymbolTable : angle::NonCopyable
{
TVariable *constant = new TVariable(
NewPoolTString(name), TType(EbtInt, EbpUndefined, EvqConst, 1));
constant->getConstPointer()->setIConst(value);
TConstantUnion *unionArray = new TConstantUnion[1];
unionArray[0].setIConst(value);
constant->shareConstPointer(unionArray);
return insert(level, constant);
}
......@@ -408,7 +392,9 @@ class TSymbolTable : angle::NonCopyable
{
TVariable *constant =
new TVariable(NewPoolTString(name), TType(EbtInt, EbpUndefined, EvqConst, 1));
constant->getConstPointer()->setIConst(value);
TConstantUnion *unionArray = new TConstantUnion[1];
unionArray[0].setIConst(value);
constant->shareConstPointer(unionArray);
return insert(level, ext, constant);
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment