[Mesa-dev] [PATCH 05/17] mesa: fix 64-bit issues with system_values_read

[Nicolai Hähnle]

On 10.06.2017 01:47, Connor Abbott wrote:
> From: Connor Abbott <cwabbott0 at gmail.com>
> 
> We're about to bump the number of system values above 32. The
> system_values_read bitfield itself is 64 bits, but some users weren't
> taking that into account. Fix the ones I could find by grepping for
> "system_values_read". This prevents regressions at least with radeonsi
> and other Gallium drivers, and probably i965 too.
> 
> Signed-off-by: Connor Abbott <cwabbott0 at gmail.com>
> ---
>   src/gallium/auxiliary/nir/tgsi_to_nir.c    | 2 +-
>   src/intel/compiler/brw_vec4_gs_visitor.cpp | 3 ++-
>   src/mesa/program/programopt.c              | 2 +-
>   src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 9 +++++----
>   src/mesa/state_tracker/st_mesa_to_tgsi.c   | 6 +++---
>   5 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/nir/tgsi_to_nir.c b/src/gallium/auxiliary/nir/tgsi_to_nir.c
> index d4914ac..e5daef4 100644
> --- a/src/gallium/auxiliary/nir/tgsi_to_nir.c
> +++ b/src/gallium/auxiliary/nir/tgsi_to_nir.c
> @@ -588,7 +588,7 @@ ttn_src_for_file_and_index(struct ttn_compile *c, unsigned file, unsigned index,
>         src = nir_src_for_ssa(&load->dest.ssa);
>   
>         b->shader->info.system_values_read |=
> -         (1 << nir_system_value_from_intrinsic(op));
> +         (1ull << nir_system_value_from_intrinsic(op));
>   
>         break;
>      }
> diff --git a/src/intel/compiler/brw_vec4_gs_visitor.cpp b/src/intel/compiler/brw_vec4_gs_visitor.cpp
> index d0236df..bdae3d4 100644
> --- a/src/intel/compiler/brw_vec4_gs_visitor.cpp
> +++ b/src/intel/compiler/brw_vec4_gs_visitor.cpp
> @@ -653,7 +653,8 @@ brw_compile_gs(const struct brw_compiler *compiler, void *log_data,
>         shader->info.clip_distance_array_size;
>   
>      prog_data->include_primitive_id =
> -      (shader->info.system_values_read & (1 << SYSTEM_VALUE_PRIMITIVE_ID)) != 0;
> +      (shader->info.system_values_read &
> +       BITFIELD64_BIT(SYSTEM_VALUE_PRIMITIVE_ID)) != 0;
>   
>      prog_data->invocations = shader->info.gs.invocations;
>   
> diff --git a/src/mesa/program/programopt.c b/src/mesa/program/programopt.c
> index f560bce..f389d2b 100644
> --- a/src/mesa/program/programopt.c
> +++ b/src/mesa/program/programopt.c
> @@ -597,7 +597,7 @@ _mesa_program_fragment_position_to_sysval(struct gl_program *prog)
>         return;
>   
>      prog->info.inputs_read &= ~BITFIELD64_BIT(VARYING_SLOT_POS);
> -   prog->info.system_values_read |= 1 << SYSTEM_VALUE_FRAG_COORD;
> +   prog->info.system_values_read |= BITFIELD64_BIT(SYSTEM_VALUE_FRAG_COORD);
>   
>      for (i = 0; i < prog->arb.NumInstructions; i++) {
>         struct prog_instruction *inst = prog->arb.Instructions + i;
> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> index 76cd4dc..bdabfa1 100644
> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> @@ -6450,10 +6450,10 @@ st_translate_program(
>      /* Declare misc input registers
>       */
>      {
> -      GLbitfield sysInputs = proginfo->info.system_values_read;
> +      GLbitfield64 sysInputs = proginfo->info.system_values_read;
>   
>         for (i = 0; sysInputs; i++) {
> -         if (sysInputs & (1 << i)) {
> +         if (sysInputs & BITFIELD64_BIT(i)) {

This is a nitpick, but why not just (1llu << i)?

Anyway, this patch, as well as 10-13 and merged 9+14:

Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>



>               unsigned semName = _mesa_sysval_to_semantic(i);
>   
>               t->systemValues[i] = ureg_DECL_system_value(ureg, semName, 0);
> @@ -6484,7 +6484,7 @@ st_translate_program(
>                  emit_wpos(st_context(ctx), t, proginfo, ureg,
>                            program->wpos_transform_const);
>   
> -            sysInputs &= ~(1 << i);
> +            sysInputs &= ~BITFIELD64_BIT(i);
>            }
>         }
>      }
> @@ -6786,7 +6786,8 @@ get_mesa_program_tgsi(struct gl_context *ctx,
>      /* This must be done before the uniform storage is associated. */
>      if (shader->Stage == MESA_SHADER_FRAGMENT &&
>          (prog->info.inputs_read & VARYING_BIT_POS ||
> -        prog->info.system_values_read & (1 << SYSTEM_VALUE_FRAG_COORD))) {
> +        prog->info.system_values_read &
> +        BITFIELD64_BIT(SYSTEM_VALUE_FRAG_COORD))) {
>         static const gl_state_index wposTransformState[STATE_LENGTH] = {
>            STATE_INTERNAL, STATE_FB_WPOS_Y_TRANSFORM
>         };
> diff --git a/src/mesa/state_tracker/st_mesa_to_tgsi.c b/src/mesa/state_tracker/st_mesa_to_tgsi.c
> index 984ff92..5556525 100644
> --- a/src/mesa/state_tracker/st_mesa_to_tgsi.c
> +++ b/src/mesa/state_tracker/st_mesa_to_tgsi.c
> @@ -938,9 +938,9 @@ st_translate_mesa_program(
>   
>      /* Declare misc input registers
>       */
> -   GLbitfield sysInputs = program->info.system_values_read;
> +   GLbitfield64 sysInputs = program->info.system_values_read;
>      for (i = 0; sysInputs; i++) {
> -      if (sysInputs & (1 << i)) {
> +      if (sysInputs & BITFIELD64_BIT(i)) {
>            unsigned semName = _mesa_sysval_to_semantic(i);
>   
>            t->systemValues[i] = ureg_DECL_system_value(ureg, semName, 0);
> @@ -972,7 +972,7 @@ st_translate_mesa_program(
>                semName == TGSI_SEMANTIC_POSITION)
>               emit_wpos(st_context(ctx), t, program, ureg);
>   
> -          sysInputs &= ~(1 << i);
> +          sysInputs &= ~BITFIELD64_BIT(i);
>         }
>      }
>   
> 


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.