[Mesa-dev] [PATCH v2 3/8] i965/fs: Add an enum for keeping track of texture instruciton sources

Kenneth Graunke kenneth at whitecape.org
Mon Feb 8 23:18:14 UTC 2016


On Saturday, February 6, 2016 10:19:47 AM PST Jason Ekstrand wrote:
> These logical texture instructions can have a *lot* of sources.  It's much
> safer if we have symbolic names for them.
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp         | 46 +++++++++++++++-------------
>  src/mesa/drivers/dri/i965/brw_fs.h           | 14 +++++++++
>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 19 ++++++++----
>  3 files changed, 52 insertions(+), 27 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 41a3f81..8f4a7c1 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -739,18 +739,20 @@ fs_inst::components_read(unsigned i) const
>     case SHADER_OPCODE_LOD_LOGICAL:
>     case SHADER_OPCODE_TG4_LOGICAL:
>     case SHADER_OPCODE_TG4_OFFSET_LOGICAL:
> -      assert(src[8].file == IMM && src[9].file == IMM);
> +      assert(src[FS_TEX_SRC_COORD_COMPONENTS].file == IMM &&
> +             src[FS_TEX_SRC_GRAD_COMPONENTS].file == IMM);
>        /* Texture coordinates. */
> -      if (i == 0)
> -         return src[8].ud;
> +      if (i == FS_TEX_SRC_COORDINATE)
> +         return src[FS_TEX_SRC_COORD_COMPONENTS].ud;
>        /* Texture derivatives. */
> -      else if ((i == 2 || i == 3) && opcode == SHADER_OPCODE_TXD_LOGICAL)
> -         return src[9].ud;
> +      else if ((i == FS_TEX_SRC_LOD || i == FS_TEX_SRC_LOD2) &&
> +               opcode == SHADER_OPCODE_TXD_LOGICAL)
> +         return src[FS_TEX_SRC_GRAD_COMPONENTS].ud;
>        /* Texture offset. */
> -      else if (i == 7)
> +      else if (i == FS_TEX_SRC_OFFSET_VALUE)
>           return 2;
>        /* MCS */
> -      else if (i == 5 && opcode == SHADER_OPCODE_TXF_CMS_W_LOGICAL)
> +      else if (i == FS_TEX_SRC_MCS && opcode == SHADER_OPCODE_TXF_CMS_W_LOGICAL)
>           return 2;
>        else
>           return 1;
> @@ -4080,17 +4082,18 @@ static void
>  lower_sampler_logical_send(const fs_builder &bld, fs_inst *inst, opcode op)
>  {
>     const brw_device_info *devinfo = bld.shader->devinfo;
> -   const fs_reg &coordinate = inst->src[0];
> -   const fs_reg &shadow_c = inst->src[1];
> -   const fs_reg &lod = inst->src[2];
> -   const fs_reg &lod2 = inst->src[3];
> -   const fs_reg &sample_index = inst->src[4];
> -   const fs_reg &mcs = inst->src[5];
> -   const fs_reg &sampler = inst->src[6];
> -   const fs_reg &offset_value = inst->src[7];
> -   assert(inst->src[8].file == IMM && inst->src[9].file == IMM);
> -   const unsigned coord_components = inst->src[8].ud;
> -   const unsigned grad_components = inst->src[9].ud;
> +   const fs_reg &coordinate = inst->src[FS_TEX_SRC_COORDINATE];
> +   const fs_reg &shadow_c = inst->src[FS_TEX_SRC_SHADOW_C];
> +   const fs_reg &lod = inst->src[FS_TEX_SRC_LOD];
> +   const fs_reg &lod2 = inst->src[FS_TEX_SRC_LOD2];
> +   const fs_reg &sample_index = inst->src[FS_TEX_SRC_SAMPLE_INDEX];
> +   const fs_reg &mcs = inst->src[FS_TEX_SRC_MCS];
> +   const fs_reg &sampler = inst->src[FS_TEX_SRC_SAMPLER];
> +   const fs_reg &offset_value = inst->src[FS_TEX_SRC_OFFSET_VALUE];
> +   assert(inst->src[FS_TEX_SRC_COORD_COMPONENTS].file == IMM);
> +   const unsigned coord_components = inst->src[FS_TEX_SRC_COORD_COMPONENTS].ud;
> +   assert(inst->src[FS_TEX_SRC_GRAD_COMPONENTS].file == IMM);
> +   const unsigned grad_components = inst->src[FS_TEX_SRC_GRAD_COMPONENTS].ud;
>  
>     if (devinfo->gen >= 7) {
>        lower_sampler_logical_send_gen7(bld, inst, op, coordinate,
> @@ -4384,7 +4387,7 @@ get_lowered_simd_width(const struct brw_device_info *devinfo,
>  
>     case SHADER_OPCODE_TG4_OFFSET_LOGICAL: {
>        /* gather4_po_c is unsupported in SIMD16 mode. */
> -      const fs_reg &shadow_c = inst->src[1];
> +      const fs_reg &shadow_c = inst->src[FS_TEX_SRC_SHADOW_C];
>        return (shadow_c.file != BAD_FILE ? 8 : inst->exec_size);
>     }
>     case SHADER_OPCODE_TXL_LOGICAL:
> @@ -4393,7 +4396,7 @@ get_lowered_simd_width(const struct brw_device_info *devinfo,
>         * Gen4-6 can't support TXL and TXB with shadow comparison in SIMD16
>         * mode because the message exceeds the maximum length of 11.
>         */
> -      const fs_reg &shadow_c = inst->src[1];
> +      const fs_reg &shadow_c = inst->src[FS_TEX_SRC_SHADOW_C];
>        if (devinfo->gen == 4 && shadow_c.file == BAD_FILE)
>           return 16;
>        else if (devinfo->gen < 7 && shadow_c.file != BAD_FILE)
> @@ -4416,7 +4419,8 @@ get_lowered_simd_width(const struct brw_device_info *devinfo,
>         * circumstances it can end up with a message that is too long in SIMD16
>         * mode.
>         */
> -      const unsigned coord_components = inst->src[8].ud;
> +      const unsigned coord_components =
> +         inst->src[FS_TEX_SRC_COORD_COMPONENTS].ud;
>        /* First three arguments are the sample index and the two arguments for
>         * the MCS data.
>         */
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
> index 4612a28..4a38f80 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -66,6 +66,20 @@ offset(fs_reg reg, const brw::fs_builder& bld, unsigned delta)
>     return reg;
>  }
>  
> +enum fs_tex_src_type {
> +   FS_TEX_SRC_COORDINATE,
> +   FS_TEX_SRC_SHADOW_C,
> +   FS_TEX_SRC_LOD,
> +   FS_TEX_SRC_LOD2,
> +   FS_TEX_SRC_SAMPLE_INDEX,
> +   FS_TEX_SRC_MCS,
> +   FS_TEX_SRC_SAMPLER,
> +   FS_TEX_SRC_OFFSET_VALUE,
> +   FS_TEX_SRC_COORD_COMPONENTS,
> +   FS_TEX_SRC_GRAD_COMPONENTS,
> +   FS_TEX_SRC_MAX,
> +};
> +

Logical FB write messages use an enum called "fb_write_logical_srcs";
it might be nice to call this fs_tex_logical_srcs to match that.

IMO they should also go in the same file, probably right next to each
other.  enum fb_write_logical_srcs is defined in brw_defines.h...

Please extend the "Texture sampling opcodes" comment in brw_defines.h
to refer to the enum as well (i.e. \sa enum fs_tex_logical_srcs).

Thanks a ton for doing this.

With or without my suggestions,
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: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160208/c781232b/attachment.sig>


More information about the mesa-dev mailing list