[Mesa-dev] [PATCH] tgsi/scan: use wrap-around shift behavior explicitly for file_mask

[Brian Paul]

On 03/01/2018 07:01 PM, sroland at vmware.com wrote:
> From: Roland Scheidegger <sroland at vmware.com>
> 
> The comment said it will only represent the lowest 32 regs. This was
> not entirely true in practice, since at least on x86 you'll get
> masked shifts (unless the compiler could recognize it already and toss
> it out). It turns out this actually works out alright (presumably
> noone uses it for temp regs) when increasing max sampler views, so
> make that behavior explicit.
> Albeit it feels a bit hacky (but in any case, explicit behavior there
> is better than undefined behavior).
> ---
>   src/gallium/auxiliary/tgsi/tgsi_scan.c     | 7 +++++--
>   src/gallium/drivers/llvmpipe/lp_state_fs.c | 7 ++++++-
>   src/gallium/drivers/swr/swr_shader.cpp     | 2 +-
>   3 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/tgsi/tgsi_scan.c b/src/gallium/auxiliary/tgsi/tgsi_scan.c
> index c35eff2..0d229c9 100644
> --- a/src/gallium/auxiliary/tgsi/tgsi_scan.c
> +++ b/src/gallium/auxiliary/tgsi/tgsi_scan.c
> @@ -585,8 +585,11 @@ scan_declaration(struct tgsi_shader_info *info,
>         int buffer;
>         unsigned index, target, type;
>   
> -      /* only first 32 regs will appear in this bitfield */
> -      info->file_mask[file] |= (1 << reg);
> +      /*
> +       * only first 32 regs will appear in this bitfield, if larger
> +       * bits will wrap around.
> +       */
> +      info->file_mask[file] |= (1 << (reg & 31));

Or, reg % 32 and let the compiler optimize it.

Either way, Reviewed-by: Brian Paul <brianp at vmware.com>

>         info->file_count[file]++;
>         info->file_max[file] = MAX2(info->file_max[file], (int)reg);
>   
> diff --git a/src/gallium/drivers/llvmpipe/lp_state_fs.c b/src/gallium/drivers/llvmpipe/lp_state_fs.c
> index 603fd84..48c004c 100644
> --- a/src/gallium/drivers/llvmpipe/lp_state_fs.c
> +++ b/src/gallium/drivers/llvmpipe/lp_state_fs.c
> @@ -3323,7 +3323,12 @@ make_variant_key(struct llvmpipe_context *lp,
>      if (shader->info.base.file_max[TGSI_FILE_SAMPLER_VIEW] != -1) {
>         key->nr_sampler_views = shader->info.base.file_max[TGSI_FILE_SAMPLER_VIEW] + 1;
>         for(i = 0; i < key->nr_sampler_views; ++i) {
> -         if(shader->info.base.file_mask[TGSI_FILE_SAMPLER_VIEW] & (1 << i)) {
> +         /*
> +          * Note sview may exceed what's representable by file_mask.
> +          * This will still work, the only downside is that not actually
> +          * used views may be included in the shader key.
> +          */
> +         if(shader->info.base.file_mask[TGSI_FILE_SAMPLER_VIEW] & (1 << (i & 31))) {
>               lp_sampler_static_texture_state(&key->state[i].texture_state,
>                                               lp->sampler_views[PIPE_SHADER_FRAGMENT][i]);
>            }
> diff --git a/src/gallium/drivers/swr/swr_shader.cpp b/src/gallium/drivers/swr/swr_shader.cpp
> index e5fb679..fa1c0b8 100644
> --- a/src/gallium/drivers/swr/swr_shader.cpp
> +++ b/src/gallium/drivers/swr/swr_shader.cpp
> @@ -98,7 +98,7 @@ swr_generate_sampler_key(const struct lp_tgsi_info &info,
>         key.nr_sampler_views =
>            info.base.file_max[TGSI_FILE_SAMPLER_VIEW] + 1;
>         for (unsigned i = 0; i < key.nr_sampler_views; i++) {
> -         if (info.base.file_mask[TGSI_FILE_SAMPLER_VIEW] & (1 << i)) {
> +         if (info.base.file_mask[TGSI_FILE_SAMPLER_VIEW] & (1 << (i & 31))) {
>               const struct pipe_sampler_view *view =
>                  ctx->sampler_views[shader_type][i];
>               lp_sampler_static_texture_state(
>