Fix issue #313: Catch internal attempts to modify built-in symbols that don't exist.

Also beefed up support for running compute shaders is #version 420, but this
work is only partially done.
This commit is contained in:
John Kessenich 2016-05-29 18:24:31 -06:00
parent 5a7f0eff69
commit 0f5e3ad23c
6 changed files with 70 additions and 7 deletions

7
Test/420.comp Executable file
View File

@ -0,0 +1,7 @@
#version 420
layout(local_size_x = 2) in; // ERROR, no compute
#extension GL_ARB_compute_shader : enable
layout(local_size_x = 2) in;

32
Test/baseResults/420.comp.out Executable file
View File

@ -0,0 +1,32 @@
420.comp
Warning, version 420 is not yet complete; most version-specific features are present, but some are missing.
ERROR: 0:3: 'gl_WorkgroupSize' : not supported for this version or the enabled extensions
WARNING: 0:5: '#extension' : extension is only partially supported: GL_ARB_compute_shader
ERROR: 1 compilation errors. No code generated.
Shader version: 420
Requested GL_ARB_compute_shader
local_size = (2, 1, 1)
ERROR: node is still EOpNull!
0:? Linker Objects
0:? 'gl_WorkGroupSize' (const 3-component vector of uint WorkGroupSize)
0:? 2 (const uint)
0:? 1 (const uint)
0:? 1 (const uint)
Linked compute stage:
ERROR: Linking compute stage: Missing entry point: Each stage requires one "void main()" entry point
Shader version: 420
Requested GL_ARB_compute_shader
local_size = (2, 1, 1)
ERROR: node is still EOpNull!
0:? Linker Objects
0:? 'gl_WorkGroupSize' (const 3-component vector of uint WorkGroupSize)
0:? 2 (const uint)
0:? 1 (const uint)
0:? 1 (const uint)

View File

