[Mesa-dev] [PATCH] RFC: gallium: increase PIPE_MAX_SHADER_SAMPLER_VIEWS to 128

[Jose Fonseca]

On 27/02/18 02:38, sroland at vmware.com wrote:
> From: Roland Scheidegger <sroland at vmware.com>
> 
> Some state trackers require 128.
> (There are no plans to increase PIPE_MAX_SAMPLERS too, since with gl
> state tracker it's unlikely more than 32 will be needed, if you need
> more use bindless.)
> ---
>   src/gallium/include/pipe/p_state.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/gallium/include/pipe/p_state.h b/src/gallium/include/pipe/p_state.h
> index 2b56d60..cddb3b4 100644
> --- a/src/gallium/include/pipe/p_state.h
> +++ b/src/gallium/include/pipe/p_state.h
> @@ -64,7 +64,7 @@ extern "C" {
>   #define PIPE_MAX_SAMPLERS         32
>   #define PIPE_MAX_SHADER_INPUTS    80 /* 32 GENERIC + 32 PATCH + 16 others */
>   #define PIPE_MAX_SHADER_OUTPUTS   80 /* 32 GENERIC + 32 PATCH + 16 others */
> -#define PIPE_MAX_SHADER_SAMPLER_VIEWS 32
> +#define PIPE_MAX_SHADER_SAMPLER_VIEWS 128
>   #define PIPE_MAX_SHADER_BUFFERS   32
>   #define PIPE_MAX_SHADER_IMAGES    32
>   #define PIPE_MAX_TEXTURE_LEVELS   16
> 

Bumping PIPE_MAX_SHADER_SAMPLER_VIEWS defintely sounds good to me.


But besides changing the #defnie, I suppose there are quite a few places 
where bitmasks will need to increase from 1 32bit words to something else.

Such as src/gallium/drivers/llvmpipe/lp_state_fs.c's make_variant_key()

Which also means that tgsi_shader_info::file_mask should be a 128 bit 
word.  But I don't think 128 bit words are portable (GCC/Clang support, 
but MSVC does not)

So perhaps we need re-spec file_mask to mean the _first_ 64bit, and have 
a second file_mask (eg file_mask_hi) with the second 64 bits.  Another 
alternative is to leave tgsi_shader_info::file_mask alone, and have a 
special uint64 srv_mask[2] field dedicated for sampler views.

I suspect there are more places that do 1 << which will need to be updated.

BTW, GCC/Clang have options to detect bad shifts: -fsanitize=undefined 
or -fsanitize=shift-exponent.

It's not the first time we get in troubles due to this, so we should 
consider just set it on all debug builds.  And we should also find a 
simple test that exercise all 128 SRVs to weed these places out.

Jose