Re: [Mesa-dev] [PATCH] i965/vec4: Fix mapping attributes

[Kenneth Graunke]

On Friday, January 13, 2017 1:41:34 PM PST Jordan Justen wrote:
> From: "Juan A. Suarez Romero" 
> 
> This patch reverts 57bab6708f2bbc1ab8a3d202e9a467963596d462, which was
> causing issues with ILK and earlier VS programs.
> 
> 1. Revert "i965/vec4/nir: vec4 also needs to remap vs attributes"
> 
>Do not perform a remap in vec4 backend. Rather, do it later when
>setup attributes
> 
> 2. This fixes mapping ATTRx to proper GRFn.
> 
> Suggested-by: Kenneth Graunke 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99391
> [jordan.l.jus...@intel.com: merge Juan's two patches from bugzilla]
> Signed-off-by: Jordan Justen 
> Cc: Kenneth Graunke 
> ---
>  I merged Juan's revert + fix patches, as suggested by Ken.
> 
>  I put this patch through jenkins, and they appeared to fix the
>  ilk/g45/g965 regressions.
> 
>  The revert is in brw_nir.c, and Juan's new change is in brw_vec4.cpp.
> 
>  src/mesa/drivers/dri/i965/brw_nir.c| 32 ++--
>  src/mesa/drivers/dri/i965/brw_vec4.cpp |  2 +-
>  2 files changed, 11 insertions(+), 23 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_nir.c 
> b/src/mesa/drivers/dri/i965/brw_nir.c
> index b39e2b1f523..3c1bc5162fc 100644
> --- a/src/mesa/drivers/dri/i965/brw_nir.c
> +++ b/src/mesa/drivers/dri/i965/brw_nir.c
> @@ -95,19 +95,9 @@ add_const_offset_to_base(nir_shader *nir, 
> nir_variable_mode mode)
> }
>  }
>  
> -struct remap_vs_attrs_params {
> -   shader_info *nir_info;
> -   bool is_scalar;
> -};
> -
>  static bool
> -remap_vs_attrs(nir_block *block, void *closure)
> +remap_vs_attrs(nir_block *block, shader_info *nir_info)
>  {
> -   struct remap_vs_attrs_params *params =
> -  (struct remap_vs_attrs_params *) closure;
> -   shader_info *nir_info = params->nir_info;
> -   bool is_scalar = params->is_scalar;
> -
> nir_foreach_instr(instr, block) {
>if (instr->type != nir_instr_type_intrinsic)
>   continue;
> @@ -123,7 +113,7 @@ remap_vs_attrs(nir_block *block, void *closure)
>   int attr = intrin->const_index[0];
>   int slot = _mesa_bitcount_64(nir_info->inputs_read &
>BITFIELD64_MASK(attr));
> - intrin->const_index[0] = is_scalar ? 4 * slot : slot;
> + intrin->const_index[0] = 4 * slot;
>}
> }
> return true;
> @@ -267,11 +257,6 @@ brw_nir_lower_vs_inputs(nir_shader *nir,
>  bool use_legacy_snorm_formula,
>  const uint8_t *vs_attrib_wa_flags)
>  {
> -   struct remap_vs_attrs_params params = {
> -  .nir_info = nir->info,
> -  .is_scalar = is_scalar
> -   };
> -
> /* Start with the location of the variable's base. */
> foreach_list_typed(nir_variable, var, node, >inputs) {
>var->data.driver_location = var->data.location;
> @@ -291,11 +276,14 @@ brw_nir_lower_vs_inputs(nir_shader *nir,
> brw_nir_apply_attribute_workarounds(nir, use_legacy_snorm_formula,
> vs_attrib_wa_flags);
>  
> -   /* Finally, translate VERT_ATTRIB_* values into the actual registers. */
> -   nir_foreach_function(function, nir) {
> -  if (function->impl) {
> - nir_foreach_block(block, function->impl) {
> -remap_vs_attrs(block, );
> +   if (is_scalar) {
> +  /* Finally, translate VERT_ATTRIB_* values into the actual registers. 
> */
> +
> +  nir_foreach_function(function, nir) {
> + if (function->impl) {
> +nir_foreach_block(block, function->impl) {
> +   remap_vs_attrs(block, nir->info);
> +}
>   }
>}
> }
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp 
> b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> index 748a068b142..5e60eb657a7 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -1739,7 +1739,7 @@ vec4_vs_visitor::setup_attributes(int payload_reg)
>int needed_slots =
>   (vs_prog_data->double_inputs_read & BITFIELD64_BIT(first)) ? 2 : 1;
>for (int c = 0; c < needed_slots; c++) {
> - attribute_map[nr_attributes] = payload_reg + nr_attributes;
> + attribute_map[first + c] = payload_reg + nr_attributes;
>   nr_attributes++;
>   vs_inputs &= ~BITFIELD64_BIT(first + c);
>}
> 

Thanks Juan and Jordan!

Reviewed-by: Kenneth Graunke 


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] i965/vec4: Fix mapping attributes

[Jordan Justen]

From: "Juan A. Suarez Romero" 

This patch reverts 57bab6708f2bbc1ab8a3d202e9a467963596d462, which was
causing issues with ILK and earlier VS programs.

1. Revert "i965/vec4/nir: vec4 also needs to remap vs attributes"

   Do not perform a remap in vec4 backend. Rather, do it later when
   setup attributes

2. This fixes mapping ATTRx to proper GRFn.

Suggested-by: Kenneth Graunke 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99391
[jordan.l.jus...@intel.com: merge Juan's two patches from bugzilla]
Signed-off-by: Jordan Justen 
Cc: Kenneth Graunke 
---
 I merged Juan's revert + fix patches, as suggested by Ken.

 I put this patch through jenkins, and they appeared to fix the
 ilk/g45/g965 regressions.

 The revert is in brw_nir.c, and Juan's new change is in brw_vec4.cpp.

 src/mesa/drivers/dri/i965/brw_nir.c| 32 ++--
 src/mesa/drivers/dri/i965/brw_vec4.cpp |  2 +-
 2 files changed, 11 insertions(+), 23 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_nir.c 
b/src/mesa/drivers/dri/i965/brw_nir.c
index b39e2b1f523..3c1bc5162fc 100644
--- a/src/mesa/drivers/dri/i965/brw_nir.c
+++ b/src/mesa/drivers/dri/i965/brw_nir.c
@@ -95,19 +95,9 @@ add_const_offset_to_base(nir_shader *nir, nir_variable_mode 
mode)
}
 }
 
-struct remap_vs_attrs_params {
-   shader_info *nir_info;
-   bool is_scalar;
-};
-
 static bool
-remap_vs_attrs(nir_block *block, void *closure)
+remap_vs_attrs(nir_block *block, shader_info *nir_info)
 {
-   struct remap_vs_attrs_params *params =
-  (struct remap_vs_attrs_params *) closure;
-   shader_info *nir_info = params->nir_info;
-   bool is_scalar = params->is_scalar;
-
nir_foreach_instr(instr, block) {
   if (instr->type != nir_instr_type_intrinsic)
  continue;
@@ -123,7 +113,7 @@ remap_vs_attrs(nir_block *block, void *closure)
  int attr = intrin->const_index[0];
  int slot = _mesa_bitcount_64(nir_info->inputs_read &
   BITFIELD64_MASK(attr));
- intrin->const_index[0] = is_scalar ? 4 * slot : slot;
+ intrin->const_index[0] = 4 * slot;
   }
}
return true;
@@ -267,11 +257,6 @@ brw_nir_lower_vs_inputs(nir_shader *nir,
 bool use_legacy_snorm_formula,
 const uint8_t *vs_attrib_wa_flags)
 {
-   struct remap_vs_attrs_params params = {
-  .nir_info = nir->info,
-  .is_scalar = is_scalar
-   };
-
/* Start with the location of the variable's base. */
foreach_list_typed(nir_variable, var, node, >inputs) {
   var->data.driver_location = var->data.location;
@@ -291,11 +276,14 @@ brw_nir_lower_vs_inputs(nir_shader *nir,
brw_nir_apply_attribute_workarounds(nir, use_legacy_snorm_formula,
vs_attrib_wa_flags);
 
-   /* Finally, translate VERT_ATTRIB_* values into the actual registers. */
-   nir_foreach_function(function, nir) {
-  if (function->impl) {
- nir_foreach_block(block, function->impl) {
-remap_vs_attrs(block, );
+   if (is_scalar) {
+  /* Finally, translate VERT_ATTRIB_* values into the actual registers. */
+
+  nir_foreach_function(function, nir) {
+ if (function->impl) {
+nir_foreach_block(block, function->impl) {
+   remap_vs_attrs(block, nir->info);
+}
  }
   }
}
diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp 
b/src/mesa/drivers/dri/i965/brw_vec4.cpp
index 748a068b142..5e60eb657a7 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
@@ -1739,7 +1739,7 @@ vec4_vs_visitor::setup_attributes(int payload_reg)
   int needed_slots =
  (vs_prog_data->double_inputs_read & BITFIELD64_BIT(first)) ? 2 : 1;
   for (int c = 0; c < needed_slots; c++) {
- attribute_map[nr_attributes] = payload_reg + nr_attributes;
+ attribute_map[first + c] = payload_reg + nr_attributes;
  nr_attributes++;
  vs_inputs &= ~BITFIELD64_BIT(first + c);
   }
-- 
2.11.0

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev