Re: [Mesa-dev] [PATCH] glsl: Allow ES2 function parameters to be hidden by variable declarations.
Ian Romanick writes: > [ Unknown signature status ] > On 07/18/2018 03:03 PM, Eric Anholt wrote: >> Ian Romanick writes: >> >>> On 07/16/2018 02:46 PM, Eric Anholt wrote: This fixes dEQP case: dEQP-GLES2.functional.shaders.scoping.valid.local_variable_hides_function_parameter_fragment >>> >>> Are we sure that test is correct? I'm sure I already know the answer, >>> but does the test contain any justification or spec references? I just >>> re-read section 4.2 (Scoping) of the ESSL 1.00 spec, and I don't see >>> anything to support this. Did I miss something? >>> >>> In fact, the grammar says: >>> >>> function_definition: >>> function_prototype compound_statement_no_new_scope >>> >>> So... I think this test is just wrong. >> >> OK, so I'm confused why this test still exists, if people have managed >> to get conformance on Mesa. I'm on master of VK-GL-CTS, and it's still >> in the mustpass file: >> >> external/openglcts/data/mustpass/gles/aosp_mustpass/master/gles2-master.txt:dEQP-GLES2.functional.shaders.scoping.valid.local_variable_hides_function_parameter_fragment > > There are a huge pile of test lists, and I have never really understood > the whole mess. There are some that only matter for some kind of > Android conformance runs. There are some that only matter for Khronos > conformance runs. And there are some that don't seem to matter for > anything at all. > >> I don't see anything that would exclude the test -- there's >> gles2-driver-issues.txt, but that appears to only be used to exclude >> tests from AOSP DEQP usage. >> >> Could whoever on the Intel side submitted a conformance package for Mesa >> send me a copy? I haven't been able to find it on the Khronos site, and >> I suspect it would help me understand how to achieve conformance with >> Mesa. >> dEQP-GLES3.functional.shaders.preprocessor.predefined_macros.line_2_vertex >> is another one that fails on Mesa with i965, and seems to have been in >> the testsuite forever. > > When we do conformance submissions, we don't run off master. We use > whatever is tip of the per-API release branch. We then do the > "official" run using 'cd external/openglcts/modules; ./cts-runner > --type='. I haven't pulled any of the repos since last > year, so this information may be out of date. I'm still doing my current run (3 days in) off of master. I'll swap to the release branch next. Still, since I can't find any delta in the release branch that would exclude these tests, I would really, really like to see someone's conformance package for GLES on Mesa. signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: Allow ES2 function parameters to be hidden by variable declarations.
On 07/18/2018 03:03 PM, Eric Anholt wrote: > Ian Romanick writes: > >> On 07/16/2018 02:46 PM, Eric Anholt wrote: >>> This fixes dEQP case: >>> >>> dEQP-GLES2.functional.shaders.scoping.valid.local_variable_hides_function_parameter_fragment >> >> Are we sure that test is correct? I'm sure I already know the answer, >> but does the test contain any justification or spec references? I just >> re-read section 4.2 (Scoping) of the ESSL 1.00 spec, and I don't see >> anything to support this. Did I miss something? >> >> In fact, the grammar says: >> >> function_definition: >> function_prototype compound_statement_no_new_scope >> >> So... I think this test is just wrong. > > OK, so I'm confused why this test still exists, if people have managed > to get conformance on Mesa. I'm on master of VK-GL-CTS, and it's still > in the mustpass file: > > external/openglcts/data/mustpass/gles/aosp_mustpass/master/gles2-master.txt:dEQP-GLES2.functional.shaders.scoping.valid.local_variable_hides_function_parameter_fragment There are a huge pile of test lists, and I have never really understood the whole mess. There are some that only matter for some kind of Android conformance runs. There are some that only matter for Khronos conformance runs. And there are some that don't seem to matter for anything at all. > I don't see anything that would exclude the test -- there's > gles2-driver-issues.txt, but that appears to only be used to exclude > tests from AOSP DEQP usage. > > Could whoever on the Intel side submitted a conformance package for Mesa > send me a copy? I haven't been able to find it on the Khronos site, and > I suspect it would help me understand how to achieve conformance with > Mesa. > dEQP-GLES3.functional.shaders.preprocessor.predefined_macros.line_2_vertex > is another one that fails on Mesa with i965, and seems to have been in > the testsuite forever. When we do conformance submissions, we don't run off master. We use whatever is tip of the per-API release branch. We then do the "official" run using 'cd external/openglcts/modules; ./cts-runner --type='. I haven't pulled any of the repos since last year, so this information may be out of date. signature.asc Description: OpenPGP digital signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: Allow ES2 function parameters to be hidden by variable declarations.
Ian Romanick writes: > On 07/16/2018 02:46 PM, Eric Anholt wrote: >> This fixes dEQP case: >> >> dEQP-GLES2.functional.shaders.scoping.valid.local_variable_hides_function_parameter_fragment > > Are we sure that test is correct? I'm sure I already know the answer, > but does the test contain any justification or spec references? I just > re-read section 4.2 (Scoping) of the ESSL 1.00 spec, and I don't see > anything to support this. Did I miss something? > > In fact, the grammar says: > > function_definition: > function_prototype compound_statement_no_new_scope > > So... I think this test is just wrong. OK, so I'm confused why this test still exists, if people have managed to get conformance on Mesa. I'm on master of VK-GL-CTS, and it's still in the mustpass file: external/openglcts/data/mustpass/gles/aosp_mustpass/master/gles2-master.txt:dEQP-GLES2.functional.shaders.scoping.valid.local_variable_hides_function_parameter_fragment I don't see anything that would exclude the test -- there's gles2-driver-issues.txt, but that appears to only be used to exclude tests from AOSP DEQP usage. Could whoever on the Intel side submitted a conformance package for Mesa send me a copy? I haven't been able to find it on the Khronos site, and I suspect it would help me understand how to achieve conformance with Mesa. dEQP-GLES3.functional.shaders.preprocessor.predefined_macros.line_2_vertex is another one that fails on Mesa with i965, and seems to have been in the testsuite forever. signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: Allow ES2 function parameters to be hidden by variable declarations.
On 07/16/2018 02:46 PM, Eric Anholt wrote: > This fixes dEQP case: > > dEQP-GLES2.functional.shaders.scoping.valid.local_variable_hides_function_parameter_fragment Are we sure that test is correct? I'm sure I already know the answer, but does the test contain any justification or spec references? I just re-read section 4.2 (Scoping) of the ESSL 1.00 spec, and I don't see anything to support this. Did I miss something? In fact, the grammar says: function_definition: function_prototype compound_statement_no_new_scope So... I think this test is just wrong. > without breaking > > dEQP-GLES3.functional.shaders.scoping.invalid.local_variable_hides_function_parameter_fragment > --- > src/compiler/glsl/ast_to_hir.cpp | 9 + > 1 file changed, 9 insertions(+) > > diff --git a/src/compiler/glsl/ast_to_hir.cpp > b/src/compiler/glsl/ast_to_hir.cpp > index dd60a2a87fd5..28f074ca8a39 100644 > --- a/src/compiler/glsl/ast_to_hir.cpp > +++ b/src/compiler/glsl/ast_to_hir.cpp > @@ -6180,10 +6180,19 @@ ast_function_definition::hir(exec_list *instructions, >} > } > > + /* On ES2, function parameters may be redeclared to be hidden within the > +* function. Do this by creating a new scope inside the function. > +*/ > + if (state->is_version(0, 100) && !state->is_version(0, 300)) > + state->symbols->push_scope(); > + > /* Convert the body of the function to HIR. */ > this->body->hir(>body, state); > signature->is_defined = true; > > + if (state->is_version(0, 100) && !state->is_version(0, 300)) > + state->symbols->pop_scope(); > + > state->symbols->pop_scope(); > > assert(state->current_function == signature); > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: Allow ES2 function parameters to be hidden by variable declarations.
On Tue, Jul 17, 2018 at 1:33 PM, Eric Anholt wrote: > Ilia Mirkin writes: > >> On Mon, Jul 16, 2018 at 7:51 PM, Eric Anholt wrote: >>> Ilia Mirkin writes: >>> Perhaps use state->es_shader instead of is_version(0, 100)? [Just a drive-by comment, not a real review, sorry] >>> >>> That doesn't respect the version overrides and such that is_version >>> handles. >> >> Under what circumstance would state->es_shader != state->is_version(0, >> 100)? I can't think of one, but perhaps I'm just insufficiently >> creative. > > So you want to replace just the 100 call and leave the !300? I guess > that would be logically equivalent, but I'm not interested in painting > the bikeshed that color. Yes, that was my suggestion. Your code, your bikeshed color. Cheers, -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: Allow ES2 function parameters to be hidden by variable declarations.
Ilia Mirkin writes: > On Mon, Jul 16, 2018 at 7:51 PM, Eric Anholt wrote: >> Ilia Mirkin writes: >> >>> Perhaps use state->es_shader instead of is_version(0, 100)? [Just a >>> drive-by comment, not a real review, sorry] >> >> That doesn't respect the version overrides and such that is_version >> handles. > > Under what circumstance would state->es_shader != state->is_version(0, > 100)? I can't think of one, but perhaps I'm just insufficiently > creative. So you want to replace just the 100 call and leave the !300? I guess that would be logically equivalent, but I'm not interested in painting the bikeshed that color. signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: Allow ES2 function parameters to be hidden by variable declarations.
On Mon, Jul 16, 2018 at 7:51 PM, Eric Anholt wrote: > Ilia Mirkin writes: > >> Perhaps use state->es_shader instead of is_version(0, 100)? [Just a >> drive-by comment, not a real review, sorry] > > That doesn't respect the version overrides and such that is_version > handles. Under what circumstance would state->es_shader != state->is_version(0, 100)? I can't think of one, but perhaps I'm just insufficiently creative. -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: Allow ES2 function parameters to be hidden by variable declarations.
Ilia Mirkin writes: > Perhaps use state->es_shader instead of is_version(0, 100)? [Just a > drive-by comment, not a real review, sorry] That doesn't respect the version overrides and such that is_version handles. signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: Allow ES2 function parameters to be hidden by variable declarations.
Perhaps use state->es_shader instead of is_version(0, 100)? [Just a drive-by comment, not a real review, sorry] On Mon, Jul 16, 2018 at 5:46 PM, Eric Anholt wrote: > This fixes dEQP case: > > dEQP-GLES2.functional.shaders.scoping.valid.local_variable_hides_function_parameter_fragment > > without breaking > > dEQP-GLES3.functional.shaders.scoping.invalid.local_variable_hides_function_parameter_fragment > --- > src/compiler/glsl/ast_to_hir.cpp | 9 + > 1 file changed, 9 insertions(+) > > diff --git a/src/compiler/glsl/ast_to_hir.cpp > b/src/compiler/glsl/ast_to_hir.cpp > index dd60a2a87fd5..28f074ca8a39 100644 > --- a/src/compiler/glsl/ast_to_hir.cpp > +++ b/src/compiler/glsl/ast_to_hir.cpp > @@ -6180,10 +6180,19 @@ ast_function_definition::hir(exec_list *instructions, >} > } > > + /* On ES2, function parameters may be redeclared to be hidden within the > +* function. Do this by creating a new scope inside the function. > +*/ > + if (state->is_version(0, 100) && !state->is_version(0, 300)) > + state->symbols->push_scope(); > + > /* Convert the body of the function to HIR. */ > this->body->hir(>body, state); > signature->is_defined = true; > > + if (state->is_version(0, 100) && !state->is_version(0, 300)) > + state->symbols->pop_scope(); > + > state->symbols->pop_scope(); > > assert(state->current_function == signature); > -- > 2.18.0 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
