Skip to content

Commit 0138472

Browse files
johnkslangjohnkslang
committed
Fix KhronosGroup#2366, fix KhronosGroup#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
1 parent 7d66a5d commit 0138472

File tree

8 files changed

+109
-74
lines changed

8 files changed

+109
-74
lines changed

‎SPIRV/GlslangToSpv.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1444,7 +1444,7 @@ TGlslangToSpvTraverser::TGlslangToSpvTraverser(unsigned int spvVersion,
14441444
// Add the source extensions
14451445
const auto& sourceExtensions = glslangIntermediate->getRequestedExtensions();
14461446
for (auto it = sourceExtensions.begin(); it != sourceExtensions.end(); ++it)
1447-
builder.addSourceExtension(it->first.c_str());
1447+
builder.addSourceExtension(it->c_str());
14481448

14491449
// Add the top-level modes for this shader.
14501450

‎Test/baseResults/versionsErrors.frag.out

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ ERROR: 4 compilation errors. No code generated.
77

88

99
Shader version: 110
10-
Requested GL_ARB_texture_rectangle
1110
ERROR: node is still EOpNull!
1211
0:42 Function Definition: main( ( global void)
1312
0:42 Function Parameters:
@@ -28,7 +27,6 @@ Linked fragment stage:
2827

2928

3029
Shader version: 110
31-
Requested GL_ARB_texture_rectangle
3230
ERROR: node is still EOpNull!
3331
0:42 Function Definition: main( ( global void)
3432
0:42 Function Parameters:

‎glslang/MachineIndependent/Intermediate.cpp

Lines changed: 31 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1162,18 +1162,18 @@ TIntermTyped* TIntermediate::addConversion(TOperator op, const TType& type, TInt
11621162
// - at the time of this writing (14-Aug-2020), no test results are changed by this.
11631163
switch (op) {
11641164
case EOpConstructFloat16:
1165-
canPromoteConstant = extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types) ||
1166-
extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types_float16);
1165+
canPromoteConstant = numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types) ||
1166+
numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types_float16);
11671167
break;
11681168
case EOpConstructInt8:
11691169
case EOpConstructUint8:
1170-
canPromoteConstant = extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types) ||
1171-
extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types_int8);
1170+
canPromoteConstant = numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types) ||
1171+
numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types_int8);
11721172
break;
11731173
case EOpConstructInt16:
11741174
case EOpConstructUint16:
1175-
canPromoteConstant = extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types) ||
1176-
extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types_int16);
1175+
canPromoteConstant = numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types) ||
1176+
numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types_int16);
11771177
break;
11781178
}
11791179
#endif
@@ -1665,14 +1665,14 @@ bool TIntermediate::canImplicitlyPromote(TBasicType from, TBasicType to, TOperat
16651665
isFPConversion(from, to) ||
16661666
isFPIntegralConversion(from, to)) {
16671667

1668-
if (extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types) ||
1669-
extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types_int8) ||
1670-
extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types_int16) ||
1671-
extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types_int32) ||
1672-
extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types_int64) ||
1673-
extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types_float16) ||
1674-
extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types_float32) ||
1675-
extensionRequested(E_GL_EXT_shader_explicit_arithmetic_types_float64)) {
1668+
if (numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types) ||
1669+
numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types_int8) ||
1670+
numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types_int16) ||
1671+
numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types_int32) ||
1672+
numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types_int64) ||
1673+
numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types_float16) ||
1674+
numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types_float32) ||
1675+
numericFeatures.contains(TNumericFeatures::shader_explicit_arithmetic_types_float64)) {
16761676
return true;
16771677
}
16781678
}
@@ -1684,14 +1684,14 @@ bool TIntermediate::canImplicitlyPromote(TBasicType from, TBasicType to, TOperat
16841684
switch (from) {
16851685
case EbtInt:
16861686
case EbtUint:
1687-
return extensionRequested(E_GL_EXT_shader_implicit_conversions);
1687+
return numericFeatures.contains(TNumericFeatures::shader_implicit_conversions);
16881688
default:
16891689
return false;
16901690
}
16911691
case EbtUint:
16921692
switch (from) {
16931693
case EbtInt:
1694-
return extensionRequested(E_GL_EXT_shader_implicit_conversions);
1694+
return numericFeatures.contains(TNumericFeatures::shader_implicit_conversions);
16951695
default:
16961696
return false;
16971697
}
@@ -1707,14 +1707,14 @@ bool TIntermediate::canImplicitlyPromote(TBasicType from, TBasicType to, TOperat
17071707
case EbtInt64:
17081708
case EbtUint64:
17091709
case EbtFloat:
1710-
return version >= 400 || extensionRequested(E_GL_ARB_gpu_shader_fp64);
1710+
return version >= 400 || numericFeatures.contains(TNumericFeatures::gpu_shader_fp64);
17111711
case EbtInt16:
17121712
case EbtUint16:
1713-
return (version >= 400 || extensionRequested(E_GL_ARB_gpu_shader_fp64)) &&
1714-
extensionRequested(E_GL_AMD_gpu_shader_int16);
1713+
return (version >= 400 || numericFeatures.contains(TNumericFeatures::gpu_shader_fp64)) &&
1714+
numericFeatures.contains(TNumericFeatures::gpu_shader_int16);
17151715
case EbtFloat16:
1716-
return (version >= 400 || extensionRequested(E_GL_ARB_gpu_shader_fp64)) &&
1717-
extensionRequested(E_GL_AMD_gpu_shader_half_float);
1716+
return (version >= 400 || numericFeatures.contains(TNumericFeatures::gpu_shader_fp64)) &&
1717+
numericFeatures.contains(TNumericFeatures::gpu_shader_half_float);
17181718
default:
17191719
return false;
17201720
}
@@ -1727,10 +1727,10 @@ bool TIntermediate::canImplicitlyPromote(TBasicType from, TBasicType to, TOperat
17271727
return getSource() == EShSourceHlsl;
17281728
case EbtInt16:
17291729
case EbtUint16:
1730-
return extensionRequested(E_GL_AMD_gpu_shader_int16);
1730+
return numericFeatures.contains(TNumericFeatures::gpu_shader_int16);
17311731
case EbtFloat16:
1732-
return
1733-
extensionRequested(E_GL_AMD_gpu_shader_half_float) || getSource() == EShSourceHlsl;
1732+
return numericFeatures.contains(TNumericFeatures::gpu_shader_half_float) ||
1733+
getSource() == EShSourceHlsl;
17341734
default:
17351735
return false;
17361736
}
@@ -1742,7 +1742,7 @@ bool TIntermediate::canImplicitlyPromote(TBasicType from, TBasicType to, TOperat
17421742
return getSource() == EShSourceHlsl;
17431743
case EbtInt16:
17441744
case EbtUint16:
1745-
return extensionRequested(E_GL_AMD_gpu_shader_int16);
1745+
return numericFeatures.contains(TNumericFeatures::gpu_shader_int16);
17461746
default:
17471747
return false;
17481748
}
@@ -1751,7 +1751,7 @@ bool TIntermediate::canImplicitlyPromote(TBasicType from, TBasicType to, TOperat
17511751
case EbtBool:
17521752
return getSource() == EShSourceHlsl;
17531753
case EbtInt16:
1754-
return extensionRequested(E_GL_AMD_gpu_shader_int16);
1754+
return numericFeatures.contains(TNumericFeatures::gpu_shader_int16);
17551755
default:
17561756
return false;
17571757
}
@@ -1763,7 +1763,7 @@ bool TIntermediate::canImplicitlyPromote(TBasicType from, TBasicType to, TOperat
17631763
return true;
17641764
case EbtInt16:
17651765
case EbtUint16:
1766-
return extensionRequested(E_GL_AMD_gpu_shader_int16);
1766+
return numericFeatures.contains(TNumericFeatures::gpu_shader_int16);
17671767
default:
17681768
return false;
17691769
}
@@ -1772,23 +1772,23 @@ bool TIntermediate::canImplicitlyPromote(TBasicType from, TBasicType to, TOperat
17721772
case EbtInt:
17731773
return true;
17741774
case EbtInt16:
1775-
return extensionRequested(E_GL_AMD_gpu_shader_int16);
1775+
return numericFeatures.contains(TNumericFeatures::gpu_shader_int16);
17761776
default:
17771777
return false;
17781778
}
17791779
case EbtFloat16:
17801780
switch (from) {
17811781
case EbtInt16:
17821782
case EbtUint16:
1783-
return extensionRequested(E_GL_AMD_gpu_shader_int16);
1783+
return numericFeatures.contains(TNumericFeatures::gpu_shader_int16);
17841784
default:
17851785
break;
17861786
}
17871787
return false;
17881788
case EbtUint16:
17891789
switch (from) {
17901790
case EbtInt16:
1791-
return extensionRequested(E_GL_AMD_gpu_shader_int16);
1791+
return numericFeatures.contains(TNumericFeatures::gpu_shader_int16);
17921792
default:
17931793
break;
17941794
}
@@ -1921,7 +1921,7 @@ std::tuple<TBasicType, TBasicType> TIntermediate::getConversionDestinationType(T
19211921
TBasicType res1 = EbtNumTypes;
19221922

19231923
if ((isEsProfile() &&
1924-
(version < 310 || !extensionRequested(E_GL_EXT_shader_implicit_conversions))) ||
1924+
(version < 310 || !numericFeatures.contains(TNumericFeatures::shader_implicit_conversions))) ||
19251925
version == 110)
19261926
return std::make_tuple(res0, res1);
19271927

‎glslang/MachineIndependent/ParseHelper.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7316,6 +7316,8 @@ TIntermTyped* TParseContext::constructBuiltIn(const TType& type, TOperator op, T
73167316
if (!node->getType().isCoopMat()) {
73177317
if (type.getBasicType() != node->getType().getBasicType()) {
73187318
node = intermediate.addConversion(type.getBasicType(), node);
7319+
if (node == nullptr)
7320+
return nullptr;
73197321
}
73207322
node = intermediate.setAggregateOperator(node, EOpConstructCooperativeMatrix, type, node->getLoc());
73217323
} else {

‎glslang/MachineIndependent/Versions.cpp

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -762,7 +762,8 @@ bool TParseVersions::checkExtensionsRequested(const TSourceLoc& loc, int numExte
762762
// Use when there are no profile/version to check, it's just an error if one of the
763763
// extensions is not present.
764764
//
765-
void TParseVersions::requireExtensions(const TSourceLoc& loc, int numExtensions, const char* const extensions[], const char* featureDesc)
765+
void TParseVersions::requireExtensions(const TSourceLoc& loc, int numExtensions, const char* const extensions[],
766+
const char* featureDesc)
766767
{
767768
if (checkExtensionsRequested(loc, numExtensions, extensions, featureDesc))
768769
return;
@@ -781,7 +782,8 @@ void TParseVersions::requireExtensions(const TSourceLoc& loc, int numExtensions,
781782
// Use by preprocessor when there are no profile/version to check, it's just an error if one of the
782783
// extensions is not present.
783784
//
784-
void TParseVersions::ppRequireExtensions(const TSourceLoc& loc, int numExtensions, const char* const extensions[], const char* featureDesc)
785+
void TParseVersions::ppRequireExtensions(const TSourceLoc& loc, int numExtensions, const char* const extensions[],
786+
const char* featureDesc)
785787
{
786788
if (checkExtensionsRequested(loc, numExtensions, extensions, featureDesc))
787789
return;
@@ -847,6 +849,7 @@ void TParseVersions::updateExtensionBehavior(int line, const char* extension, co
847849
error(getCurrentLoc(), "behavior not supported:", "#extension", behaviorString);
848850
return;
849851
}
852+
bool on = behavior != EBhDisable;
850853

851854
// check if extension is used with correct shader stage
852855
checkExtensionStage(getCurrentLoc(), extension);
@@ -916,6 +919,32 @@ void TParseVersions::updateExtensionBehavior(int line, const char* extension, co
916919
updateExtensionBehavior(line, "GL_EXT_shader_explicit_arithmetic_types_int64", behaviorString);
917920
else if (strcmp(extension, "GL_EXT_shader_subgroup_extended_types_float16") == 0)
918921
updateExtensionBehavior(line, "GL_EXT_shader_explicit_arithmetic_types_float16", behaviorString);
922+
923+
// see if we need to update the numeric features
924+
else if (strcmp(extension, "GL_EXT_shader_explicit_arithmetic_types") == 0)
925+
intermediate.updateNumericFeature(TNumericFeatures::shader_explicit_arithmetic_types, on);
926+
else if (strcmp(extension, "GL_EXT_shader_explicit_arithmetic_types_int8") == 0)
927+
intermediate.updateNumericFeature(TNumericFeatures::shader_explicit_arithmetic_types_int8, on);
928+
else if (strcmp(extension, "GL_EXT_shader_explicit_arithmetic_types_int16") == 0)
929+
intermediate.updateNumericFeature(TNumericFeatures::shader_explicit_arithmetic_types_int16, on);
930+
else if (strcmp(extension, "GL_EXT_shader_explicit_arithmetic_types_int32") == 0)
931+
intermediate.updateNumericFeature(TNumericFeatures::shader_explicit_arithmetic_types_int32, on);
932+
else if (strcmp(extension, "GL_EXT_shader_explicit_arithmetic_types_int64") == 0)
933+
intermediate.updateNumericFeature(TNumericFeatures::shader_explicit_arithmetic_types_int64, on);
934+
else if (strcmp(extension, "GL_EXT_shader_explicit_arithmetic_types_float16") == 0)
935+
intermediate.updateNumericFeature(TNumericFeatures::shader_explicit_arithmetic_types_float16, on);
936+
else if (strcmp(extension, "GL_EXT_shader_explicit_arithmetic_types_float32") == 0)
937+
intermediate.updateNumericFeature(TNumericFeatures::shader_explicit_arithmetic_types_float32, on);
938+
else if (strcmp(extension, "GL_EXT_shader_explicit_arithmetic_types_float64") == 0)
939+
intermediate.updateNumericFeature(TNumericFeatures::shader_explicit_arithmetic_types_float64, on);
940+
else if (strcmp(extension, "GL_EXT_shader_implicit_conversions") == 0)
941+
intermediate.updateNumericFeature(TNumericFeatures::shader_implicit_conversions, on);
942+
else if (strcmp(extension, "GL_ARB_gpu_shader_fp64") == 0)
943+
intermediate.updateNumericFeature(TNumericFeatures::gpu_shader_fp64, on);
944+
else if (strcmp(extension, "GL_AMD_gpu_shader_int16") == 0)
945+
intermediate.updateNumericFeature(TNumericFeatures::gpu_shader_int16, on);
946+
else if (strcmp(extension, "GL_AMD_gpu_shader_half_float") == 0)
947+
intermediate.updateNumericFeature(TNumericFeatures::gpu_shader_half_float, on);
919948
}
920949

921950
void TParseVersions::updateExtensionBehavior(const char* extension, TExtensionBehavior behavior)
@@ -951,8 +980,8 @@ void TParseVersions::updateExtensionBehavior(const char* extension, TExtensionBe
951980
} else {
952981
if (iter->second == EBhDisablePartial)
953982
warn(getCurrentLoc(), "extension is only partially supported:", "#extension", extension);
954-
if (behavior == EBhEnable || behavior == EBhRequire || behavior == EBhDisable)
955-
intermediate.updateRequestedExtension(extension, behavior);
983+
if (behavior != EBhDisable)
984+
intermediate.addRequestedExtension(extension);
956985
iter->second = behavior;
957986
}
958987
}

‎glslang/MachineIndependent/intermOut.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1466,7 +1466,7 @@ void TIntermediate::output(TInfoSink& infoSink, bool tree)
14661466
infoSink.debug << "Shader version: " << version << "\n";
14671467
if (requestedExtensions.size() > 0) {
14681468
for (auto extIt = requestedExtensions.begin(); extIt != requestedExtensions.end(); ++extIt)
1469-
infoSink.debug << "Requested " << extIt->first << "\n";
1469+
infoSink.debug << "Requested " << *extIt << "\n";
14701470
}
14711471

14721472
if (xfbMode)

0 commit comments

Comments
 (0)