@ -1871,7 +1871,7 @@ void TBuiltIns::initialize(int version, EProfile profile, int spv, int vulkan)
// //
//============================================================================ //============================================================================
if ((profile != EEsProfile && version >= 430) || if ((profile != EEsProfile && version >= 420) ||
(profile == EEsProfile && version >= 310)) { (profile == EEsProfile && version >= 310)) {
stageBuiltins[EShLangCompute].append( stageBuiltins[EShLangCompute].append(
"in highp uvec3 gl_NumWorkGroups;" "in highp uvec3 gl_NumWorkGroups;"
@ -3896,6 +3896,15 @@ void TBuiltIns::identifyBuiltIns(int version, EProfile profile, int spv, int vul
BuiltInVariable("gl_LocalInvocationID", EbvLocalInvocationId, symbolTable); BuiltInVariable("gl_LocalInvocationID", EbvLocalInvocationId, symbolTable);
BuiltInVariable("gl_GlobalInvocationID", EbvGlobalInvocationId, symbolTable); BuiltInVariable("gl_GlobalInvocationID", EbvGlobalInvocationId, symbolTable);
BuiltInVariable("gl_LocalInvocationIndex", EbvLocalInvocationIndex, symbolTable); BuiltInVariable("gl_LocalInvocationIndex", EbvLocalInvocationIndex, symbolTable);
if (profile != EEsProfile && version < 430) {
symbolTable.setVariableExtensions("gl_NumWorkGroups", 1, &E_GL_ARB_compute_shader);
symbolTable.setVariableExtensions("gl_WorkGroupSize", 1, &E_GL_ARB_compute_shader);
symbolTable.setVariableExtensions("gl_WorkGroupID", 1, &E_GL_ARB_compute_shader);
symbolTable.setVariableExtensions("gl_LocalInvocationID", 1, &E_GL_ARB_compute_shader);
symbolTable.setVariableExtensions("gl_GlobalInvocationID", 1, &E_GL_ARB_compute_shader);
symbolTable.setVariableExtensions("gl_LocalInvocationIndex", 1, &E_GL_ARB_compute_shader);
}
break; break;
default: default:

View File

@ -622,10 +622,20 @@ void TParseContext::makeEditable(TSymbol*& symbol)
intermediate.addSymbolLinkageNode(linkage, *symbol); intermediate.addSymbolLinkageNode(linkage, *symbol);
} }
// Return a writable version of the variable 'name'.
//
// Return nullptr if 'name' is not found. This should mean
// something is seriously wrong (e.g., compiler asking self for
// built-in that doesn't exist).
TVariable* TParseContext::getEditableVariable(const char* name) TVariable* TParseContext::getEditableVariable(const char* name)
{ {
bool builtIn; bool builtIn;
TSymbol* symbol = symbolTable.find(name, &builtIn); TSymbol* symbol = symbolTable.find(name, &builtIn);
assert(symbol != nullptr);
if (symbol == nullptr)
return nullptr;
if (builtIn) if (builtIn)
makeEditable(symbol); makeEditable(symbol);
@ -3844,7 +3854,7 @@ void TParseContext::finalErrorCheck()
break; break;
case EShLangCompute: case EShLangCompute:
if (profile != EEsProfile && version < 430) if (profile != EEsProfile && version < 430)
requireExtensions(getCurrentLoc(), 1, &E_GL_ARB_compute_shader, "tessellation shaders"); requireExtensions(getCurrentLoc(), 1, &E_GL_ARB_compute_shader, "compute shaders");
break; break;
default: default:
break; break;
@ -4222,6 +4232,8 @@ void TParseContext::setLayoutQualifier(const TSourceLoc& loc, TPublicType& publi
case EShLangCompute: case EShLangCompute:
if (id.compare(0, 11, "local_size_") == 0) { if (id.compare(0, 11, "local_size_") == 0) {
profileRequires(loc, EEsProfile, 310, 0, "gl_WorkgroupSize");
profileRequires(loc, ~EEsProfile, 430, E_GL_ARB_compute_shader, "gl_WorkgroupSize");
if (id == "local_size_x") { if (id == "local_size_x") {
publicType.shaderQualifiers.localSize[0] = value; publicType.shaderQualifiers.localSize[0] = value;
return; return;
@ -5967,7 +5979,8 @@ void TParseContext::updateStandaloneQualifierDefaults(const TSourceLoc& loc, con
// Fix the existing constant gl_WorkGroupSize with this new information. // Fix the existing constant gl_WorkGroupSize with this new information.
TVariable* workGroupSize = getEditableVariable("gl_WorkGroupSize"); TVariable* workGroupSize = getEditableVariable("gl_WorkGroupSize");
workGroupSize->getWritableConstArray()[i].setUConst(intermediate.getLocalSize(i)); if (workGroupSize != nullptr)
workGroupSize->getWritableConstArray()[i].setUConst(intermediate.getLocalSize(i));
} }
} else } else
error(loc, "can only apply to 'in'", "local_size", ""); error(loc, "can only apply to 'in'", "local_size", "");
@ -5980,7 +5993,8 @@ void TParseContext::updateStandaloneQualifierDefaults(const TSourceLoc& loc, con
error(loc, "can only apply to 'in'", "local_size id", ""); error(loc, "can only apply to 'in'", "local_size id", "");
// Set the workgroup built-in variable as a specialization constant // Set the workgroup built-in variable as a specialization constant
TVariable* workGroupSize = getEditableVariable("gl_WorkGroupSize"); TVariable* workGroupSize = getEditableVariable("gl_WorkGroupSize");
workGroupSize->getWritableType().getQualifier().specConstant = true; if (workGroupSize != nullptr)
workGroupSize->getWritableType().getQualifier().specConstant = true;
} }
} }
if (publicType.shaderQualifiers.earlyFragmentTests) { if (publicType.shaderQualifiers.earlyFragmentTests) {

View File

@ -235,7 +235,7 @@ bool InitializeSymbolTables(TInfoSink& infoSink, TSymbolTable** commonTable, TS
InitializeStageSymbolTable(*builtInParseables, version, profile, spv, vulkan, EShLangGeometry, infoSink, commonTable, symbolTables); InitializeStageSymbolTable(*builtInParseables, version, profile, spv, vulkan, EShLangGeometry, infoSink, commonTable, symbolTables);
// check for compute // check for compute
if ((profile != EEsProfile && version >= 430) || if ((profile != EEsProfile && version >= 420) ||
(profile == EEsProfile && version >= 310)) (profile == EEsProfile && version >= 310))
InitializeStageSymbolTable(*builtInParseables, version, profile, spv, vulkan, EShLangCompute, infoSink, commonTable, symbolTables); InitializeStageSymbolTable(*builtInParseables, version, profile, spv, vulkan, EShLangCompute, infoSink, commonTable, symbolTables);
@ -417,7 +417,7 @@ bool DeduceVersionProfile(TInfoSink& infoSink, EShLanguage stage, bool versionNo
(profile != EEsProfile && version < 420)) { (profile != EEsProfile && version < 420)) {
correct = false; correct = false;
infoSink.info.message(EPrefixError, "#version: compute shaders require es profile with version 310 or above, or non-es profile with version 420 or above"); infoSink.info.message(EPrefixError, "#version: compute shaders require es profile with version 310 or above, or non-es profile with version 420 or above");
version = profile == EEsProfile ? 310 : 430; // 420 supports the extension, correction is to 430 which does not version = profile == EEsProfile ? 310 : 420;
} }
break; break;
default: default:

View File

@ -113,6 +113,7 @@ INSTANTIATE_TEST_CASE_P(
"110scope.vert", "110scope.vert",
"300scope.vert", "300scope.vert",
"400.frag", "400.frag",
"420.comp",
"420.frag", "420.frag",
"420.vert", "420.vert",
"420.geom", "420.geom",