[Mesa-dev] [PATCH (resend)] r600/sb: Don't require array declarations for TGSI_FILE_SYSTEM_VALUE

[Gert Wollny]

Am Freitag, den 02.02.2018, 02:13 +0100 schrieb Roland Scheidegger:
> Am 30.01.2018 um 09:21 schrieb Gert Wollny:
> > Although gl_SampleMaskIn is declared as an array in GLSL, it is
> > effectively a 32 bit mask on all hardware supported by mesa, so the
> > array indexing is ignored (Thanks Glenn Kennard for the
> > explanation).
> > 
> > Add a comment that the assert is not made superfluos by the else
> > branch.
> > 
> > Corrects: piglit spec at arb_gpu_shader5@execution at samplemaskin-
> > indirect for
> > debug builds (it already passed in release).
> > 
> > Signed-off-by: Gert Wollny <gw.fossdev at gmail.com>
> > ---
> > 
> >  src/gallium/drivers/r600/sb/sb_bc_parser.cpp | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/gallium/drivers/r600/sb/sb_bc_parser.cpp
> > b/src/gallium/drivers/r600/sb/sb_bc_parser.cpp
> > index 970e4141d5..b92e058daf 100644
> > --- a/src/gallium/drivers/r600/sb/sb_bc_parser.cpp
> > +++ b/src/gallium/drivers/r600/sb/sb_bc_parser.cpp
> > @@ -125,7 +125,9 @@ int bc_parser::parse_decls() {
> >  		return 0;
> >  	}
> >  
> > -	if (pshader->indirect_files & ~((1 << TGSI_FILE_CONSTANT)
> > | (1 << TGSI_FILE_SAMPLER))) {
> > +	if (pshader->indirect_files &
> > +	    ~((1 << TGSI_FILE_CONSTANT) | (1 << TGSI_FILE_SAMPLER)
> > |
> > +          (1 << TGSI_FILE_SYSTEM_VALUE))) {
> 
> So I suppose this is ok because none of the other system values
> could be arrays.
> That said, isn't that actually a bug in glsl to tgsi translation? the
> sample mask is never an array in gallium/tgsi (and not declared as
> such), so I think it should not turn up as indirect when properly
> translated, reading the value should not end up as indexed.

Well, the spec says the sample mask is an array [1], but Glenn told me
that in the end on all supported hardware it ends up as a bit mask [2].

Best, 
Gert 

[1] 
https://www.khronos.org/registry/OpenGL-Refpages/gl4/html/gl_SampleMask
.xhtml

[2]https://lists.freedesktop.org/archives/mesa-dev/2017-September/16937
4.html
> 
> Roland
> 
> 
> >  		assert(pshader->num_arrays);
> >  
> > @@ -135,6 +137,10 @@ int bc_parser::parse_decls() {
> >  				sh->add_gpr_array(a.gpr_start,
> > a.gpr_count, a.comp_mask);
> >  			}
> >  		} else {
> > +			/* When the above assert is disabled and
> > proper array info
> > +			 * is missing for some reason then, as a
> > fallback, make sure
> > +			 * that all GPRs can be accessed
> > indirectly.
> > +			 */
> >  			sh->add_gpr_array(0, pshader->bc.ngpr,
> > 0x0F);
> >  		}
> >  	}
> > 
> 
>