Bug 80778

Summary:	[bisected regression] piglit spec/glsl-1.50/compiler/incorrect-in-layout-qualifier-repeated-prim.geom
Product:	Mesa	Reporter:	Jordan Justen <jljusten>
Component:	glsl-compiler	Assignee:	Samuel Iglesias Gonsálvez <siglesias>
Status:	RESOLVED FIXED	QA Contact:	Intel 3D Bugs Mailing List <intel-3d-bugs>
Severity:	normal
Priority:	medium	CC:	huax.lu, idr, jljusten, vlee
Version:	git
Hardware:	Other
OS:	All
Whiteboard:
i915 platform:		i915 features:
Attachments:	Patch to fix the issue

Description Jordan Justen 2014-07-02 00:47:01 UTC

Reproduce with:
glslparsertest tests/spec/glsl-1.50/compiler/incorrect-in-layout-qualifier-repeated-prim.geom fail 1.50

a7e6ec68985dda9ca70c3eeb4fa9d807b67f7c99 is the first bad commit
commit a7e6ec68985dda9ca70c3eeb4fa9d807b67f7c99
Author: Samuel Iglesias Gonsalvez <siglesias@igalia.com>
Date:   Tue Jun 10 08:45:43 2014 +0200

    glsl: Add parsing support for multi-stream output in geometry shaders.
    
    This implements parsing requirements for multi-stream support in
    geometry shaders as defined in ARB_gpu_shader5.
    
    Signed-off-by: Samuel Iglesias Gonsalvez <siglesias@igalia.com>
    Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>

Comment 1 Ian Romanick 2014-07-02 06:39:09 UTC

*** Bug 80781 has been marked as a duplicate of this bug. ***

Comment 2 Samuel Iglesias Gonsálvez 2014-07-02 06:47:21 UTC

Created attachment 102112 [details] [review]
Patch to fix the issue

I have reproduced the error. Attached is the patch to fix it.

Please test it. If it is OK I will send it to the mailing list.

Comment 3 Jordan Justen 2014-07-02 07:14:18 UTC

(In reply to comment #2)
> Created attachment 102112 [details] [review] [review]
> Patch to fix the issue
> 
> I have reproduced the error. Attached is the patch to fix it.
> 
> Please test it. If it is OK I will send it to the mailing list.

Were you able to verify the patch?

It is usually easier to reply to email with comments.
So, I would suggest just sending it to the list after
you have tested it and feel fairly confident in the
patch.

Nevertheless, my comments on the patch are,
how about creating a new local variable?

ast_type_qualifier allowed_duplicates_mask;
allowed_duplicates_mask.flags.i =
   ubo_mat_mask.flags.i |
   ubo_layout_mask.flags.i |
   ubo_binding_mask.flags.i;
if (state->stage == MESA_SHADER_GEOMETRY)
   allowed_duplicates_mask.flags.i |=
      stream_layout_mask.flags.i;

Comment 4 Samuel Iglesias Gonsálvez 2014-07-02 07:20:19 UTC

(In reply to comment #3)
> (In reply to comment #2)
> > Created attachment 102112 [details] [review] [review] [review]
> > Patch to fix the issue
> > 
> > I have reproduced the error. Attached is the patch to fix it.
> > 
> > Please test it. If it is OK I will send it to the mailing list.
> 
> Were you able to verify the patch?
> 

Yes, I verified it.

> It is usually easier to reply to email with comments.
> So, I would suggest just sending it to the list after
> you have tested it and feel fairly confident in the
> patch.
> 

OK, I will do that as of now. Thanks for the tip!

> Nevertheless, my comments on the patch are,
> how about creating a new local variable?
> 
> ast_type_qualifier allowed_duplicates_mask;
> allowed_duplicates_mask.flags.i =
>    ubo_mat_mask.flags.i |
>    ubo_layout_mask.flags.i |
>    ubo_binding_mask.flags.i;
> if (state->stage == MESA_SHADER_GEOMETRY)
>    allowed_duplicates_mask.flags.i |=
>       stream_layout_mask.flags.i;

Good idea. I'm going to create that variable to simplify the code and submit it to the mailing list.

Thanks,

Sam

Comment 5 Samuel Iglesias Gonsálvez 2014-07-03 06:01:51 UTC

*** Bug 80836 has been marked as a duplicate of this bug. ***

Comment 6 Jordan Justen 2014-07-03 20:47:07 UTC

Fixed in 7f0420700c473caee00a84596b22a600a7517b4d

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.