Re: [Mesa-dev] [PATCH 3/5] i965/fs: Emit shader w/a for Gen6 gather

2014-02-04 Thread Kenneth Graunke
On 02/03/2014 01:29 AM, Chris Forbes wrote:
> Signed-off-by: Chris Forbes 
> ---
>  src/mesa/drivers/dri/i965/brw_fs.h   |  1 +
>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 26 ++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h 
> b/src/mesa/drivers/dri/i965/brw_fs.h
> index 9c5c13a..3d668b9 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -360,6 +360,7 @@ public:
>fs_reg shadow_comp, fs_reg lod, fs_reg lod2,
>fs_reg sample_index, fs_reg mcs, int sampler);
> fs_reg emit_mcs_fetch(ir_texture *ir, fs_reg coordinate, int sampler);
> +   void emit_gen6_gather_wa(uint8_t wa, fs_reg dst);
> fs_reg fix_math_operand(fs_reg src);
> fs_inst *emit_math(enum opcode op, fs_reg dst, fs_reg src0);
> fs_inst *emit_math(enum opcode op, fs_reg dst, fs_reg src0, fs_reg src1);
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp 
> b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> index d88d24c..109f2e8 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> @@ -1699,9 +1699,35 @@ fs_visitor::visit(ir_texture *ir)
>}
> }
>  
> +   if (brw->gen == 6 && ir->op == ir_tg4 && 
> c->key.tex.gen6_gather_wa[sampler]) {

I think it might be easier to read if you did:

   if (brw->gen == 6 && ir->op == ir_tg4) {
  emit_gen6_gather_wa(c->key.tex.gen6_gather_wa[sampler], dst);
   }

and then in the body of the function did:

   if (!wa)
  return;

> +  emit_gen6_gather_wa(c->key.tex.gen6_gather_wa[sampler], dst);
> +   }
> +
> swizzle_result(ir, dst, sampler);
>  }
>  
> +/*

Comments above functions should start with /**, for Doxygen.

> + * Apply workarounds for Gen6 gather with UINT/SINT
> + */
> +void
> +fs_visitor::emit_gen6_gather_wa(uint8_t wa, fs_reg dst)
> +{
> +   int width = (wa & WA_8BIT) ? 8 : 16;
> +
> +   for (int i = 0; i < 4; i++) {
> +  fs_reg dst_f = dst.retype(BRW_REGISTER_TYPE_F);

Adding a comment would help:

  /* Convert from UNORM to UINT. */

> +  emit(MUL(dst_f, dst_f, fs_reg((float)((1 << width) - 1;

If you like, you could write this using C++ constructor casts
emit(MUL(dst_f, dst_f, fs_reg(float((1 << width) - 1;
which reduces the parenthesis jumble a bit.  Either way is fine.

> +  emit(MOV(dst, dst_f));
> +
> +  if (wa & WA_SIGN) {

 /* Reinterpret the UINT value as a signed INT value by shifting
  * the sign bit into place, then shifting back preserving sign.
  */

> + emit(SHL(dst, dst, fs_reg(32 - width)));
> + emit(ASR(dst, dst, fs_reg(32 - width)));
> +  }
> +
> +  dst.reg_offset++;
> +   }
> +}
> +
>  /**
>   * Set up the gather channel based on the swizzle, for gather4.
>   */
> 

These suggestions apply to patch 4 as well.



signature.asc
Description: OpenPGP digital signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 3/5] i965/fs: Emit shader w/a for Gen6 gather

2014-02-03 Thread Chris Forbes
Signed-off-by: Chris Forbes 
---
 src/mesa/drivers/dri/i965/brw_fs.h   |  1 +
 src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 26 ++
 2 files changed, 27 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/brw_fs.h 
b/src/mesa/drivers/dri/i965/brw_fs.h
index 9c5c13a..3d668b9 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.h
+++ b/src/mesa/drivers/dri/i965/brw_fs.h
@@ -360,6 +360,7 @@ public:
   fs_reg shadow_comp, fs_reg lod, fs_reg lod2,
   fs_reg sample_index, fs_reg mcs, int sampler);
fs_reg emit_mcs_fetch(ir_texture *ir, fs_reg coordinate, int sampler);
+   void emit_gen6_gather_wa(uint8_t wa, fs_reg dst);
fs_reg fix_math_operand(fs_reg src);
fs_inst *emit_math(enum opcode op, fs_reg dst, fs_reg src0);
fs_inst *emit_math(enum opcode op, fs_reg dst, fs_reg src0, fs_reg src1);
diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp 
b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
index d88d24c..109f2e8 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
@@ -1699,9 +1699,35 @@ fs_visitor::visit(ir_texture *ir)
   }
}
 
+   if (brw->gen == 6 && ir->op == ir_tg4 && 
c->key.tex.gen6_gather_wa[sampler]) {
+  emit_gen6_gather_wa(c->key.tex.gen6_gather_wa[sampler], dst);
+   }
+
swizzle_result(ir, dst, sampler);
 }
 
+/*
+ * Apply workarounds for Gen6 gather with UINT/SINT
+ */
+void
+fs_visitor::emit_gen6_gather_wa(uint8_t wa, fs_reg dst)
+{
+   int width = (wa & WA_8BIT) ? 8 : 16;
+
+   for (int i = 0; i < 4; i++) {
+  fs_reg dst_f = dst.retype(BRW_REGISTER_TYPE_F);
+  emit(MUL(dst_f, dst_f, fs_reg((float)((1 << width) - 1;
+  emit(MOV(dst, dst_f));
+
+  if (wa & WA_SIGN) {
+ emit(SHL(dst, dst, fs_reg(32 - width)));
+ emit(ASR(dst, dst, fs_reg(32 - width)));
+  }
+
+  dst.reg_offset++;
+   }
+}
+
 /**
  * Set up the gather channel based on the swizzle, for gather4.
  */
-- 
1.8.5.3

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