OK, sounds good to me.
On Fri, May 2, 2014 at 5:55 AM, Anuj Phogat <anuj.pho...@gmail.com> wrote:
> On Thu, Apr 17, 2014 at 1:05 AM, Chris Forbes <chr...@ijw.co.nz> wrote:
>> Correct, this doesn't try to tackle the link-time cases at all.
>>
>> After sending these out, Anuj told me that he had similar patches
>> pending review. Unfortunate to duplicate effort, but it's what I get
>> for doing a bunch of random bug-stomping, I guess :)
>>
>> Hopefully we can pick the best ideas from both series.
>>
> Chris, my series got r-b Ian. It'll be extra work to decouple compiler and
> linker patches in the series and then testing. So, I'm inclined towards
> landing my series as it is. Hope that is fine with you. We can always
> improve the compiler code later if required.
>
>> On Thu, Apr 17, 2014 at 6:29 PM, Kenneth Graunke <kenn...@whitecape.org>
>> wrote:
>>> On 04/12/2014 10:30 PM, Chris Forbes wrote:
>>>> The ARB_fragment_coord_conventions spec, section 4.3.x (Input Layout
>>>> Qualifiers) says:
>>>>
>>>> "All redeclarations of gl_FragCoord in all fragment shaders in a
>>>> single program must have the same set of qualifiers. Within any
>>>> shader, the first redeclarations of gl_FragCoord must appear before
>>>> any use of gl_FragCoord."
>>>>
>>>> Fixes piglit's tests:
>>>> arb_fragment_coord_conventions/compiler/redeclaration-after-use
>>>> arb_fragment_coord_conventions/compiler/redeclaration-around-use
>>>>
>>>> glsl-1.50/compiler/fragment_coord_conventions/layout-qualifiers-conflicting-case-1
>>>>
>>>> glsl-1.50/compiler/fragment_coord_conventions/layout-qualifiers-conflicting-case-2
>>>>
>>>> Signed-off-by: Chris Forbes <chr...@ijw.co.nz>
>>>> ---
>>>> src/glsl/ast_to_hir.cpp | 20 ++++++++++++++++++++
>>>> 1 file changed, 20 insertions(+)
>>>>
>>>> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
>>>> index 9b9d511..d1eb3e2 100644
>>>> --- a/src/glsl/ast_to_hir.cpp
>>>> +++ b/src/glsl/ast_to_hir.cpp
>>>> @@ -2715,9 +2715,29 @@ get_variable_being_redeclared(ir_variable *var,
>>>> YYLTYPE loc,
>>>> /* Allow redeclaration of gl_FragCoord for ARB_fcc layout
>>>> * qualifiers.
>>>> */
>>>> +
>>>> + if (earlier->data.how_declared != ir_var_declared_implicitly
>>>> + && (earlier->data.origin_upper_left !=
>>>> var->data.origin_upper_left
>>>> + || earlier->data.pixel_center_integer !=
>>>> var->data.pixel_center_integer)) {
>>>> + _mesa_glsl_error(&loc, state,
>>>> + "Inconsistent redeclarations of gl_FragCoord");
>>>> + }
>>>> +
>>>> earlier->data.origin_upper_left = var->data.origin_upper_left;
>>>> earlier->data.pixel_center_integer = var->data.pixel_center_integer;
>>>>
>>>> + if (earlier->data.used &&
>>>> + earlier->data.how_declared == ir_var_declared_implicitly) {
>>>> + /* ARB_fragment_coord_conventions spec Section 4.3.x.1
>>>> + * (Input Layout Qualifier) says:
>>>> + *
>>>> + * "Within any shader, the first redeclarations of gl_FragCoord
>>>> + * must appear before any use of gl_FragCoord."
>>>> + */
>>>> + _mesa_glsl_error(&loc, state,
>>>> + "First redeclaration of gl_FragCoord must
>>>> appear before any use");
>>>> + }
>>>> +
>>>> /* According to section 4.3.7 of the GLSL 1.30 spec,
>>>> * the following built-in varaibles can be redeclared with an
>>>> * interpolation qualifier:
>>>>
>>>
>>> It seems like this will catch multiple, inconsistent redeclarations
>>> within a single fragment shader...but the spec text quoted indicates
>>> that redeclarations in all fragment shaders being linked together into a
>>> single program need to be consistent.
>>>
>>> I don't see any code to handle that in linker*.cpp, so presumably we
>>> ought to add that as well. Regardless, this is a good improvement!
>>>
>>> Both patches are:
>>> Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
>>>
>>> Thanks, Chris.
>>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
