El 30/04/18 a las 23:14, Jason Ekstrand escribió:
> 
> 
> On Mon, Apr 30, 2018 at 7:18 AM, Iago Toral Quiroga <ito...@igalia.com
> <mailto:ito...@igalia.com>> wrote:
> 
>     From: Jose Maria Casanova Crespo <jmcasan...@igalia.com
>     <mailto:jmcasan...@igalia.com>>
> 
>     From Intel Skylake PRM, vol 07, "Immediate" section (page 768):
> 
>     "For a word, unsigned word, or half-float immediate data,
>     software must replicate the same 16-bit immediate value to both
>     the lower word and the high word of the 32-bit immediate field
>     in a GEN instruction."
> 
>     This patch implements float16 negate and fix the int16/uint16
>     negate that wasn't taking into account the replication in lower
>     and higher words.
> 
> 
> Since this fixes a bug, do we want to split it in two and send the
> bug-fix to stable?

Makes sense to split. I'm going to send for stable also the brw_imm_w patch.

I detected the same issue with brw_abs_immediate. So I'm including it in
the v3 of this patch.


> 
>     v2: Integer cases are different to Float cases. (Jason Ekstrand)
>         Included reference to PRM (Jose Maria Casanova)
>     ---
>      src/intel/compiler/brw_shader.cpp | 11 +++++++----
>      1 file changed, 7 insertions(+), 4 deletions(-)
> 
>     diff --git a/src/intel/compiler/brw_shader.cpp
>     b/src/intel/compiler/brw_shader.cpp
>     index 9cdf9fcb23..76dd1173fa 100644
>     --- a/src/intel/compiler/brw_shader.cpp
>     +++ b/src/intel/compiler/brw_shader.cpp
>     @@ -580,8 +580,13 @@ brw_negate_immediate(enum brw_reg_type type,
>     struct brw_reg *reg)
>            reg->d = -reg->d;
>            return true;
>         case BRW_REGISTER_TYPE_W:
>     -   case BRW_REGISTER_TYPE_UW:
>     -      reg->d = -(int16_t)reg->ud;
>     +   case BRW_REGISTER_TYPE_UW: {
>     +      uint16_t value = -(int16_t)reg->ud;
>     +      reg->ud = value | value << 16;
> 
> 
> You're shifting an explicitly 16-bit value by 16.  I think you want to
> cast to uint32_t.

As agreed I'll change this for:

reg->ud = value | (uint32_t) value << 16;


>     +      return true;
>     +   }
>     +   case BRW_REGISTER_TYPE_HF:
>     +      reg->ud ^= 0x80008000;
>            return true;
>         case BRW_REGISTER_TYPE_F:
>            reg->f = -reg->f;
>     @@ -602,8 +607,6 @@ brw_negate_immediate(enum brw_reg_type type,
>     struct brw_reg *reg)
>         case BRW_REGISTER_TYPE_UV:
>         case BRW_REGISTER_TYPE_V:
>            assert(!"unimplemented: negate UV/V immediate");
>     -   case BRW_REGISTER_TYPE_HF:
>     -      assert(!"unimplemented: negate HF immediate");
>         case BRW_REGISTER_TYPE_NF:
>            unreachable("no NF immediates");
>         }
>     -- 
>     2.14.1
> 
> 
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to