[Mesa-dev] [PATCH 05/17] mesa: fix 64-bit issues with system_values_read
Connor Abbott
cwabbott0 at gmail.com
Mon Jun 12 19:59:06 UTC 2017
On Mon, Jun 12, 2017 at 2:25 AM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
> 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)?
I was already using BITFIELD64_BIT in other places to be consistent
with the surrounding code, and I wanted to make what's going on here
more obvious, so I figured I might as well use the macro. I didn't
want to bother adding another header to tgsi_to_nir.c so I didn't use
it there though. Not terribly important though... if anyone feels
strongly I can change it.
>
> 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.
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list