[Mesa-dev] [PATCH 14/22] intel/compiler: Do image load/store lowering to NIR

Kenneth Graunke kenneth at whitecape.org
Wed Aug 29 05:49:21 UTC 2018


On Friday, August 17, 2018 1:06:20 PM PDT Jason Ekstrand wrote:
[snip]
> +# Intel-specific query for loading from the brw_image_param struct passed
> +# into the shader as a uniform.  The variable is a deref to the image
> +# variable. The const index specifies which of the six parameters to load.

This might be our first driver-specific intrinsics.  Some people make
big extensibility systems for that, where drivers can extend with their
own concepts.  But given that we all live in the same project, I think
this makes a lot of sense - just have everybody add their own here,
suffixed with a name they own (i.e. intel, amd, radv, ir3, whatever).

It's certainly nice and simple.

> +intrinsic("image_deref_load_param_intel", src_comp=[1], dest_comp=0,
> +          indices=[BASE], flags=[CAN_ELIMINATE, CAN_REORDER])
> +intrinsic("image_deref_load_raw_intel", src_comp=[1, 1], dest_comp=0,
> +          flags=[CAN_ELIMINATE, CAN_REORDER])
> +intrinsic("image_deref_store_raw_intel", src_comp=[1, 1, 0])

I don't think you want CAN_REORDER for the new load intrinsic...at
least, image_deref_load only has CAN_ELIMINATE.  It probably makes
sense for the two to match...

