Skip to content

G
glslang
Unverified Commit 5bfb4230 by John Kessenich Committed by GitHub
Options

Merge pull request #2367 from KhronosGroup/fix-semantic-checking

Remove incorrect style of extension-based semantic checking.
parents d253278f 01384725
Showing with 115 additions and 64 deletions
SPIRV/GlslangToSpv.cpp
......@@ -1444,7 +1444,7 @@ TGlslangToSpvTraverser::TGlslangToSpvTraverser(unsigned int spvVersion,
// Add the source extensions
const auto& sourceExtensions = glslangIntermediate->getRequestedExtensions();
for (auto it = sourceExtensions.begin(); it != sourceExtensions.end(); ++it)
builder.addSourceExtension(it->first.c_str());
builder.addSourceExtension(it->c_str());
// Add the top-level modes for this shader.
......
Test/baseResults/spv.bufferhandle17_Errors.frag.out
......@@ -2,8 +2,12 @@ spv.bufferhandle17_Errors.frag
ERROR: 0:11: 'qualifier' : variables with reference type can't have qualifier 'const'
ERROR: 0:16: 'qualifier' : variables with reference type can't have qualifier 'const'
ERROR: 0:18: '==' : can't use with reference types
ERROR: 0:18: 'buffer reference math' : required extension not requested: GL_EXT_buffer_reference2
ERROR: 0:18: '==' : wrong operand types: no operation '==' exists that takes a left-hand operand of type ' temp reference' and a right operand of type ' temp reference' (or there is no acceptable conversion)
ERROR: 0:19: '!=' : can't use with reference types
ERROR: 4 compilation errors. No code generated.
ERROR: 0:19: 'buffer reference math' : required extension not requested: GL_EXT_buffer_reference2
ERROR: 0:19: '!=' : wrong operand types: no operation '!=' exists that takes a left-hand operand of type ' temp reference' and a right operand of type ' temp reference' (or there is no acceptable conversion)
ERROR: 8 compilation errors. No code generated.
SPIR-V is not generated for failed compile or link
Test/baseResults/versionsErrors.frag.out
......@@ -7,7 +7,6 @@ ERROR: 4 compilation errors. No code generated.
Shader version: 110
Requested GL_ARB_texture_rectangle
ERROR: node is still EOpNull!
0:42 Function Definition: main( ( global void)
0:42 Function Parameters:
......@@ -28,7 +27,6 @@ Linked fragment stage:
Shader version: 110
Requested GL_ARB_texture_rectangle
ERROR: node is still EOpNull!
0:42 Function Definition: main( ( global void)
0:42 Function Parameters:
......
glslang/MachineIndependent/ParseHelper.cpp
......@@ -751,8 +751,11 @@ TIntermTyped* TParseContext::handleBinaryMath(const TSourceLoc& loc, const char*
}
TIntermTyped* result = nullptr;
if (allowed)
if (allowed) {
if ((left->isReference() || right->isReference()))
requireExtensions(loc, 1, &E_GL_EXT_buffer_reference2, "buffer reference math");
result = intermediate.addBinaryMath(op, left, right, loc);
}
if (result == nullptr)
binaryOpError(loc, str, left->getCompleteString(), right->getCompleteString());
......@@ -1680,6 +1683,14 @@ TIntermTyped* TParseContext::addOutputArgumentConversions(const TFunction& funct
#endif
}
TIntermTyped* TParseContext::addAssign(const TSourceLoc& loc, TOperator op, TIntermTyped* left, TIntermTyped* right)
{
if ((op == EOpAddAssign || op == EOpSubAssign) && left->isReference())
requireExtensions(loc, 1, &E_GL_EXT_buffer_reference2, "+= and -= on a buffer reference");
return intermediate.addAssign(op, left, right, loc);
}
void TParseContext::memorySemanticsCheck(const TSourceLoc& loc, const TFunction& fnCandidate, const TIntermOperator& callNode)
{
const TIntermSequence* argp = &callNode.getAsAggregate()->getSequence();
......@@ -7305,6 +7316,8 @@ TIntermTyped* TParseContext::constructBuiltIn(const TType& type, TOperator op, T
if (!node->getType().isCoopMat()) {
if (type.getBasicType() != node->getType().getBasicType()) {
node = intermediate.addConversion(type.getBasicType(), node);
if (node == nullptr)
return nullptr;
}
node = intermediate.setAggregateOperator(node, EOpConstructCooperativeMatrix, type, node->getLoc());
} else {
......
glslang/MachineIndependent/ParseHelper.h
......@@ -328,6 +328,7 @@ public:
TIntermTyped* handleLengthMethod(const TSourceLoc&, TFunction*, TIntermNode*);
void addInputArgumentConversions(const TFunction&, TIntermNode*&) const;
TIntermTyped* addOutputArgumentConversions(const TFunction&, TIntermAggregate&) const;
TIntermTyped* addAssign(const TSourceLoc&, TOperator op, TIntermTyped* left, TIntermTyped* right);
void builtInOpCheck(const TSourceLoc&, const TFunction&, TIntermOperator&);
void nonOpBuiltInCheck(const TSourceLoc&, const TFunction&, TIntermAggregate&);
void userFunctionCallCheck(const TSourceLoc&, TIntermAggregate&);
......
glslang/MachineIndependent/Versions.cpp
......@@ -762,7 +762,8 @@ bool TParseVersions::checkExtensionsRequested(const TSourceLoc& loc, int numExte
// Use when there are no profile/version to check, it's just an error if one of the
// extensions is not present.
//
void TParseVersions::requireExtensions(const TSourceLoc& loc, int numExtensions, const char* const extensions[], const char* featureDesc)
void TParseVersions::requireExtensions(const TSourceLoc& loc, int numExtensions, const char* const extensions[],
const char* featureDesc)
{
if (checkExtensionsRequested(loc, numExtensions, extensions, featureDesc))
return;
......@@ -781,7 +782,8 @@ void TParseVersions::requireExtensions(const TSourceLoc& loc, int numExtensions,
// Use by preprocessor when there are no profile/version to check, it's just an error if one of the
// extensions is not present.
//
void TParseVersions::ppRequireExtensions(const TSourceLoc& loc, int numExtensions, const char* const extensions[], const char* featureDesc)
void TParseVersions::ppRequireExtensions(const TSourceLoc& loc, int numExtensions, const char* const extensions[],
const char* featureDesc)
{
if (checkExtensionsRequested(loc, numExtensions, extensions, featureDesc))
return;
......@@ -847,6 +849,7 @@ void TParseVersions::updateExtensionBehavior(int line, const char* extension, co
error(getCurrentLoc(), "behavior not supported:", "#extension", behaviorString);
return;
}
bool on = behavior != EBhDisable;
// check if extension is used with correct shader stage
checkExtensionStage(getCurrentLoc(), extension);
......@@ -916,6 +919,32 @@ void TParseVersions::updateExtensionBehavior(int line, const char* extension, co
updateExtensionBehavior(line, "GL_EXT_shader_explicit_arithmetic_types_int64", behaviorString);
else if (strcmp(extension, "GL_EXT_shader_subgroup_extended_types_float16") == 0)
updateExtensionBehavior(line, "GL_EXT_shader_explicit_arithmetic_types_float16", behaviorString);
// see if we need to update the numeric features
else if (strcmp(extension, "GL_EXT_shader_explicit_arithmetic_types") == 0)
intermediate.updateNumericFeature(TNumericFeatures::shader_explicit_arithmetic_types, on);
else if (strcmp(extension, "GL_EXT_shader_explicit_arithmetic_types_int8") == 0)
intermediate.updateNumericFeature(TNumericFeatures::shader_explicit_arithmetic_types_int8, on);
else if (strcmp(extension, "GL_EXT_shader_explicit_arithmetic_types_int16") == 0)
intermediate.updateNumericFeature(TNumericFeatures::shader_explicit_arithmetic_types_int16, on);
else if (strcmp(extension, "GL_EXT_shader_explicit_arithmetic_types_int32") == 0)
intermediate.updateNumericFeature(TNumericFeatures::shader_explicit_arithmetic_types_int32, on);
else if (strcmp(extension, "GL_EXT_shader_explicit_arithmetic_types_int64") == 0)
intermediate.updateNumericFeature(TNumericFeatures::shader_explicit_arithmetic_types_int64, on);
else if (strcmp(extension, "GL_EXT_shader_explicit_arithmetic_types_float16") == 0)
intermediate.updateNumericFeature(TNumericFeatures::shader_explicit_arithmetic_types_float16, on);
else if (strcmp(extension, "GL_EXT_shader_explicit_arithmetic_types_float32") == 0)
intermediate.updateNumericFeature(TNumericFeatures::shader_explicit_arithmetic_types_float32, on);
else if (strcmp(extension, "GL_EXT_shader_explicit_arithmetic_types_float64") == 0)
intermediate.updateNumericFeature(TNumericFeatures::shader_explicit_arithmetic_types_float64, on);
else if (strcmp(extension, "GL_EXT_shader_implicit_conversions") == 0)
intermediate.updateNumericFeature(TNumericFeatures::shader_implicit_conversions, on);
else if (strcmp(extension, "GL_ARB_gpu_shader_fp64") == 0)
intermediate.updateNumericFeature(TNumericFeatures::gpu_shader_fp64, on);
else if (strcmp(extension, "GL_AMD_gpu_shader_int16") == 0)
intermediate.updateNumericFeature(TNumericFeatures::gpu_shader_int16, on);
else if (strcmp(extension, "GL_AMD_gpu_shader_half_float") == 0)
intermediate.updateNumericFeature(TNumericFeatures::gpu_shader_half_float, on);
}
void TParseVersions::updateExtensionBehavior(const char* extension, TExtensionBehavior behavior)
......@@ -951,8 +980,8 @@ void TParseVersions::updateExtensionBehavior(const char* extension, TExtensionBe
} else {
if (iter->second == EBhDisablePartial)
warn(getCurrentLoc(), "extension is only partially supported:", "#extension", extension);
if (behavior == EBhEnable || behavior == EBhRequire || behavior == EBhDisable)
intermediate.updateRequestedExtension(extension, behavior);
if (behavior != EBhDisable)
intermediate.addRequestedExtension(extension);
iter->second = behavior;
}
}
......
glslang/MachineIndependent/glslang.m4
......@@ -778,7 +778,7 @@ assignment_expression
parseContext.specializationCheck($2.loc, $1->getType(), "=");
parseContext.lValueErrorCheck($2.loc, "assign", $1);
parseContext.rValueErrorCheck($2.loc, "assign", $3);
$$ = parseContext.intermediate.addAssign($2.op, $1, $3, $2.loc);
$$ = parseContext.addAssign($2.loc, $2.op, $1, $3);
if ($$ == 0) {
parseContext.assignError($2.loc, "assign", $1->getCompleteString(), $3->getCompleteString());
$$ = $1;
......
glslang/MachineIndependent/glslang.y
......@@ -778,7 +778,7 @@ assignment_expression
parseContext.specializationCheck($2.loc, $1->getType(), "=");
parseContext.lValueErrorCheck($2.loc, "assign", $1);
parseContext.rValueErrorCheck($2.loc, "assign", $3);
$$ = parseContext.intermediate.addAssign($2.op, $1, $3, $2.loc);
$$ = parseContext.addAssign($2.loc, $2.op, $1, $3);
if ($$ == 0) {
parseContext.assignError($2.loc, "assign", $1->getCompleteString(), $3->getCompleteString());
$$ = $1;
......@@ -3885,4 +3885,3 @@ single_attribute
%%
glslang/MachineIndependent/glslang_tab.cpp
This source diff could not be displayed because it is too large. You can view the blob instead.
glslang/MachineIndependent/glslang_tab.cpp.h
......@@ -30,8 +30,8 @@
This special exception was added by the Free Software Foundation in
version 2.2 of Bison. */
#ifndef YY_YY_GLSLANG_TAB_CPP_H_INCLUDED
# define YY_YY_GLSLANG_TAB_CPP_H_INCLUDED
#ifndef YY_YY_MACHINEINDEPENDENT_GLSLANG_TAB_CPP_H_INCLUDED
# define YY_YY_MACHINEINDEPENDENT_GLSLANG_TAB_CPP_H_INCLUDED
/* Debug traces. */
#ifndef YYDEBUG
# define YYDEBUG 1
......@@ -470,7 +470,7 @@ extern int yydebug;
union YYSTYPE
{
#line 97 "glslang.y" /* yacc.c:1909 */
#line 97 "MachineIndependent/glslang.y" /* yacc.c:1909 */
struct {
glslang::TSourceLoc loc;
......@@ -506,7 +506,7 @@ union YYSTYPE
glslang::TArraySizes* typeParameters;
} interm;
#line 510 "glslang_tab.cpp.h" /* yacc.c:1909 */
#line 510 "MachineIndependent/glslang_tab.cpp.h" /* yacc.c:1909 */
};
typedef union YYSTYPE YYSTYPE;
......@@ -518,4 +518,4 @@ typedef union YYSTYPE YYSTYPE;
int yyparse (glslang::TParseContext* pParseContext);
#endif /* !YY_YY_GLSLANG_TAB_CPP_H_INCLUDED */
#endif /* !YY_YY_MACHINEINDEPENDENT_GLSLANG_TAB_CPP_H_INCLUDED */
glslang/MachineIndependent/intermOut.cpp
......@@ -1466,7 +1466,7 @@ void TIntermediate::output(TInfoSink& infoSink, bool tree)
infoSink.debug << "Shader version: " << version << "\n";
if (requestedExtensions.size() > 0) {
for (auto extIt = requestedExtensions.begin(); extIt != requestedExtensions.end(); ++extIt)
infoSink.debug << "Requested " << extIt->first << "\n";
infoSink.debug << "Requested " << *extIt << "\n";
}
if (xfbMode)
......
glslang/MachineIndependent/localintermediate.h
......@@ -233,6 +233,31 @@ private:
TMap<TString, int> maps[EsiCount];
};
class TNumericFeatures {
public:
TNumericFeatures() : features(0) { }
TNumericFeatures(const TNumericFeatures&) = delete;
TNumericFeatures& operator=(const TNumericFeatures&) = delete;
typedef enum : unsigned int {
shader_explicit_arithmetic_types = 1 << 0,
shader_explicit_arithmetic_types_int8 = 1 << 1,
shader_explicit_arithmetic_types_int16 = 1 << 2,
shader_explicit_arithmetic_types_int32 = 1 << 3,
shader_explicit_arithmetic_types_int64 = 1 << 4,
shader_explicit_arithmetic_types_float16 = 1 << 5,
shader_explicit_arithmetic_types_float32 = 1 << 6,
shader_explicit_arithmetic_types_float64 = 1 << 7,
shader_implicit_conversions = 1 << 8,
gpu_shader_fp64 = 1 << 9,
gpu_shader_int16 = 1 << 10,
gpu_shader_half_float = 1 << 11,
} feature;
void insert(feature f) { features |= f; }
void erase(feature f) { features &= ~f; }
bool contains(feature f) const { return (features & f) != 0; }
private:
unsigned int features;
};
//
// Set of helper functions to help parse and build the tree.
......@@ -371,15 +396,8 @@ public:
}
const SpvVersion& getSpv() const { return spvVersion; }
EShLanguage getStage() const { return language; }
void updateRequestedExtension(const char* extension, TExtensionBehavior behavior) {
if(requestedExtensions.find(extension) != requestedExtensions.end()) {
requestedExtensions[extension] = behavior;
} else {
requestedExtensions.insert(std::make_pair(extension, behavior));
}
}
const std::map<std::string, TExtensionBehavior>& getRequestedExtensions() const { return requestedExtensions; }
void addRequestedExtension(const char* extension) { requestedExtensions.insert(extension); }
const std::set<std::string>& getRequestedExtensions() const { return requestedExtensions; }
void setTreeRoot(TIntermNode* r) { treeRoot = r; }
TIntermNode* getTreeRoot() const { return treeRoot; }
......@@ -425,15 +443,15 @@ public:
TIntermSymbol* addSymbol(const TType&, const TSourceLoc&);
TIntermSymbol* addSymbol(const TIntermSymbol&);
TIntermTyped* addConversion(TOperator, const TType&, TIntermTyped*);
std::tuple<TIntermTyped*, TIntermTyped*> addConversion(TOperator op, TIntermTyped* node0, TIntermTyped* node1);
std::tuple<TIntermTyped*, TIntermTyped*> addPairConversion(TOperator op, TIntermTyped* node0, TIntermTyped* node1);
TIntermTyped* addUniShapeConversion(TOperator, const TType&, TIntermTyped*);
TIntermTyped* addConversion(TBasicType convertTo, TIntermTyped* node) const;
void addBiShapeConversion(TOperator, TIntermTyped*& lhsNode, TIntermTyped*& rhsNode);
TIntermTyped* addShapeConversion(const TType&, TIntermTyped*);
TIntermTyped* addBinaryMath(TOperator, TIntermTyped* left, TIntermTyped* right, TSourceLoc);
TIntermTyped* addAssign(TOperator op, TIntermTyped* left, TIntermTyped* right, TSourceLoc);
TIntermTyped* addIndex(TOperator op, TIntermTyped* base, TIntermTyped* index, TSourceLoc);
TIntermTyped* addUnaryMath(TOperator, TIntermTyped* child, TSourceLoc);
TIntermTyped* addBinaryMath(TOperator, TIntermTyped* left, TIntermTyped* right, const TSourceLoc&);
TIntermTyped* addAssign(TOperator op, TIntermTyped* left, TIntermTyped* right, const TSourceLoc&);
TIntermTyped* addIndex(TOperator op, TIntermTyped* base, TIntermTyped* index, const TSourceLoc&);
TIntermTyped* addUnaryMath(TOperator, TIntermTyped* child, const TSourceLoc&);
TIntermTyped* addBuiltInFunctionCall(const TSourceLoc& line, TOperator, bool unary, TIntermNode*, const TType& returnType);
bool canImplicitlyPromote(TBasicType from, TBasicType to, TOperator op = EOpNull) const;
bool isIntegralPromotion(TBasicType from, TBasicType to) const;
......@@ -447,7 +465,7 @@ public:
TIntermAggregate* makeAggregate(TIntermNode* node);
TIntermAggregate* makeAggregate(TIntermNode* node, const TSourceLoc&);
TIntermAggregate* makeAggregate(const TSourceLoc&);
TIntermTyped* setAggregateOperator(TIntermNode*, TOperator, const TType& type, TSourceLoc);
TIntermTyped* setAggregateOperator(TIntermNode*, TOperator, const TType& type, const TSourceLoc&);
bool areAllChildConst(TIntermAggregate* aggrNode);
TIntermSelection* addSelection(TIntermTyped* cond, TIntermNodePair code, const TSourceLoc&);
TIntermTyped* addSelection(TIntermTyped* cond, TIntermTyped* trueBlock, TIntermTyped* falseBlock, const TSourceLoc&);
......@@ -476,10 +494,11 @@ public:
// Low level functions to add nodes (no conversions or other higher level transformations)
// If a type is provided, the node's type will be set to it.
TIntermBinary* addBinaryNode(TOperator op, TIntermTyped* left, TIntermTyped* right, TSourceLoc) const;
TIntermBinary* addBinaryNode(TOperator op, TIntermTyped* left, TIntermTyped* right, TSourceLoc, const TType&) const;
TIntermUnary* addUnaryNode(TOperator op, TIntermTyped* child, TSourceLoc) const;
TIntermUnary* addUnaryNode(TOperator op, TIntermTyped* child, TSourceLoc, const TType&) const;
TIntermBinary* addBinaryNode(TOperator op, TIntermTyped* left, TIntermTyped* right, const TSourceLoc&) const;
TIntermBinary* addBinaryNode(TOperator op, TIntermTyped* left, TIntermTyped* right, const TSourceLoc&,
const TType&) const;
TIntermUnary* addUnaryNode(TOperator op, TIntermTyped* child, const TSourceLoc&) const;
TIntermUnary* addUnaryNode(TOperator op, TIntermTyped* child, const TSourceLoc&, const TType&) const;
// Constant folding (in Constant.cpp)
TIntermTyped* fold(TIntermAggregate* aggrNode);
......@@ -866,22 +885,25 @@ public:
bool getArithemeticInt8Enabled() const { return false; }
bool getArithemeticInt16Enabled() const { return false; }
bool getArithemeticFloat16Enabled() const { return false; }
void updateNumericFeature(TNumericFeatures::feature f, bool on) { }
#else
bool getArithemeticInt8Enabled() const {
return extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types) ||
extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types_int8);
return numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types) ||
numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types_int8);
}
bool getArithemeticInt16Enabled() const {
return extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types) ||
extensionRequested(E_GL_AMD_gpu_shader_int16) ||
extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types_int16);
return numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types) ||
numericFeatures.contains(TNumericFeatures::gpu_shader_int16) ||
numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types_int16);
}
bool getArithemeticFloat16Enabled() const {
return extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types) ||
extensionRequested(E_GL_AMD_gpu_shader_half_float) ||
extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types_float16);
return numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types) ||
numericFeatures.contains(TNumericFeatures::gpu_shader_half_float) ||
numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types_float16);
}
void updateNumericFeature(TNumericFeatures::feature f, bool on)
{ on ? numericFeatures.insert(f) : numericFeatures.erase(f); }
#endif
protected:
......@@ -913,23 +935,7 @@ protected:
bool specConstantPropagates(const TIntermTyped&, const TIntermTyped&);
void performTextureUpgradeAndSamplerRemovalTransformation(TIntermNode* root);
bool isConversionAllowed(TOperator op, TIntermTyped* node) const;
std::tuple<TBasicType, TBasicType> getConversionDestinatonType(TBasicType type0, TBasicType type1, TOperator op) const;
// JohnK: I think this function should go away.
// This data structure is just a log to pass on to back ends.
// Versioning and extensions are handled in Version.cpp, with a rich
// set of functions for querying stages, versions, extension enable/disabled, etc.
#ifdef GLSLANG_WEB
bool extensionRequested(const char *extension) const { return false; }
#else
bool extensionRequested(const char *extension) const {
auto it = requestedExtensions.find(extension);
if (it != requestedExtensions.end()) {
return (it->second == EBhDisable) ? false : true;
}
return false;
}
#endif
std::tuple<TBasicType, TBasicType> getConversionDestinationType(TBasicType type0, TBasicType type1, TOperator op) const;
static const char* getResourceName(TResourceType);
......@@ -948,7 +954,7 @@ protected:
#endif
SpvVersion spvVersion;
TIntermNode* treeRoot;
std::map<std::string, TExtensionBehavior> requestedExtensions; // cumulation of all enabled or required extensions; not connected to what subset of the shader used them
std::set<std::string> requestedExtensions; // cumulation of all enabled or required extensions; not connected to what subset of the shader used them
TBuiltInResource resources;
int numEntryPoints;
int numErrors;
......@@ -1019,6 +1025,7 @@ protected:
std::unordered_map<std::string, int> uniformLocationOverrides;
int uniformLocationBase;
TNumericFeatures numericFeatures;
#endif
std::unordered_set<int> usedConstantId; // specialization constant ids used
......
glslang/MachineIndependent/parseVersions.h
......@@ -229,7 +229,7 @@ public:
TIntermediate& intermediate; // helper for making and hooking up pieces of the parse tree
protected:
TMap<TString, TExtensionBehavior> extensionBehavior; // for each extension string, what its current behavior is set to
TMap<TString, TExtensionBehavior> extensionBehavior; // for each extension string, what its current behavior is
TMap<TString, unsigned int> extensionMinSpv; // for each extension string, store minimum spirv required
EShMessages messages; // errors/warnings/rule-sets
int numErrors; // number of compile-time errors encountered
......
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