[Mesa-dev] [PATCH 14/20] radeonsi: reserve a parameter slot for FMASK resources in shaders

[Tom Stellard]

On Thu, Aug 08, 2013 at 05:36:09PM +0200, Michel Dänzer wrote:
> On Don, 2013-08-08 at 08:00 -0700, Tom Stellard wrote:
> > On Thu, Aug 08, 2013 at 02:20:54AM +0200, Marek Olšák wrote:
> > > 
> > > diff --git a/src/gallium/drivers/radeonsi/si_state_draw.c b/src/gallium/drivers/radeonsi/si_state_draw.c
> > > index 746ace6..4208fa7 100644
> > > --- a/src/gallium/drivers/radeonsi/si_state_draw.c
> > > +++ b/src/gallium/drivers/radeonsi/si_state_draw.c
> > > @@ -241,6 +241,7 @@ static void si_pipe_shader_ps(struct pipe_context *ctx, struct si_pipe_shader *s
> > >  		/* Last 2 reserved SGPRs are used for VCC */
> > >  		num_sgprs = num_user_sgprs + 2;
> > >  	}
> > > +	num_sgprs += 1; /* XXX this fixes VM faults */
> > 
> > One problem is that the compiler is under reporting the number of SGPRs,
> > when there are unused USER_SGPRs in the shader.  It should always be
> > reporting a number greater than or equal to the number of USER_SGPRs.
> 
> That itself shouldn't be a problem, as the radeonsi code ensures this.
>

The way radeonsi checks for this on pixel shaders won't work in all
cases.  For example, if there are 8 USER_SGPR and the shader uses VCC,
then the backend should report 10 SGPRs used.  This value is eventually
rounded up to the next multiple of 8, so what is actually stored in
shader->num_sgprs is 16.

Now lets say that the shader doesn't see 2 of the USER_SGPRs, it will
report 8 SGPRs (6 USER_SGPRs + 2 for VCC).  This value is rounded up to
the nearest multiple of 8, which is still 8, and stored in shader->num_sgprs.

Since the number of SGPRs reported matches the number of USER_SGPRs,
the code in si_pipe_shader_ps() will accepted the reported number of
SGPRs as valid, even though it isn't.

-Tom
> 
> > I think Michel mentioned this earlier, but there may also be a problem
> > with the way we determine usage of the VCC register in the shader, maybe
> > it is being used for more instructions than we realize.
> 
> I was just worrying that AMDGPUAsmPrinter::EmitProgramInfoSI() might not
> take implicit access to VCC into account, but Christian confirmed it
> should. AFAICT we should be encoding any such implicit access correctly
> (except for V_DIV_SCALE_F* and V_DIV_FMAS_F*, which aren't used yet).
> 
> 
> -- 
> Earthling Michel Dänzer           |                   http://www.amd.com
> Libre software enthusiast         |          Debian, X and DRI developer
>