Fix #2366, fix #2358, correctly separate out numerical feature checking
We need separate concepts for
- total set of extensions ever enabled, for the back end
- current state of extensions, for parsing
- the set of features currently enabled for building the AST
diff --git a/SPIRV/GlslangToSpv.cpp b/SPIRV/GlslangToSpv.cpp
index 5a39c1e..c855416 100644
--- a/SPIRV/GlslangToSpv.cpp
+++ b/SPIRV/GlslangToSpv.cpp
@@ -1444,7 +1444,7 @@
// 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.
diff --git a/Test/baseResults/versionsErrors.frag.out b/Test/baseResults/versionsErrors.frag.out
index b072669..dbeb941 100644
--- a/Test/baseResults/versionsErrors.frag.out
+++ b/Test/baseResults/versionsErrors.frag.out
@@ -7,7 +7,6 @@
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 @@
Shader version: 110
-Requested GL_ARB_texture_rectangle
ERROR: node is still EOpNull!
0:42 Function Definition: main( ( global void)
0:42 Function Parameters:
diff --git a/glslang/MachineIndependent/Intermediate.cpp b/glslang/MachineIndependent/Intermediate.cpp
index 0504ac8..17e3fbc 100755
--- a/glslang/MachineIndependent/Intermediate.cpp
+++ b/glslang/MachineIndependent/Intermediate.cpp
@@ -1162,18 +1162,18 @@
// - at the time of this writing (14-Aug-2020), no test results are changed by this.
switch (op) {
case EOpConstructFloat16:
- canPromoteConstant = extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types) ||
- extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types_float16);
+ canPromoteConstant = numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types) ||
+ numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types_float16);
break;
case EOpConstructInt8:
case EOpConstructUint8:
- canPromoteConstant = extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types) ||
- extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types_int8);
+ canPromoteConstant = numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types) ||
+ numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types_int8);
break;
case EOpConstructInt16:
case EOpConstructUint16:
- canPromoteConstant = extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types) ||
- extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types_int16);
+ canPromoteConstant = numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types) ||
+ numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types_int16);
break;
}
#endif
@@ -1665,14 +1665,14 @@
isFPConversion(from, to) ||
isFPIntegralConversion(from, to)) {
- if (extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types) ||
- extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types_int8) ||
- extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types_int16) ||
- extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types_int32) ||
- extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types_int64) ||
- extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types_float16) ||
- extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types_float32) ||
- extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types_float64)) {
+ if (numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types) ||
+ numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types_int8) ||
+ numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types_int16) ||
+ numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types_int32) ||
+ numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types_int64) ||
+ numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types_float16) ||
+ numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types_float32) ||
+ numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types_float64)) {
return true;
}
}
@@ -1684,14 +1684,14 @@
switch (from) {
case EbtInt:
case EbtUint:
- return extensionRequested(E_GL_EXT_shader_implicit_conversions);
+ return numericFeatures.contains(TNumericFeatures::shader_implicit_conversions);
default:
return false;
}
case EbtUint:
switch (from) {
case EbtInt:
- return extensionRequested(E_GL_EXT_shader_implicit_conversions);
+ return numericFeatures.contains(TNumericFeatures::shader_implicit_conversions);
default:
return false;
}
@@ -1707,14 +1707,14 @@
case EbtInt64:
case EbtUint64:
case EbtFloat:
- return version >= 400 || extensionRequested(E_GL_ARB_gpu_shader_fp64);
+ return version >= 400 || numericFeatures.contains(TNumericFeatures::gpu_shader_fp64);
case EbtInt16:
case EbtUint16:
- return (version >= 400 || extensionRequested(E_GL_ARB_gpu_shader_fp64)) &&
- extensionRequested(E_GL_AMD_gpu_shader_int16);
+ return (version >= 400 || numericFeatures.contains(TNumericFeatures::gpu_shader_fp64)) &&
+ numericFeatures.contains(TNumericFeatures::gpu_shader_int16);
case EbtFloat16:
- return (version >= 400 || extensionRequested(E_GL_ARB_gpu_shader_fp64)) &&
- extensionRequested(E_GL_AMD_gpu_shader_half_float);
+ return (version >= 400 || numericFeatures.contains(TNumericFeatures::gpu_shader_fp64)) &&
+ numericFeatures.contains(TNumericFeatures::gpu_shader_half_float);
default:
return false;
}
@@ -1727,10 +1727,10 @@
return getSource() == EShSourceHlsl;
case EbtInt16:
case EbtUint16:
- return extensionRequested(E_GL_AMD_gpu_shader_int16);
+ return numericFeatures.contains(TNumericFeatures::gpu_shader_int16);
case EbtFloat16:
- return
- extensionRequested(E_GL_AMD_gpu_shader_half_float) || getSource() == EShSourceHlsl;
+ return numericFeatures.contains(TNumericFeatures::gpu_shader_half_float) ||
+ getSource() == EShSourceHlsl;
default:
return false;
}
@@ -1742,7 +1742,7 @@
return getSource() == EShSourceHlsl;
case EbtInt16:
case EbtUint16:
- return extensionRequested(E_GL_AMD_gpu_shader_int16);
+ return numericFeatures.contains(TNumericFeatures::gpu_shader_int16);
default:
return false;
}
@@ -1751,7 +1751,7 @@
case EbtBool:
return getSource() == EShSourceHlsl;
case EbtInt16:
- return extensionRequested(E_GL_AMD_gpu_shader_int16);
+ return numericFeatures.contains(TNumericFeatures::gpu_shader_int16);
default:
return false;
}
@@ -1763,7 +1763,7 @@
return true;
case EbtInt16:
case EbtUint16:
- return extensionRequested(E_GL_AMD_gpu_shader_int16);
+ return numericFeatures.contains(TNumericFeatures::gpu_shader_int16);
default:
return false;
}
@@ -1772,7 +1772,7 @@
case EbtInt:
return true;
case EbtInt16:
- return extensionRequested(E_GL_AMD_gpu_shader_int16);
+ return numericFeatures.contains(TNumericFeatures::gpu_shader_int16);
default:
return false;
}
@@ -1780,7 +1780,7 @@
switch (from) {
case EbtInt16:
case EbtUint16:
- return extensionRequested(E_GL_AMD_gpu_shader_int16);
+ return numericFeatures.contains(TNumericFeatures::gpu_shader_int16);
default:
break;
}
@@ -1788,7 +1788,7 @@
case EbtUint16:
switch (from) {
case EbtInt16:
- return extensionRequested(E_GL_AMD_gpu_shader_int16);
+ return numericFeatures.contains(TNumericFeatures::gpu_shader_int16);
default:
break;
}
@@ -1921,7 +1921,7 @@
TBasicType res1 = EbtNumTypes;
if ((isEsProfile() &&
- (version < 310 || !extensionRequested(E_GL_EXT_shader_implicit_conversions))) ||
+ (version < 310 || !numericFeatures.contains(TNumericFeatures::shader_implicit_conversions))) ||
version == 110)
return std::make_tuple(res0, res1);
diff --git a/glslang/MachineIndependent/ParseHelper.cpp b/glslang/MachineIndependent/ParseHelper.cpp
index 2c4b086..045c8f3 100644
--- a/glslang/MachineIndependent/ParseHelper.cpp
+++ b/glslang/MachineIndependent/ParseHelper.cpp
@@ -7316,6 +7316,8 @@
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 {
diff --git a/glslang/MachineIndependent/Versions.cpp b/glslang/MachineIndependent/Versions.cpp
index b311024..7469ef1 100644
--- a/glslang/MachineIndependent/Versions.cpp
+++ b/glslang/MachineIndependent/Versions.cpp
@@ -762,7 +762,8 @@
// 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 @@
// 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 @@
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 @@
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 @@
} 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;
}
}
diff --git a/glslang/MachineIndependent/intermOut.cpp b/glslang/MachineIndependent/intermOut.cpp
index 1b409df..f23a705 100644
--- a/glslang/MachineIndependent/intermOut.cpp
+++ b/glslang/MachineIndependent/intermOut.cpp
@@ -1466,7 +1466,7 @@
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)
diff --git a/glslang/MachineIndependent/localintermediate.h b/glslang/MachineIndependent/localintermediate.h
index 4345a09..3cdc1f1 100644
--- a/glslang/MachineIndependent/localintermediate.h
+++ b/glslang/MachineIndependent/localintermediate.h
@@ -233,6 +233,31 @@
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 @@
}
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; }
@@ -867,22 +885,25 @@
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:
@@ -916,22 +937,6 @@
bool isConversionAllowed(TOperator op, TIntermTyped* node) const;
std::tuple<TBasicType, TBasicType> getConversionDestinationType(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
-
static const char* getResourceName(TResourceType);
const EShLanguage language; // stage, known at construction time
@@ -949,7 +954,7 @@
#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;
@@ -1020,6 +1025,7 @@
std::unordered_map<std::string, int> uniformLocationOverrides;
int uniformLocationBase;
+ TNumericFeatures numericFeatures;
#endif
std::unordered_set<int> usedConstantId; // specialization constant ids used
diff --git a/glslang/MachineIndependent/parseVersions.h b/glslang/MachineIndependent/parseVersions.h
index 957aaef..7248354 100644
--- a/glslang/MachineIndependent/parseVersions.h
+++ b/glslang/MachineIndependent/parseVersions.h
@@ -229,7 +229,7 @@
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
