[Mesa-dev] [PATCH 11/19] i965: Convert brw_eu_compact.c to the new brw_inst API.

[Kenneth Graunke]

On Friday, June 13, 2014 11:14:12 PM Matt Turner wrote:
> ---
>  src/mesa/drivers/dri/i965/brw_eu_compact.c | 269 
+++++++++++++++++------------
>  1 file changed, 161 insertions(+), 108 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_eu_compact.c 
b/src/mesa/drivers/dri/i965/brw_eu_compact.c
> index 0ae3f2d..7e08d1d 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu_compact.c
> +++ b/src/mesa/drivers/dri/i965/brw_eu_compact.c
> @@ -331,16 +331,25 @@ set_control_index(struct brw_context *brw,
>                    struct brw_compact_instruction *dst,
>                    struct brw_instruction *src)
>  {
> -   uint32_t *src_u32 = (uint32_t *)src;
> -   uint32_t uncompacted = 0;
> -
> -   uncompacted |= ((src_u32[0] >> 8) & 0xffff) << 0;
> -   uncompacted |= ((src_u32[0] >> 31) & 0x1) << 16;
> -   /* On gen7, the flag register number gets integrated into the control
> -    * index.
> +   uint32_t uncompacted =                         /* 17b/SNB; 19b/IVB+ */
> +      (brw_inst_saturate(brw, src)       << 16) | /* 1b */
> +      (brw_inst_exec_size(brw, src)      << 13) | /* 3b */
> +      (brw_inst_pred_inv(brw, src)       << 12) | /* 1b */
> +      (brw_inst_pred_control(brw, src)   <<  8) | /* 4b */
> +      (brw_inst_thread_control(brw, src) <<  6) | /* 2b */
> +      (brw_inst_qtr_control(brw, src)    <<  4) | /* 2b */
> +      (brw_inst_no_dd_check(brw, src)    <<  3) | /* 1b */
> +      (brw_inst_no_dd_clear(brw, src)    <<  2) | /* 1b */
> +      (brw_inst_mask_control(brw, src)   <<  1) | /* 1b */
> +      (brw_inst_access_mode(brw, src)    <<  0);  /* 1b */
> +
> +   /* On gen7, the flag register and subregister numbers are integrated 
into
> +    * the control index.
>      */
>     if (brw->gen >= 7)
> -      uncompacted |= ((src_u32[2] >> 25) & 0x3) << 17;
> +      uncompacted |=
> +         (brw_inst_flag_reg_nr(brw, src)    << 18) | /* 1b */
> +         (brw_inst_flag_subreg_nr(brw, src) << 17);  /* 1b */
>  
>     for (int i = 0; i < 32; i++) {
>        if (control_index_table[i] == uncompacted) {
> @@ -353,13 +362,19 @@ set_control_index(struct brw_context *brw,
>  }
>  
>  static bool
> -set_datatype_index(struct brw_compact_instruction *dst,
> +set_datatype_index(struct brw_context *brw,
> +                   struct brw_compact_instruction *dst,
>                     struct brw_instruction *src)
>  {
> -   uint32_t uncompacted = 0;
> -
> -   uncompacted |= src->bits1.ud & 0x7fff;
> -   uncompacted |= (src->bits1.ud >> 29) << 15;
> +   uint32_t uncompacted =                           /* 18b */
> +      (brw_inst_dst_address_mode(brw, src) << 17) | /* 1b */
> +      (brw_inst_dst_hstride(brw, src)      << 15) | /* 2b */
> +      (brw_inst_src1_reg_type(brw, src)    << 12) | /* 3b */
> +      (brw_inst_src1_reg_file(brw, src)    << 10) | /* 2b */
> +      (brw_inst_src0_reg_type(brw, src)    <<  7) | /* 3b */
> +      (brw_inst_src0_reg_file(brw, src)    <<  5) | /* 2b */
> +      (brw_inst_dst_reg_type(brw, src)     <<  2) | /* 3b */
> +      (brw_inst_dst_reg_file(brw, src)     <<  0);  /* 2b */
>  
>     for (int i = 0; i < 32; i++) {
>        if (datatype_table[i] == uncompacted) {
> @@ -372,17 +387,17 @@ set_datatype_index(struct brw_compact_instruction 
*dst,
>  }
>  
>  static bool
> -set_subreg_index(struct brw_compact_instruction *dst,
> +set_subreg_index(struct brw_context *brw,
> +                 struct brw_compact_instruction *dst,
>                   struct brw_instruction *src,
>                   bool is_immediate)
>  {
> -   uint16_t uncompacted = 0;
> -
> -   uncompacted |= src->bits1.da1.dest_subreg_nr << 0;
> -   uncompacted |= src->bits2.da1.src0_subreg_nr << 5;
> +   uint16_t uncompacted =                            /* 15b */
> +      (brw_inst_dst_da1_subreg_nr(brw, src)  << 0) | /* 5b */
> +      (brw_inst_src0_da1_subreg_nr(brw, src) << 5);  /* 5b */
>  
>     if (!is_immediate)
> -      uncompacted |= src->bits3.da1.src1_subreg_nr << 10;
> +      uncompacted |= brw_inst_src1_da1_subreg_nr(brw, src) << 10; /* 5b */
>  
>     for (int i = 0; i < 32; i++) {
>        if (subreg_table[i] == uncompacted) {
> @@ -409,12 +424,18 @@ get_src_index(uint16_t uncompacted,
>  }
>  
>  static bool
> -set_src0_index(struct brw_compact_instruction *dst,
> +set_src0_index(struct brw_context *brw,
> +               struct brw_compact_instruction *dst,
>                 struct brw_instruction *src)
>  {
> -   uint16_t compacted, uncompacted = 0;
> -
> -   uncompacted |= (src->bits2.ud >> 13) & 0xfff;
> +   uint16_t compacted;
> +   uint16_t uncompacted =                           /* 12b */
> +      (brw_inst_src0_vstride(brw, src)      << 8) | /* 4b */
> +      (brw_inst_src0_width(brw, src)        << 5) | /* 3b */
> +      (brw_inst_src0_hstride(brw, src)      << 3) | /* 2b */

One thing that's a little funny here...we pull out hstride/width/vstride, 
which makes sense for align1 mode...but presumably this function is also used 
for align16 mode, where we instead have src0_da16_swiz_x etc.

But, it's the same bits, so this ought to work.  I'm not objecting, it's 
just...a little funny at first glance.

I don't think it's worth adding conditionals, but would it be worth adding a 
comment saying basically /* this also works for align16 mode because they 
share the same bits */?

Whatever you decide is fine.  This patch is:
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140614/864819ab/attachment.sig>