Re: [Mesa-dev] [PATCH v3 3/3] i965/fs: fix indirect load DF uniforms on BSW/BXT

[Francisco Jerez]

Samuel Iglesias Gonsálvez  writes:

> The lowered BSW/BXT indirect move instructions had incorrect
> source types, which luckily wasn't causing incorrect assembly to be
> generated due to the bug fixed in the next patch, but would have
> confused the remaining back-end IR infrastructure due to the mismatch
> between the IR source types and the emitted machine code.
>
> v2:
> - Improve commit log (Curro)
> - Fix read_size (Curro)
> - Fix DF uniform array detection in assign_constant_locations() when
>   it is acceded with 32-bit MOV_INDIRECTs in BSW/BXT.
>
> v3:
> - Move changes in assign_constant_locations() to other patch.
>
> Signed-off-by: Samuel Iglesias Gonsálvez 
> Cc: "17.0" 

Reviewed-by: Francisco Jerez 

> ---
>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 41 
> 
>  1 file changed, 20 insertions(+), 21 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp 
> b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> index a977ee4273..10aa5fde32 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> @@ -3815,31 +3815,30 @@ fs_visitor::nir_emit_intrinsic(const fs_builder , 
> nir_intrinsic_instr *instr
>   unsigned read_size = instr->const_index[1] -
>  (instr->num_components - 1) * type_sz(dest.type);
>  
> - fs_reg indirect_chv_high_32bit;
> - bool is_chv_bxt_64bit =
> -(devinfo->is_cherryview || devinfo->is_broxton) &&
> -type_sz(dest.type) == 8;
> - if (is_chv_bxt_64bit) {
> -indirect_chv_high_32bit = vgrf(glsl_type::uint_type);
> -/* Calculate indirect address to read high 32 bits */
> -bld.ADD(indirect_chv_high_32bit, indirect, brw_imm_ud(4));
> - }
> + bool supports_64bit_indirects =
> +!devinfo->is_cherryview && !devinfo->is_broxton;
>  
> - for (unsigned j = 0; j < instr->num_components; j++) {
> -if (!is_chv_bxt_64bit) {
> + if (type_sz(dest.type) != 8 || supports_64bit_indirects) {
> +for (unsigned j = 0; j < instr->num_components; j++) {
> bld.emit(SHADER_OPCODE_MOV_INDIRECT,
>  offset(dest, bld, j), offset(src, bld, j),
>  indirect, brw_imm_ud(read_size));
> -} else {
> -   bld.emit(SHADER_OPCODE_MOV_INDIRECT,
> -subscript(offset(dest, bld, j), 
> BRW_REGISTER_TYPE_UD, 0),
> -offset(src, bld, j),
> -indirect, brw_imm_ud(read_size));
> -
> -   bld.emit(SHADER_OPCODE_MOV_INDIRECT,
> -subscript(offset(dest, bld, j), 
> BRW_REGISTER_TYPE_UD, 1),
> -offset(src, bld, j),
> -indirect_chv_high_32bit, brw_imm_ud(read_size));
> +}
> + } else {
> +const unsigned num_mov_indirects =
> +   type_sz(dest.type) / type_sz(BRW_REGISTER_TYPE_UD);
> +/* We read a little bit less per MOV INDIRECT, as they are now
> + * 32-bits ones instead of 64-bit. Fix read_size then.
> + */
> +const unsigned read_size_32bit = read_size -
> +(num_mov_indirects - 1) * type_sz(BRW_REGISTER_TYPE_UD);
> +for (unsigned j = 0; j < instr->num_components; j++) {
> +   for (unsigned i = 0; i < num_mov_indirects; i++) {
> +  bld.emit(SHADER_OPCODE_MOV_INDIRECT,
> +   subscript(offset(dest, bld, j), 
> BRW_REGISTER_TYPE_UD, i),
> +   subscript(offset(src, bld, j), 
> BRW_REGISTER_TYPE_UD, i),
> +   indirect, brw_imm_ud(read_size_32bit));
> +   }
>  }
>   }
>}
> -- 
> 2.11.0


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

[Mesa-dev] [PATCH v3 3/3] i965/fs: fix indirect load DF uniforms on BSW/BXT

[Samuel Iglesias Gonsálvez]

The lowered BSW/BXT indirect move instructions had incorrect
source types, which luckily wasn't causing incorrect assembly to be
generated due to the bug fixed in the next patch, but would have
confused the remaining back-end IR infrastructure due to the mismatch
between the IR source types and the emitted machine code.

v2:
- Improve commit log (Curro)
- Fix read_size (Curro)
- Fix DF uniform array detection in assign_constant_locations() when
  it is acceded with 32-bit MOV_INDIRECTs in BSW/BXT.

v3:
- Move changes in assign_constant_locations() to other patch.

Signed-off-by: Samuel Iglesias Gonsálvez 
Cc: "17.0" 
---
 src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 41 
 1 file changed, 20 insertions(+), 21 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
index a977ee4273..10aa5fde32 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
@@ -3815,31 +3815,30 @@ fs_visitor::nir_emit_intrinsic(const fs_builder , 
nir_intrinsic_instr *instr
  unsigned read_size = instr->const_index[1] -
 (instr->num_components - 1) * type_sz(dest.type);
 
- fs_reg indirect_chv_high_32bit;
- bool is_chv_bxt_64bit =
-(devinfo->is_cherryview || devinfo->is_broxton) &&
-type_sz(dest.type) == 8;
- if (is_chv_bxt_64bit) {
-indirect_chv_high_32bit = vgrf(glsl_type::uint_type);
-/* Calculate indirect address to read high 32 bits */
-bld.ADD(indirect_chv_high_32bit, indirect, brw_imm_ud(4));
- }
+ bool supports_64bit_indirects =
+!devinfo->is_cherryview && !devinfo->is_broxton;
 
- for (unsigned j = 0; j < instr->num_components; j++) {
-if (!is_chv_bxt_64bit) {
+ if (type_sz(dest.type) != 8 || supports_64bit_indirects) {
+for (unsigned j = 0; j < instr->num_components; j++) {
bld.emit(SHADER_OPCODE_MOV_INDIRECT,
 offset(dest, bld, j), offset(src, bld, j),
 indirect, brw_imm_ud(read_size));
-} else {
-   bld.emit(SHADER_OPCODE_MOV_INDIRECT,
-subscript(offset(dest, bld, j), BRW_REGISTER_TYPE_UD, 
0),
-offset(src, bld, j),
-indirect, brw_imm_ud(read_size));
-
-   bld.emit(SHADER_OPCODE_MOV_INDIRECT,
-subscript(offset(dest, bld, j), BRW_REGISTER_TYPE_UD, 
1),
-offset(src, bld, j),
-indirect_chv_high_32bit, brw_imm_ud(read_size));
+}
+ } else {
+const unsigned num_mov_indirects =
+   type_sz(dest.type) / type_sz(BRW_REGISTER_TYPE_UD);
+/* We read a little bit less per MOV INDIRECT, as they are now
+ * 32-bits ones instead of 64-bit. Fix read_size then.
+ */
+const unsigned read_size_32bit = read_size -
+(num_mov_indirects - 1) * type_sz(BRW_REGISTER_TYPE_UD);
+for (unsigned j = 0; j < instr->num_components; j++) {
+   for (unsigned i = 0; i < num_mov_indirects; i++) {
+  bld.emit(SHADER_OPCODE_MOV_INDIRECT,
+   subscript(offset(dest, bld, j), 
BRW_REGISTER_TYPE_UD, i),
+   subscript(offset(src, bld, j), 
BRW_REGISTER_TYPE_UD, i),
+   indirect, brw_imm_ud(read_size_32bit));
+   }
 }
  }
   }
-- 
2.11.0

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