[Mesa-dev] [PATCH v2 1/3] i965: Introduce a MOV_INDIRECT opcode.

Jason Ekstrand jason at jlekstrand.net
Thu Nov 12 19:27:31 PST 2015


On Wed, Nov 11, 2015 at 12:02 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> The geometry and tessellation control shader stages both read from
> multiple URB entries (one per vertex).  The thread payload contains
> several URB handles which reference these separate memory segments.
>
> In GLSL, these inputs are represented as per-vertex arrays; the
> outermost array index selects which vertex's inputs to read.  This
> array index does not necessarily need to be constant.
>
> To handle that, we need to use indirect addressing on GRFs to select
> which of the thread payload registers has the appropriate URB handle.
> (This is before we can even think about applying the pull model!)
>
> This patch introduces a new opcode which performs a MOV from a
> source using VxH indirect addressing (which allows each of the 8
> SIMD channels to select distinct data.)
>
> Based on a patch by Jason Ekstrand.
>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/drivers/dri/i965/brw_defines.h        |  9 +++++++
>  src/mesa/drivers/dri/i965/brw_fs.cpp           | 24 ++++++++++++++++++
>  src/mesa/drivers/dri/i965/brw_fs.h             |  5 ++++
>  src/mesa/drivers/dri/i965/brw_fs_cse.cpp       |  1 +
>  src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 34 ++++++++++++++++++++++++++
>  src/mesa/drivers/dri/i965/brw_shader.cpp       |  2 ++
>  6 files changed, 75 insertions(+)
>
> Jason,
>
> I omitted a few things from your patch:
>
>  - UNIFORM handling in regs_read() - didn't want to ship it without testing it
>    So you'll need to add this back in when you start using it.
>  - Gen7 base_offset munging - on IRC you suggested dropping imm_offset, so it
>    didn't appear to actually do anything.  Feel free to put it back in if you
>    need it, or switch your code to not use base_offset.
>
> But we're using regs_read() now rather than RA hackery, and using a register
> with a HW_REG file instead of an IMM offset, so that should be a lot more
> reusable.
>
> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h b/src/mesa/drivers/dri/i965/brw_defines.h
> index 99a3a2d..6e1cfed 100644
> --- a/src/mesa/drivers/dri/i965/brw_defines.h
> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> @@ -1268,6 +1268,15 @@ enum opcode {
>      * Calculate the high 32-bits of a 32x32 multiply.
>      */
>     SHADER_OPCODE_MULH,
> +
> +   /**
> +    * A MOV that uses VxH indirect addressing.
> +    *
> +    * Source 0: A register to start from (HW_REG).
> +    * Source 1: An indirect offset (in bytes, UD GRF).
> +    * Source 2: The maximum value of the indirect offset (in bytes, UD IMM).
> +    */
> +   SHADER_OPCODE_MOV_INDIRECT,
>  };
>
>  enum brw_urb_write_flags {
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index be712e5..bf8a4a6 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -841,6 +841,30 @@ fs_inst::regs_read(int arg) const
>     case SHADER_OPCODE_BARRIER:
>        return 1;
>
> +   case SHADER_OPCODE_MOV_INDIRECT:
> +      if (arg == 0) {
> +         assert(src[2].file == IMM);
> +         unsigned max_indirect = src[2].fixed_hw_reg.dw1.ud;
> +
> +         if (src[0].file == HW_REG) {
> +            assert(src[0].file == HW_REG);
> +
> +            unsigned regs = DIV_ROUND_UP(max_indirect, REG_SIZE);
> +
> +            /* If the start of the region is not register aligned, then
> +             * we only read part of the register at the beginning, but
> +             * overlap into another register at the end.  So add 1.
> +             */
> +            if (src[0].fixed_hw_reg.subnr)
> +               regs++;

This isn't quite correct.  It's actually more complicated based on the
register stride, the actual subnr, and the actual (not rounded) size
of max_indirect.  However, this will give you enough registers and,
since we're dealing with HW_REG, that's good enough.

> +
> +            return regs;
> +         } else {
> +            assert(!"Invalid register file");
> +         }
> +      }
> +      break;
> +
>     default:
>        if (is_tex() && arg == 0 && src[0].file == GRF)
>           return mlen;
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
> index 8a93b56..c599cde 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -526,6 +526,11 @@ private:
>                                   struct brw_reg offset,
>                                   struct brw_reg value);
>
> +   void generate_mov_indirect(fs_inst *inst,
> +                              struct brw_reg dst,
> +                              struct brw_reg reg,
> +                              struct brw_reg indirect_byte_offset);
> +
>     bool patch_discard_jumps_to_fb_writes();
>
>     const struct brw_compiler *compiler;
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_cse.cpp b/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
> index 3a28c8d..ffb5f87 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
> @@ -78,6 +78,7 @@ is_expression(const fs_visitor *v, const fs_inst *const inst)
>     case FS_OPCODE_LINTERP:
>     case SHADER_OPCODE_FIND_LIVE_CHANNEL:
>     case SHADER_OPCODE_BROADCAST:
> +   case SHADER_OPCODE_MOV_INDIRECT:
>        return true;
>     case SHADER_OPCODE_RCP:
>     case SHADER_OPCODE_RSQ:
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> index 974219f..97a85bb 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> @@ -368,6 +368,36 @@ fs_generator::generate_fb_write(fs_inst *inst, struct brw_reg payload)
>  }
>
>  void
> +fs_generator::generate_mov_indirect(fs_inst *inst,
> +                                    struct brw_reg dst,
> +                                    struct brw_reg reg,
> +                                    struct brw_reg indirect_byte_offset)
> +{
> +   assert(indirect_byte_offset.type == BRW_REGISTER_TYPE_UD);
> +   assert(indirect_byte_offset.file == BRW_GENERAL_REGISTER_FILE);
> +
> +   unsigned imm_byte_offset = reg.nr * REG_SIZE + reg.subnr;
> +
> +   /* We use VxH indirect addressing, clobbering a0.0 through a0.7. */
> +   struct brw_reg addr = vec8(brw_address_reg(0));
> +
> +   /* The destination stride of an instruction (in bytes) must be greater
> +    * than or equal to the size of the rest of the instruction.  Since the
> +    * address register is of type UW, we can't use a D-type instruction.
> +    * In order to get around this, re re-type to UW and use a stride.
> +    */
> +   indirect_byte_offset =
> +      retype(spread(indirect_byte_offset, 2), BRW_REGISTER_TYPE_UW);
> +
> +   /* Prior to Broadwell, there are only 8 address registers. */
> +   assert(inst->exec_size == 8 || devinfo->gen >= 8);
> +
> +   brw_MOV(p, addr, indirect_byte_offset);
> +   brw_inst_set_mask_control(devinfo, brw_last_inst, BRW_MASK_DISABLE);

Why are we disabling the mask?

> +   brw_MOV(p, dst, retype(brw_VxH_indirect(0, imm_byte_offset), dst.type));
> +}
> +
> +void
>  fs_generator::generate_urb_read(fs_inst *inst,
>                                  struct brw_reg dst,
>                                  struct brw_reg header)
> @@ -2070,6 +2100,10 @@ fs_generator::generate_code(const cfg_t *cfg, int dispatch_width)
>           fill_count++;
>          break;
>
> +      case SHADER_OPCODE_MOV_INDIRECT:
> +         generate_mov_indirect(inst, dst, src[0], src[1]);
> +         break;
> +
>        case SHADER_OPCODE_URB_READ_SIMD8:
>           generate_urb_read(inst, dst, src[0]);
>           break;
> diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp b/src/mesa/drivers/dri/i965/brw_shader.cpp
> index a6b57c3..9550a62 100644
> --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
> @@ -552,6 +552,8 @@ brw_instruction_name(enum opcode op)
>        return "barrier";
>     case SHADER_OPCODE_MULH:
>        return "mulh";
> +   case SHADER_OPCODE_MOV_INDIRECT:
> +      return "mov_indirect";
>     }
>
>     unreachable("not reached");
> --
> 2.6.2
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list