Re: [Mesa-dev] [PATCH 4/9] i965/fs: Lower gl_VertexID and friends to inputs at the NIR level

2017-05-05 Thread Kenneth Graunke 
On Thursday, May 4, 2017 7:11:42 PM PDT Jason Ekstrand wrote:
> NIR calls these system values but they come in from the VF unit as
> vertex data.  It's terribly convenient to just be able to treat them as
> such in the back-end.
> ---
>  src/intel/compiler/brw_fs.h   |  1 -
>  src/intel/compiler/brw_fs_nir.cpp | 30 +--
>  src/intel/compiler/brw_fs_visitor.cpp | 38 --
>  src/intel/compiler/brw_nir.c  | 72 
> ++-
>  4 files changed, 71 insertions(+), 70 deletions(-)
> 
> diff --git a/src/intel/compiler/brw_fs.h b/src/intel/compiler/brw_fs.h
> index e230b5e..6c8c027 100644
> --- a/src/intel/compiler/brw_fs.h
> +++ b/src/intel/compiler/brw_fs.h
> @@ -175,7 +175,6 @@ public:
> fs_reg *emit_samplepos_setup();
> fs_reg *emit_sampleid_setup();
> fs_reg *emit_samplemaskin_setup();
> -   fs_reg *emit_vs_system_value(int location);
> void emit_interpolation_setup_gen4();
> void emit_interpolation_setup_gen6();
> void compute_sample_position(fs_reg dst, fs_reg int_sample_pos);
> diff --git a/src/intel/compiler/brw_fs_nir.cpp 
> b/src/intel/compiler/brw_fs_nir.cpp
> index 3ab41df..d4ce753 100644
> --- a/src/intel/compiler/brw_fs_nir.cpp
> +++ b/src/intel/compiler/brw_fs_nir.cpp
> @@ -89,39 +89,11 @@ emit_system_values_block(nir_block *block, fs_visitor *v)
>   unreachable("should be lowered by lower_vertex_id().");
>  
>case nir_intrinsic_load_vertex_id_zero_base:
> - assert(v->stage == MESA_SHADER_VERTEX);
> - reg = >nir_system_values[SYSTEM_VALUE_VERTEX_ID_ZERO_BASE];
> - if (reg->file == BAD_FILE)
> -*reg = 
> *v->emit_vs_system_value(SYSTEM_VALUE_VERTEX_ID_ZERO_BASE);
> - break;
> -
>case nir_intrinsic_load_base_vertex:
> - assert(v->stage == MESA_SHADER_VERTEX);
> - reg = >nir_system_values[SYSTEM_VALUE_BASE_VERTEX];
> - if (reg->file == BAD_FILE)
> -*reg = *v->emit_vs_system_value(SYSTEM_VALUE_BASE_VERTEX);
> - break;
> -
>case nir_intrinsic_load_instance_id:
> - assert(v->stage == MESA_SHADER_VERTEX);
> - reg = >nir_system_values[SYSTEM_VALUE_INSTANCE_ID];
> - if (reg->file == BAD_FILE)
> -*reg = *v->emit_vs_system_value(SYSTEM_VALUE_INSTANCE_ID);
> - break;
> -
>case nir_intrinsic_load_base_instance:
> - assert(v->stage == MESA_SHADER_VERTEX);
> - reg = >nir_system_values[SYSTEM_VALUE_BASE_INSTANCE];
> - if (reg->file == BAD_FILE)
> -*reg = *v->emit_vs_system_value(SYSTEM_VALUE_BASE_INSTANCE);
> - break;
> -
>case nir_intrinsic_load_draw_id:
> - assert(v->stage == MESA_SHADER_VERTEX);
> - reg = >nir_system_values[SYSTEM_VALUE_DRAW_ID];
> - if (reg->file == BAD_FILE)
> -*reg = *v->emit_vs_system_value(SYSTEM_VALUE_DRAW_ID);
> - break;
> + unreachable("should be lowered by brw_nir_lower_vs_inputs().");
>  
>case nir_intrinsic_load_invocation_id:
>   if (v->stage == MESA_SHADER_TESS_CTRL)
> diff --git a/src/intel/compiler/brw_fs_visitor.cpp 
> b/src/intel/compiler/brw_fs_visitor.cpp
> index 7904434..b6524d2 100644
> --- a/src/intel/compiler/brw_fs_visitor.cpp
> +++ b/src/intel/compiler/brw_fs_visitor.cpp
> @@ -32,44 +32,6 @@
>  
>  using namespace brw;
>  
> -fs_reg *
> -fs_visitor::emit_vs_system_value(int location)
> -{
> -   fs_reg *reg = new(this->mem_ctx)
> -  fs_reg(ATTR, 4 * _mesa_bitcount_64(nir->info->inputs_read),
> - BRW_REGISTER_TYPE_D);
> -
> -   switch (location) {
> -   case SYSTEM_VALUE_BASE_VERTEX:
> -  reg->offset = 0;
> -  break;
> -   case SYSTEM_VALUE_BASE_INSTANCE:
> -  reg->offset = REG_SIZE;
> -  break;
> -   case SYSTEM_VALUE_VERTEX_ID:
> -  unreachable("should have been lowered");
> -   case SYSTEM_VALUE_VERTEX_ID_ZERO_BASE:
> -  reg->offset = 2 * REG_SIZE;
> -  break;
> -   case SYSTEM_VALUE_INSTANCE_ID:
> -  reg->offset = 3 * REG_SIZE;
> -  break;
> -   case SYSTEM_VALUE_DRAW_ID:
> -  if (nir->info->system_values_read &
> -  (BITFIELD64_BIT(SYSTEM_VALUE_BASE_VERTEX) |
> -   BITFIELD64_BIT(SYSTEM_VALUE_BASE_INSTANCE) |
> -   BITFIELD64_BIT(SYSTEM_VALUE_VERTEX_ID_ZERO_BASE) |
> -   BITFIELD64_BIT(SYSTEM_VALUE_INSTANCE_ID)))
> - reg->nr += 4;
> -  reg->offset = 0;
> -  break;
> -   default:
> -  unreachable("not reached");
> -   }
> -
> -   return reg;
> -}
> -
>  /* Sample from the MCS surface attached to this multisample texture. */
>  fs_reg
>  fs_visitor::emit_mcs_fetch(const fs_reg , unsigned components,
> diff --git a/src/intel/compiler/brw_nir.c b/src/intel/compiler/brw_nir.c
> index 7248594..556782e 100644
> --- a/src/intel/compiler/brw_nir.c
> +++ b/src/intel/compiler/brw_nir.c
> @@ -259,18 +259,81 @@ brw_nir_lower_vs_inputs(nir_shader *nir,
> if (!is_scalar)
>return;
>  
>

Re: [Mesa-dev] [PATCH 4/9] i965/fs: Lower gl_VertexID and friends to inputs at the NIR level

2017-05-05 Thread Alejandro Piñeiro 
Reviewed-by: Alejandro Piñeiro 

On 05/05/17 04:11, Jason Ekstrand wrote:
> NIR calls these system values but they come in from the VF unit as
> vertex data.  It's terribly convenient to just be able to treat them as
> such in the back-end.
> ---
>  src/intel/compiler/brw_fs.h   |  1 -
>  src/intel/compiler/brw_fs_nir.cpp | 30 +--
>  src/intel/compiler/brw_fs_visitor.cpp | 38 --
>  src/intel/compiler/brw_nir.c  | 72 
> ++-
>  4 files changed, 71 insertions(+), 70 deletions(-)
>
> diff --git a/src/intel/compiler/brw_fs.h b/src/intel/compiler/brw_fs.h
> index e230b5e..6c8c027 100644
> --- a/src/intel/compiler/brw_fs.h
> +++ b/src/intel/compiler/brw_fs.h
> @@ -175,7 +175,6 @@ public:
> fs_reg *emit_samplepos_setup();
> fs_reg *emit_sampleid_setup();
> fs_reg *emit_samplemaskin_setup();
> -   fs_reg *emit_vs_system_value(int location);
> void emit_interpolation_setup_gen4();
> void emit_interpolation_setup_gen6();
> void compute_sample_position(fs_reg dst, fs_reg int_sample_pos);
> diff --git a/src/intel/compiler/brw_fs_nir.cpp 
> b/src/intel/compiler/brw_fs_nir.cpp
> index 3ab41df..d4ce753 100644
> --- a/src/intel/compiler/brw_fs_nir.cpp
> +++ b/src/intel/compiler/brw_fs_nir.cpp
> @@ -89,39 +89,11 @@ emit_system_values_block(nir_block *block, fs_visitor *v)
>   unreachable("should be lowered by lower_vertex_id().");
>  
>case nir_intrinsic_load_vertex_id_zero_base:
> - assert(v->stage == MESA_SHADER_VERTEX);
> - reg = >nir_system_values[SYSTEM_VALUE_VERTEX_ID_ZERO_BASE];
> - if (reg->file == BAD_FILE)
> -*reg = 
> *v->emit_vs_system_value(SYSTEM_VALUE_VERTEX_ID_ZERO_BASE);
> - break;
> -
>case nir_intrinsic_load_base_vertex:
> - assert(v->stage == MESA_SHADER_VERTEX);
> - reg = >nir_system_values[SYSTEM_VALUE_BASE_VERTEX];
> - if (reg->file == BAD_FILE)
> -*reg = *v->emit_vs_system_value(SYSTEM_VALUE_BASE_VERTEX);
> - break;
> -
>case nir_intrinsic_load_instance_id:
> - assert(v->stage == MESA_SHADER_VERTEX);
> - reg = >nir_system_values[SYSTEM_VALUE_INSTANCE_ID];
> - if (reg->file == BAD_FILE)
> -*reg = *v->emit_vs_system_value(SYSTEM_VALUE_INSTANCE_ID);
> - break;
> -
>case nir_intrinsic_load_base_instance:
> - assert(v->stage == MESA_SHADER_VERTEX);
> - reg = >nir_system_values[SYSTEM_VALUE_BASE_INSTANCE];
> - if (reg->file == BAD_FILE)
> -*reg = *v->emit_vs_system_value(SYSTEM_VALUE_BASE_INSTANCE);
> - break;
> -
>case nir_intrinsic_load_draw_id:
> - assert(v->stage == MESA_SHADER_VERTEX);
> - reg = >nir_system_values[SYSTEM_VALUE_DRAW_ID];
> - if (reg->file == BAD_FILE)
> -*reg = *v->emit_vs_system_value(SYSTEM_VALUE_DRAW_ID);
> - break;
> + unreachable("should be lowered by brw_nir_lower_vs_inputs().");
>  
>case nir_intrinsic_load_invocation_id:
>   if (v->stage == MESA_SHADER_TESS_CTRL)
> diff --git a/src/intel/compiler/brw_fs_visitor.cpp 
> b/src/intel/compiler/brw_fs_visitor.cpp
> index 7904434..b6524d2 100644
> --- a/src/intel/compiler/brw_fs_visitor.cpp
> +++ b/src/intel/compiler/brw_fs_visitor.cpp
> @@ -32,44 +32,6 @@
>  
>  using namespace brw;
>  
> -fs_reg *
> -fs_visitor::emit_vs_system_value(int location)
> -{
> -   fs_reg *reg = new(this->mem_ctx)
> -  fs_reg(ATTR, 4 * _mesa_bitcount_64(nir->info->inputs_read),
> - BRW_REGISTER_TYPE_D);
> -
> -   switch (location) {
> -   case SYSTEM_VALUE_BASE_VERTEX:
> -  reg->offset = 0;
> -  break;
> -   case SYSTEM_VALUE_BASE_INSTANCE:
> -  reg->offset = REG_SIZE;
> -  break;
> -   case SYSTEM_VALUE_VERTEX_ID:
> -  unreachable("should have been lowered");
> -   case SYSTEM_VALUE_VERTEX_ID_ZERO_BASE:
> -  reg->offset = 2 * REG_SIZE;
> -  break;
> -   case SYSTEM_VALUE_INSTANCE_ID:
> -  reg->offset = 3 * REG_SIZE;
> -  break;
> -   case SYSTEM_VALUE_DRAW_ID:
> -  if (nir->info->system_values_read &
> -  (BITFIELD64_BIT(SYSTEM_VALUE_BASE_VERTEX) |
> -   BITFIELD64_BIT(SYSTEM_VALUE_BASE_INSTANCE) |
> -   BITFIELD64_BIT(SYSTEM_VALUE_VERTEX_ID_ZERO_BASE) |
> -   BITFIELD64_BIT(SYSTEM_VALUE_INSTANCE_ID)))
> - reg->nr += 4;
> -  reg->offset = 0;
> -  break;
> -   default:
> -  unreachable("not reached");
> -   }
> -
> -   return reg;
> -}
> -
>  /* Sample from the MCS surface attached to this multisample texture. */
>  fs_reg
>  fs_visitor::emit_mcs_fetch(const fs_reg , unsigned components,
> diff --git a/src/intel/compiler/brw_nir.c b/src/intel/compiler/brw_nir.c
> index 7248594..556782e 100644
> --- a/src/intel/compiler/brw_nir.c
> +++ b/src/intel/compiler/brw_nir.c
> @@ -259,18 +259,81 @@ brw_nir_lower_vs_inputs(nir_shader *nir,
> if