[snip]
> +static nir_ssa_def *
> +image_address(nir_builder *b, const struct gen_device_info *devinfo,
> +              nir_deref_instr *deref, nir_ssa_def *coord)
> +{
> +   coord = sanitize_image_coord(b, deref, coord);
> +
> +   nir_ssa_def *offset = load_image_param(b, deref, OFFSET);
> +   nir_ssa_def *tiling = load_image_param(b, deref, TILING);
> +   nir_ssa_def *stride = load_image_param(b, deref, STRIDE);
> +
> +   /* Shift the coordinates by the fixed surface offset.  It may be non-zero
> +    * if the image is a single slice of a higher-dimensional surface, or if a
> +    * non-zero mipmap level of the surface is bound to the pipeline.  The
> +    * offset needs to be applied here rather than at surface state set-up time
> +    * because the desired slice-level may start mid-tile, so simply shifting
> +    * the surface base address wouldn't give a well-formed tiled surface in
> +    * the general case.
> +    */
> +   nir_ssa_def *xypos = (coord->num_components == 1) ?
> +                        nir_vec2(b, coord, nir_imm_int(b, 0)) :
> +                        nir_channels(b, coord, 0x3);
> +   xypos = nir_iadd(b, xypos, offset);
> +
> +   /* The layout of 3-D textures in memory is sort-of like a tiling
> +    * format.  At each miplevel, the slices are arranged in rows of
> +    * 2^level slices per row.  The slice row is stored in tmp.y and
> +    * the slice within the row is stored in tmp.x.
> +    *
> +    * The layout of 2-D array textures and cubemaps is much simpler:
> +    * Depending on whether the ARYSPC_LOD0 layout is in use it will be
> +    * stored in memory as an array of slices, each one being a 2-D
> +    * arrangement of miplevels, or as a 2D arrangement of miplevels,
> +    * each one being an array of slices.  In either case the separation
> +    * between slices of the same LOD is equal to the qpitch value
> +    * provided as stride.w.
> +    *
> +    * This code can be made to handle either 2D arrays and 3D textures
> +    * by passing in the miplevel as tile.z for 3-D textures and 0 in
> +    * tile.z for 2-D array textures.
> +    *
> +    * See Volume 1 Part 1 of the Gen7 PRM, sections 6.18.4.7 "Surface
> +    * Arrays" and 6.18.6 "3D Surfaces" for a more extensive discussion
> +    * of the hardware 3D texture and 2D array layouts.
> +    */
> +   if (coord->num_components > 2) {
> +      /* Decompose z into a major (tmp.y) and a minor (tmp.x)
> +       * index.
> +       */
> +      nir_ssa_def *z = nir_channel(b, coord, 2);
> +      nir_ssa_def *z_x = nir_ubfe(b, z, nir_imm_int(b, 0),
> +                                  nir_channel(b, tiling, 2));
> +      nir_ssa_def *z_y = nir_ushr(b, z, nir_channel(b, tiling, 2));
> +
> +      /* Take into account the horizontal (tmp.x) and vertical (tmp.y)
> +       * slice offset.
> +       */
> +      xypos = nir_iadd(b, xypos, nir_imul(b, nir_vec2(b, z_x, z_y),
> +                                             nir_channels(b, stride, 0xc)));
> +   }
> +
> +   nir_ssa_def *addr;
> +   if (coord->num_components > 1) {
> +      /* Calculate the major/minor x and y indices.  In order to
> +       * accommodate both X and Y tiling, the Y-major tiling format is
> +       * treated as being a bunch of narrow X-tiles placed next to each
> +       * other.  This means that the tile width for Y-tiling is actually
> +       * the width of one sub-column of the Y-major tile where each 4K
> +       * tile has 8 512B sub-columns.
> +       *
> +       * The major Y value is the row of tiles in which the pixel lives.
> +       * The major X value is the tile sub-column in which the pixel
> +       * lives; for X tiling, this is the same as the tile column, for Y
> +       * tiling, each tile has 8 sub-columns.  The minor X and Y indices
> +       * are the position within the sub-column.
> +       */
> +
> +      /* Calculate the minor x and y indices. */
> +      nir_ssa_def *minor = nir_ubfe(b, xypos, nir_imm_int(b, 0),
> +                                       nir_channels(b, tiling, 0x3));
> +      nir_ssa_def *major = nir_ushr(b, xypos, nir_channels(b, tiling, 0x3));
> +
> +      /* Calculate the texel index from the start of the tile row and the
> +       * vertical coordinate of the row.
> +       * Equivalent to:
> +       *   tmp.x = (major.x << tile.y << tile.x) +
> +       *           (minor.y << tile.x) + minor.x
> +       *   tmp.y = major.y << tile.y
> +       */
> +      nir_ssa_def *idx_x, *idx_y;
> +      idx_x = nir_ishl(b, nir_channel(b, major, 0), nir_channel(b, tiling, 1));
> +      idx_x = nir_iadd(b, idx_x, nir_channel(b, minor, 1));
> +      idx_x = nir_ishl(b, idx_x, nir_channel(b, tiling, 0));
> +      idx_x = nir_iadd(b, idx_x, nir_channel(b, minor, 0));
> +      idx_y = nir_ishl(b, nir_channel(b, major, 1), nir_channel(b, tiling, 1));
> +
> +      /* Add it to the start of the tile row. */
> +      nir_ssa_def *idx;
> +      idx = nir_imul(b, idx_y, nir_channel(b, stride, 1));
> +      idx = nir_iadd(b, idx, idx_x);
> +

Maybe preserve the /* Multiply by Bpp value */ comment here?

> +      addr = nir_imul(b, idx, nir_channel(b, stride, 0));
> +
> +      if (devinfo->gen < 8 && !devinfo->is_baytrail) {
> +         /* Take into account the two dynamically specified shifts.  Both need
> +          * are used to implement swizzling of X-tiled surfaces.  For Y-tiled

May as well fix the "Both need are used" wording here -> "Both are used"

[snip]
> +static bool
> +lower_image_load_instr(nir_builder *b,
> +                       const struct gen_device_info *devinfo,
> +                       nir_intrinsic_instr *intrin)
> +{
> +   nir_deref_instr *deref = nir_src_as_deref(intrin->src[0]);
> +   nir_variable *var = nir_deref_instr_get_variable(deref);
> +   const enum isl_format image_fmt =
> +      isl_format_for_gl_format(var->data.image.format);
> +
> +   if (isl_has_matching_typed_storage_image_format(devinfo, image_fmt)) {
> +      const enum isl_format lower_fmt =
> +         isl_lower_storage_image_format(devinfo, image_fmt);
> +      const unsigned dest_components = intrin->num_components;
> +
> +      /* Use an undef to hold the uses of the load while we do the color
> +       * conversion.
> +       */
> +      nir_ssa_def *placeholder = nir_ssa_undef(b, 4, 32);
> +      nir_ssa_def_rewrite_uses(&intrin->dest.ssa, nir_src_for_ssa(placeholder));
> +
> +      intrin->num_components = isl_format_get_num_channels(lower_fmt);
> +      intrin->dest.ssa.num_components = intrin->num_components;
> +
> +      nir_ssa_def *value;
> +      if (devinfo->gen == 7 && !devinfo->is_haswell) {
> +         /* Check the first component of the size field to find out if the
> +          * image is bound.  Necessary on IVB because it don't seem to respect
> +          * null surfaces and will hang when no image is bound.
> +          */
> +         b->cursor = nir_instr_remove(&intrin->instr);
> +         nir_ssa_def *size = load_image_param(b, deref, SIZE);
> +         nir_push_if(b, nir_ine(b, nir_channel(b, size, 0), nir_imm_int(b, 0)));
> +         nir_builder_instr_insert(b, &intrin->instr);
> +         nir_push_else(b, NULL);
> +         nir_ssa_def *zero = nir_zero_vec(b, intrin->num_components);
> +         nir_pop_if(b, NULL);
> +         value = nir_if_phi(b, &intrin->dest.ssa, zero);
> +      } else {
> +         b->cursor = nir_after_instr(&intrin->instr);
> +         value = &intrin->dest.ssa;
> +      }

I don't think this Gen7 code is necessary...at least, I don't see the
old code checking this for typed loads.  (Atomics, yes...loads, no...)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180828/8ba3b305/attachment.sig>


More information about the mesa-dev mailing list