On Wed, Aug 24, 2016 at 6:19 PM, Ian Romanick <[email protected]> wrote: > On 08/24/2016 12:57 PM, Ilia Mirkin wrote: >> On Wed, Aug 24, 2016 at 3:40 PM, Ian Romanick <[email protected]> wrote: >>> On 08/24/2016 10:40 AM, Ilia Mirkin wrote: >>>> The GL_ARB_compute_shader spec does not make a mention of a #extension >>>> enable, nor a #define with the ext name. It follows that local_size_* >>>> should always be allowed in a compute shader stage, and the #extension >>>> support removed. >>>> >>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97447#c7 >>>> Signed-off-by: Ilia Mirkin <[email protected]> >>>> Cc: [email protected] >>>> --- >>>> >>>> The change to exposing the gl_MaxCompute* constants is a little dodgy, >>>> arguably >>>> it should be exposed everywhere when compute functionality is available, >>>> but >>>> that is a little annoying to do, and is no worse than the previous state of >>>> things. >>>> >>>> src/compiler/glsl/builtin_variables.cpp | 2 +- >>>> src/compiler/glsl/glsl_lexer.ll | 4 ++-- >>>> src/compiler/glsl/glsl_parser.yy | 5 ++--- >>>> src/compiler/glsl/glsl_parser_extras.cpp | 1 - >>>> src/compiler/glsl/glsl_parser_extras.h | 7 ------- >>>> 5 files changed, 5 insertions(+), 14 deletions(-) >>>> >>>> diff --git a/src/compiler/glsl/builtin_variables.cpp >>>> b/src/compiler/glsl/builtin_variables.cpp >>>> index c9d8b1c..4f88170 100644 >>>> --- a/src/compiler/glsl/builtin_variables.cpp >>>> +++ b/src/compiler/glsl/builtin_variables.cpp >>>> @@ -789,7 +789,7 @@ builtin_variable_generator::generate_constants() >>>> } >>>> } >>>> >>>> - if (state->is_version(430, 310) || state->ARB_compute_shader_enable) { >>>> + if (state->is_version(430, 310) || state->stage == >>>> MESA_SHADER_COMPUTE) { >>>> add_const("gl_MaxComputeAtomicCounterBuffers", >>>> state->Const.MaxComputeAtomicCounterBuffers); >>>> add_const("gl_MaxComputeAtomicCounters", >>>> diff --git a/src/compiler/glsl/glsl_lexer.ll >>>> b/src/compiler/glsl/glsl_lexer.ll >>>> index 7be2b89..3324cde 100644 >>>> --- a/src/compiler/glsl/glsl_lexer.ll >>>> +++ b/src/compiler/glsl/glsl_lexer.ll >>>> @@ -410,7 +410,7 @@ writeonly KEYWORD_WITH_ALT(420, 300, 420, 310, >>>> yyextra->ARB_shader_image_lo >>>> >>>> atomic_uint KEYWORD_WITH_ALT(420, 300, 420, 310, >>>> yyextra->ARB_shader_atomic_counters_enable, ATOMIC_UINT); >>>> >>>> -shared KEYWORD_WITH_ALT(430, 310, 430, 310, >>>> yyextra->ARB_compute_shader_enable, SHARED); >>>> +shared KEYWORD_WITH_ALT(430, 310, 430, 310, yyextra->stage == >>>> MESA_SHADER_COMPUTE, SHARED); >>>> >>>> struct return STRUCT; >>>> void return VOID_TOK; >>>> @@ -425,7 +425,7 @@ layout { >>>> || yyextra->ARB_uniform_buffer_object_enable >>>> || yyextra->ARB_fragment_coord_conventions_enable >>>> || yyextra->ARB_shading_language_420pack_enable >>>> - || yyextra->ARB_compute_shader_enable >>>> + || yyextra->stage == MESA_SHADER_COMPUTE >>>> || yyextra->ARB_tessellation_shader_enable) { >>>> return LAYOUT_TOK; >>>> } else { >>>> diff --git a/src/compiler/glsl/glsl_parser.yy >>>> b/src/compiler/glsl/glsl_parser.yy >>>> index 4f4a83c..cf2fb60 100644 >>>> --- a/src/compiler/glsl/glsl_parser.yy >>>> +++ b/src/compiler/glsl/glsl_parser.yy >>>> @@ -1570,10 +1570,9 @@ layout_qualifier_id: >>>> for (int i = 0; i < 3; i++) { >>>> if (match_layout_qualifier(local_size_qualifiers[i], $1, >>>> state) == 0) { >>>> - if (!state->has_compute_shader()) { >>>> + if (state->stage != MESA_SHADER_COMPUTE) { >>>> _mesa_glsl_error(& @3, state, >>>> - "%s qualifier requires GLSL 4.30 or " >>>> - "GLSL ES 3.10 or ARB_compute_shader", >>>> + "%s qualifier only valid in a compute >>>> shader", >>>> local_size_qualifiers[i]); >>>> YYERROR; >>>> } else { >>>> diff --git a/src/compiler/glsl/glsl_parser_extras.cpp >>>> b/src/compiler/glsl/glsl_parser_extras.cpp >>>> index 14a5540..37aff38 100644 >>>> --- a/src/compiler/glsl/glsl_parser_extras.cpp >>>> +++ b/src/compiler/glsl/glsl_parser_extras.cpp >>>> @@ -578,7 +578,6 @@ static const _mesa_glsl_extension >>>> _mesa_glsl_supported_extensions[] = { >>>> EXT(ARB_ES3_1_compatibility), >>>> EXT(ARB_ES3_2_compatibility), >>>> EXT(ARB_arrays_of_arrays), >>>> - EXT(ARB_compute_shader), >>> >>> I think this will mean that having >>> >>> #extension GL_ARB_compute_shader: require >>> >>> will not compile. That seems bad. >> >> I see no indication that that should ever have worked in the first place. > > There are a good number of conformance tests that do this. That's > making me start to question the validity of this change at all.
OK. Cheerfully withdrawn. -ilia _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
