Skip to content

G
glslang
Commit 0de16da2 by steve-lunarg
Options

HLSL: phase 2c: use lValueErrorCheck in HLSL FE

This commit splits lValueErrorCheck into machine dependent and independent parts. The GLSL form in TParseContext inherits from and invokes the machine dependent part in TParseContextBase. The base form checks language independent things. This split does not change the set of errors tested for: the test results are identical. The new base class interface is now used from the HLSL FE to test lvalues. There was one test diff due to this, where the test was writing to a uniform. It still does the same indirections, but does not attempt a uniform write.
parent 90707966
Showing with 222 additions and 131 deletions
Test/hlsl.struct.frag
...@@ -37,7 +37,7 @@ float4 PixelShaderFunction(float4 input, IN_S s) : COLOR0 ...@@ -37,7 +37,7 @@ float4 PixelShaderFunction(float4 input, IN_S s) : COLOR0
} s3; } s3;
s3 == s3; s3 == s3;
s2.i = s.ff4; s2.i; s.ff4; // no assignments to uniforms, but preserve indirections.
return input; return input;
} }
\ No newline at end of file
glslang/MachineIndependent/ParseContextBase.cpp
...@@ -113,6 +113,114 @@ void C_DECL TParseContextBase::ppWarn(const TSourceLoc& loc, const char* szReaso ...@@ -113,6 +113,114 @@ void C_DECL TParseContextBase::ppWarn(const TSourceLoc& loc, const char* szReaso
va_end(args); va_end(args);
} }
//
// Both test and if necessary, spit out an error, to see if the node is really
// an l-value that can be operated on this way.
//
// Returns true if there was an error.
//
bool TParseContextBase::lValueErrorCheck(const TSourceLoc& loc, const char* op, TIntermTyped* node)
{
TIntermBinary* binaryNode = node->getAsBinaryNode();
if (binaryNode) {
switch(binaryNode->getOp()) {
case EOpIndexDirect:
case EOpIndexIndirect: // fall through
case EOpIndexDirectStruct: // fall through
case EOpVectorSwizzle:
return lValueErrorCheck(loc, op, binaryNode->getLeft());
default:
break;
}
error(loc, " l-value required", op, "", "");
return true;
}
const char* symbol = nullptr;
TIntermSymbol* symNode = node->getAsSymbolNode();
if (symNode != nullptr)
symbol = symNode->getName().c_str();
const char* message = nullptr;
switch (node->getQualifier().storage) {
case EvqConst: message = "can't modify a const"; break;
case EvqConstReadOnly: message = "can't modify a const"; break;
case EvqUniform: message = "can't modify a uniform"; break;
case EvqBuffer:
if (node->getQualifier().readonly)
message = "can't modify a readonly buffer";
break;
default:
//
// Type that can't be written to?
//
switch (node->getBasicType()) {
case EbtSampler:
message = "can't modify a sampler";
break;
case EbtAtomicUint:
message = "can't modify an atomic_uint";
break;
case EbtVoid:
message = "can't modify void";
break;
default:
break;
}
}
if (message == nullptr && binaryNode == nullptr && symNode == nullptr) {
error(loc, " l-value required", op, "", "");
return true;
}
//
// Everything else is okay, no error.
//
if (message == nullptr)
return false;
//
// If we get here, we have an error and a message.
//
if (symNode)
error(loc, " l-value required", op, "\"%s\" (%s)", symbol, message);
else
error(loc, " l-value required", op, "(%s)", message);
return true;
}
// Test for and give an error if the node can't be read from.
void TParseContextBase::rValueErrorCheck(const TSourceLoc& loc, const char* op, TIntermTyped* node)
{
if (! node)
return;
TIntermBinary* binaryNode = node->getAsBinaryNode();
if (binaryNode) {
switch(binaryNode->getOp()) {
case EOpIndexDirect:
case EOpIndexIndirect:
case EOpIndexDirectStruct:
case EOpVectorSwizzle:
rValueErrorCheck(loc, op, binaryNode->getLeft());
default:
break;
}
return;
}
TIntermSymbol* symNode = node->getAsSymbolNode();
if (symNode && symNode->getQualifier().writeonly)
error(loc, "can't read from writeonly object: ", op, symNode->getName().c_str());
}
// Make a shared symbol have a non-shared version that can be edited by the current // Make a shared symbol have a non-shared version that can be edited by the current
// compile, such that editing its type will not change the shared version and will // compile, such that editing its type will not change the shared version and will
// effect all nodes sharing it. // effect all nodes sharing it.
......
glslang/MachineIndependent/ParseHelper.cpp
...@@ -1902,14 +1902,14 @@ void TParseContext::variableCheck(TIntermTyped*& nodePtr) ...@@ -1902,14 +1902,14 @@ void TParseContext::variableCheck(TIntermTyped*& nodePtr)
// Both test and if necessary, spit out an error, to see if the node is really // Both test and if necessary, spit out an error, to see if the node is really
// an l-value that can be operated on this way. // an l-value that can be operated on this way.
// //
// Returns true if the was an error. // Returns true if there was an error.
// //
bool TParseContext::lValueErrorCheck(const TSourceLoc& loc, const char* op, TIntermTyped* node) bool TParseContext::lValueErrorCheck(const TSourceLoc& loc, const char* op, TIntermTyped* node)
{ {
TIntermBinary* binaryNode = node->getAsBinaryNode(); TIntermBinary* binaryNode = node->getAsBinaryNode();
if (binaryNode) { if (binaryNode) {
bool errorReturn; bool errorReturn = false;
switch(binaryNode->getOp()) { switch(binaryNode->getOp()) {
case EOpIndexDirect: case EOpIndexDirect:
...@@ -1928,9 +1928,9 @@ bool TParseContext::lValueErrorCheck(const TSourceLoc& loc, const char* op, TInt ...@@ -1928,9 +1928,9 @@ bool TParseContext::lValueErrorCheck(const TSourceLoc& loc, const char* op, TInt
} }
} }
// fall through break; // left node is checked by base class
case EOpIndexDirectStruct: case EOpIndexDirectStruct:
return lValueErrorCheck(loc, op, binaryNode->getLeft()); break; // left node is checked by base class
case EOpVectorSwizzle: case EOpVectorSwizzle:
errorReturn = lValueErrorCheck(loc, op, binaryNode->getLeft()); errorReturn = lValueErrorCheck(loc, op, binaryNode->getLeft());
if (!errorReturn) { if (!errorReturn) {
...@@ -1955,12 +1955,18 @@ bool TParseContext::lValueErrorCheck(const TSourceLoc& loc, const char* op, TInt ...@@ -1955,12 +1955,18 @@ bool TParseContext::lValueErrorCheck(const TSourceLoc& loc, const char* op, TInt
default: default:
break; break;
} }
error(loc, " l-value required", op, "", "");
return true; if (errorReturn) {
error(loc, " l-value required", op, "", "");
return true;
}
} }
// Let the base class check errors
if (TParseContextBase::lValueErrorCheck(loc, op, node))
return true;
// GLSL specific
const char* symbol = nullptr; const char* symbol = nullptr;
TIntermSymbol* symNode = node->getAsSymbolNode(); TIntermSymbol* symNode = node->getAsSymbolNode();
if (symNode != nullptr) if (symNode != nullptr)
...@@ -1968,19 +1974,12 @@ bool TParseContext::lValueErrorCheck(const TSourceLoc& loc, const char* op, TInt ...@@ -1968,19 +1974,12 @@ bool TParseContext::lValueErrorCheck(const TSourceLoc& loc, const char* op, TInt
const char* message = nullptr; const char* message = nullptr;
switch (node->getQualifier().storage) { switch (node->getQualifier().storage) {
case EvqConst: message = "can't modify a const"; break;
case EvqConstReadOnly: message = "can't modify a const"; break;
case EvqVaryingIn: message = "can't modify shader input"; break; case EvqVaryingIn: message = "can't modify shader input"; break;
case EvqInstanceId: message = "can't modify gl_InstanceID"; break; case EvqInstanceId: message = "can't modify gl_InstanceID"; break;
case EvqVertexId: message = "can't modify gl_VertexID"; break; case EvqVertexId: message = "can't modify gl_VertexID"; break;
case EvqFace: message = "can't modify gl_FrontFace"; break; case EvqFace: message = "can't modify gl_FrontFace"; break;
case EvqFragCoord: message = "can't modify gl_FragCoord"; break; case EvqFragCoord: message = "can't modify gl_FragCoord"; break;
case EvqPointCoord: message = "can't modify gl_PointCoord"; break; case EvqPointCoord: message = "can't modify gl_PointCoord"; break;
case EvqUniform: message = "can't modify a uniform"; break;
case EvqBuffer:
if (node->getQualifier().readonly)
message = "can't modify a readonly buffer";
break;
case EvqFragDepth: case EvqFragDepth:
intermediate.setDepthReplacing(); intermediate.setDepthReplacing();
// "In addition, it is an error to statically write to gl_FragDepth in the fragment shader." // "In addition, it is an error to statically write to gl_FragDepth in the fragment shader."
...@@ -1989,22 +1988,7 @@ bool TParseContext::lValueErrorCheck(const TSourceLoc& loc, const char* op, TInt ...@@ -1989,22 +1988,7 @@ bool TParseContext::lValueErrorCheck(const TSourceLoc& loc, const char* op, TInt
break; break;
default: default:
// break;
// Type that can't be written to?
//
switch (node->getBasicType()) {
case EbtSampler:
message = "can't modify a sampler";
break;
case EbtAtomicUint:
message = "can't modify an atomic_uint";
break;
case EbtVoid:
message = "can't modify void";
break;
default:
break;
}
} }
if (message == nullptr && binaryNode == nullptr && symNode == nullptr) { if (message == nullptr && binaryNode == nullptr && symNode == nullptr) {
...@@ -2013,7 +1997,6 @@ bool TParseContext::lValueErrorCheck(const TSourceLoc& loc, const char* op, TInt ...@@ -2013,7 +1997,6 @@ bool TParseContext::lValueErrorCheck(const TSourceLoc& loc, const char* op, TInt
return true; return true;
} }
// //
// Everything else is okay, no error. // Everything else is okay, no error.
// //
...@@ -2034,30 +2017,14 @@ bool TParseContext::lValueErrorCheck(const TSourceLoc& loc, const char* op, TInt ...@@ -2034,30 +2017,14 @@ bool TParseContext::lValueErrorCheck(const TSourceLoc& loc, const char* op, TInt
// Test for and give an error if the node can't be read from. // Test for and give an error if the node can't be read from.
void TParseContext::rValueErrorCheck(const TSourceLoc& loc, const char* op, TIntermTyped* node) void TParseContext::rValueErrorCheck(const TSourceLoc& loc, const char* op, TIntermTyped* node)
{ {
if (! node) // Let the base class check errors
return; TParseContextBase::rValueErrorCheck(loc, op, node);
TIntermBinary* binaryNode = node->getAsBinaryNode();
if (binaryNode) {
switch(binaryNode->getOp()) {
case EOpIndexDirect:
case EOpIndexIndirect:
case EOpIndexDirectStruct:
case EOpVectorSwizzle:
rValueErrorCheck(loc, op, binaryNode->getLeft());
default:
break;
}
return;
}
TIntermSymbol* symNode = node->getAsSymbolNode();
if (symNode && symNode->getQualifier().writeonly)
error(loc, "can't read from writeonly object: ", op, symNode->getName().c_str());
#ifdef AMD_EXTENSIONS #ifdef AMD_EXTENSIONS
else if (symNode && symNode->getQualifier().explicitInterp) TIntermSymbol* symNode = node->getAsSymbolNode();
error(loc, "can't read from explicitly-interpolated object: ", op, symNode->getName().c_str()); if (!(symNode && symNode->getQualifier().writeonly)) // base class checks
if (symNode && symNode->getQualifier().explicitInterp)
error(loc, "can't read from explicitly-interpolated object: ", op, symNode->getName().c_str());
#endif #endif
} }
......
glslang/MachineIndependent/ParseHelper.h
...@@ -143,6 +143,9 @@ public: ...@@ -143,6 +143,9 @@ public:
virtual void growGlobalUniformBlock(TSourceLoc&, TType&, TString& memberName); virtual void growGlobalUniformBlock(TSourceLoc&, TType&, TString& memberName);
virtual bool insertGlobalUniformBlock(); virtual bool insertGlobalUniformBlock();
virtual bool lValueErrorCheck(const TSourceLoc&, const char* op, TIntermTyped*);
virtual void rValueErrorCheck(const TSourceLoc&, const char* op, TIntermTyped*);
protected: protected:
TParseContextBase(TParseContextBase&); TParseContextBase(TParseContextBase&);
TParseContextBase& operator=(TParseContextBase&); TParseContextBase& operator=(TParseContextBase&);
...@@ -278,8 +281,8 @@ public: ...@@ -278,8 +281,8 @@ public:
void unaryOpError(const TSourceLoc&, const char* op, TString operand); void unaryOpError(const TSourceLoc&, const char* op, TString operand);
void binaryOpError(const TSourceLoc&, const char* op, TString left, TString right); void binaryOpError(const TSourceLoc&, const char* op, TString left, TString right);
void variableCheck(TIntermTyped*& nodePtr); void variableCheck(TIntermTyped*& nodePtr);
bool lValueErrorCheck(const TSourceLoc&, const char* op, TIntermTyped*); bool lValueErrorCheck(const TSourceLoc&, const char* op, TIntermTyped*) override;
void rValueErrorCheck(const TSourceLoc&, const char* op, TIntermTyped*); void rValueErrorCheck(const TSourceLoc&, const char* op, TIntermTyped*) override;
void constantValueCheck(TIntermTyped* node, const char* token); void constantValueCheck(TIntermTyped* node, const char* token);
void integerCheck(const TIntermTyped* node, const char* token); void integerCheck(const TIntermTyped* node, const char* token);
void globalCheck(const TSourceLoc&, const char* token); void globalCheck(const TSourceLoc&, const char* token);
......
hlsl/hlslParseHelper.cpp
...@@ -136,12 +136,36 @@ bool HlslParseContext::shouldConvertLValue(const TIntermNode* node) const ...@@ -136,12 +136,36 @@ bool HlslParseContext::shouldConvertLValue(const TIntermNode* node) const
return false; return false;
const TIntermAggregate* lhsAsAggregate = node->getAsAggregate(); const TIntermAggregate* lhsAsAggregate = node->getAsAggregate();
if (lhsAsAggregate != nullptr && lhsAsAggregate->getOp() == EOpImageLoad) if (lhsAsAggregate != nullptr && lhsAsAggregate->getOp() == EOpImageLoad)
return true; return true;
return false; return false;
} }
//
// Both test and if necessary, spit out an error, to see if the node is really
// an l-value that can be operated on this way.
//
// Returns true if there was an error.
//
bool HlslParseContext::lValueErrorCheck(const TSourceLoc& loc, const char* op, TIntermTyped* node)
{
if (shouldConvertLValue(node)) {
// if we're writing to a texture, it must be an RW form.
TIntermAggregate* lhsAsAggregate = node->getAsAggregate();
TIntermTyped* object = lhsAsAggregate->getSequence()[0]->getAsTyped();
if (!object->getType().getSampler().isImage()) {
error(loc, "operator[] on a non-RW texture must be an r-value", "", "");
return true;
}
}
// Let the base class check errors
return TParseContextBase::lValueErrorCheck(loc, op, node);
}
// //
// This function handles l-value conversions and verifications. It uses, but is not synonymous // This function handles l-value conversions and verifications. It uses, but is not synonymous
...@@ -161,13 +185,11 @@ TIntermTyped* HlslParseContext::handleLvalue(const TSourceLoc& loc, const char* ...@@ -161,13 +185,11 @@ TIntermTyped* HlslParseContext::handleLvalue(const TSourceLoc& loc, const char*
nodeAsBinary ? nodeAsBinary->getLeft() : nodeAsBinary ? nodeAsBinary->getLeft() :
nullptr; nullptr;
// Early bail out if there is no conversion to apply // Early bail out if there is no conversion to apply
if (!shouldConvertLValue(lhs)) { if (!shouldConvertLValue(lhs)) {
// TODO: >.. if (lhs != nullptr)
// if (lhs != nullptr) if (lValueErrorCheck(loc, op, lhs))
// if (lValueErrorCheck(loc, op, lhs)) return nullptr;
// return nullptr;
return node; return node;
} }
...@@ -356,8 +378,6 @@ TIntermTyped* HlslParseContext::handleLvalue(const TSourceLoc& loc, const char* ...@@ -356,8 +378,6 @@ TIntermTyped* HlslParseContext::handleLvalue(const TSourceLoc& loc, const char*
addUnary(assignOp, rhsTmp2); // rhsTmp op addUnary(assignOp, rhsTmp2); // rhsTmp op
makeStore(object, coordTmp, rhsTmp2); // OpImageStore(object, coordTmp, rhsTmp2) makeStore(object, coordTmp, rhsTmp2); // OpImageStore(object, coordTmp, rhsTmp2)
return finishSequence(rhsTmp1, objDerefType); // return rhsTmp from sequence return finishSequence(rhsTmp1, objDerefType); // return rhsTmp from sequence
break;
} }
default: default:
...@@ -365,10 +385,9 @@ TIntermTyped* HlslParseContext::handleLvalue(const TSourceLoc& loc, const char* ...@@ -365,10 +385,9 @@ TIntermTyped* HlslParseContext::handleLvalue(const TSourceLoc& loc, const char*
} }
} }
// TODO: if (lhs)
// if (lhs) if (lValueErrorCheck(loc, op, lhs))
// if (lValueErrorCheck(loc, op, lhs)) return nullptr;
// return nullptr;
return node; return node;
} }
......
hlsl/hlslParseHelper.h
...@@ -154,6 +154,7 @@ public: ...@@ -154,6 +154,7 @@ public:
// Apply L-value conversions. E.g, turning a write to a RWTexture into an ImageStore. // Apply L-value conversions. E.g, turning a write to a RWTexture into an ImageStore.
TIntermTyped* handleLvalue(const TSourceLoc&, const char* op, TIntermTyped* node); TIntermTyped* handleLvalue(const TSourceLoc&, const char* op, TIntermTyped* node);
bool lValueErrorCheck(const TSourceLoc&, const char* op, TIntermTyped*) override;
protected: protected:
void inheritGlobalDefaults(TQualifier& dst) const; void inheritGlobalDefaults(TQualifier& dst) const;
......
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