[Mesa-dev] [PATCH 4/4] radeonsi: add instance divisor support

[Michel Dänzer]

On Die, 2013-03-26 at 17:37 +0100, Christian König wrote: 
> Am 26.03.2013 15:56, schrieb Michel Dänzer:
> > On Die, 2013-03-26 at 14:51 +0100, Christian König wrote:
> >> From: Christian König <christian.koenig at amd.com>
> >>
> >> Signed-off-by: Christian König <christian.koenig at amd.com>
> >> [...]
> >> diff --git a/src/gallium/drivers/radeonsi/radeonsi_shader.h b/src/gallium/drivers/radeonsi/radeonsi_shader.h
> >> index 9dae742..e09f297 100644
> >> --- a/src/gallium/drivers/radeonsi/radeonsi_shader.h
> >> +++ b/src/gallium/drivers/radeonsi/radeonsi_shader.h
> >> @@ -111,13 +111,18 @@ struct si_shader {
> >>   	unsigned		nr_cbufs;
> >>   };
> >>   
> >> -struct si_shader_key {
> >> -	unsigned		export_16bpc:8;
> >> -	unsigned		nr_cbufs:4;
> >> -	unsigned		color_two_side:1;
> >> -	unsigned		alpha_func:3;
> >> -	unsigned		flatshade:1;
> >> -	float			alpha_ref;
> >> +union si_shader_key {
> >> +	struct {
> >> +		unsigned	export_16bpc:8;
> >> +		unsigned	nr_cbufs:4;
> >> +		unsigned	color_two_side:1;
> >> +		unsigned	alpha_func:3;
> >> +		unsigned	flatshade:1;
> >> +		float		alpha_ref;
> >> +	} ps;
> >> +	struct {
> >> +		unsigned	instance_divisors[PIPE_MAX_ATTRIBS];
> >> +	} vs;
> >>   };
> > This grows the shader key from 8 to 128 bytes. I don't suppose the
> > instance divisors could be encoded in a more compact way? E.g. loading
> > the divisor values from constants and only tracking which elements use a
> > divisor in a bitmask in the key.
> 
> Considered that also, and I have two problems with that approach:
> 1. While immediates are converted to shifts & muls, dividing even by a 
> constant in the shader isn't cheap.

Is that really significant? How much work would it be to come up with a
worst case test and measure the difference?


> How about storing only a byte for the instance_divisor? That limit's the 
> divisor to a modulo of 256, but I don't think that would be so extremly bad.

I have no idea what the impact of that would be. What happens if an app
tries to use a divisor >= 256?

> That would reduce the key to 32 bytes instead.

Still seems kind of big.


-- 
Earthling Michel Dänzer           |                   http://www.amd.com
Libre software enthusiast         |          Debian, X and DRI developer