[Mesa-dev] [PATCH 5/5] radeonsi: Enable VGPR spilling for all shader types v2

[Tom Stellard]

On Fri, Jan 16, 2015 at 11:44:36AM +0900, Michel Dänzer wrote:
> On 16.01.2015 09:48, Tom Stellard wrote:
> >   - Use context global scratch buffers, one for each shader type.
> 
> AFAICT the code actually uses a single buffer for all shader types. As
> we discussed before, that needs to be fixed.
> 

I forgot to update this comment, but after further investigation it
turns out it is OK to use a single scratch buffer for all shader types.

> 
> > @@ -208,9 +208,15 @@ void radeon_shader_binary_free_relocs(struct radeon_shader_reloc *relocs,
> >  	FREE(relocs);
> >  }
> >  
> > -void radeon_shader_binary_free_members(struct radeon_shader_binary *binary) {
> > +void radeon_shader_binary_free_members(struct radeon_shader_binary *binary,
> > +					unsigned free_relocs)
> > +{
> >  	FREE(binary->code);
> >  	FREE(binary->config);
> >  	FREE(binary->rodata);
> > -	radeon_shader_binary_free_relocs(binary->relocs, binary->reloc_count);
> > +
> > +	if (free_relocs) {
> > +		radeon_shader_binary_free_relocs(binary->relocs,
> > +						binary->reloc_count);
> > +	}
> >  }
> 
> Looks like this and related changes should be in patch 2.
> 
> 
> > @@ -55,6 +56,7 @@ static void si_destroy_context(struct pipe_context *context)
> >  	if (sctx->dummy_pixel_shader) {
> >  		sctx->b.b.delete_fs_state(&sctx->b.b, sctx->dummy_pixel_shader);
> >  	}
> > +
> >  	sctx->b.b.delete_depth_stencil_alpha_state(&sctx->b.b, sctx->custom_dsa_flush);
> >  	sctx->b.b.delete_blend_state(&sctx->b.b, sctx->custom_blend_resolve);
> >  	sctx->b.b.delete_blend_state(&sctx->b.b, sctx->custom_blend_decompress);
> 
> Please eliminate whitespace-only hunks like this one.
> 
> 
> Apart from the parts which should be moved to patch 2, the other patches
> of the series look good to me.
> 
> 
> -- 
> Earthling Michel Dänzer               |               http://www.amd.com
> Libre software enthusiast             |             Mesa and X developer
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev