[Mesa-dev] [PATCH 1/6] i965/fs: add a helper function to create double immediates

[Francisco Jerez]

Samuel Iglesias Gonsálvez <siglesias at igalia.com> writes:

> From: Iago Toral Quiroga <itoral at igalia.com>
>
> Gen7 hardware does not support double immediates so these need
> to be moved in 32-bit chunks to a regular vgrf instead. Instead
> of doing this every time we need to create a DF immediate,
> create a helper function that does the right thing depending
> on the hardware generation.
> ---
>  src/mesa/drivers/dri/i965/brw_fs.h       |  2 ++
>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 43 ++++++++++++++++++++++++++++++++
>  2 files changed, 45 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
> index 4237197..dd7ce7d 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -167,6 +167,8 @@ public:
>     bool lower_simd_width();
>     bool opt_combine_constants();
>  
> +   fs_reg setup_imm_df(double v);
> +
>     void emit_dummy_fs();
>     void emit_repclear_shader();
>     fs_reg *emit_fragcoord_interpolation();
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> index b3f5dfd..268c847 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> @@ -616,6 +616,49 @@ fs_visitor::optimize_frontfacing_ternary(nir_alu_instr *instr,
>     return true;
>  }
>  
> +fs_reg
> +fs_visitor::setup_imm_df(double v)

Mainly nitpicking here, but because this function only needs an i965 IR
builder and doesn't otherwise care about the fs_visitor class, it would
make more sense for it to be a stand-alone function independent from
fs_visitor taking an fs_builder as argument instead.

> +{
> +   assert(devinfo->gen >= 7);
> +
> +   if (devinfo->gen >= 8)
> +      return brw_imm_df(v);
> +
> +   /* gen7 does not support DF immediates, so we generate a 64-bit constant by
> +    * writing the low 32-bit of the constant to suboffset 0 of a VGRF and
> +    * the high 32-bit to suboffset 4 and then applying a stride of 0.
> +    *
> +    * Alternatively, we could also produce a normal VGRF (without stride 0)
> +    * by writing to all the channels in the VGRF, however, that would hit the
> +    * gen7 bug where we have to split writes that span more than 1 register
> +    * into instructions with a width of 4 (otherwise the write to the second
> +    * register written runs into an execmask hardware bug) which isn't very
> +    * nice.
> +    */
> +   union {
> +      double d;
> +      struct {
> +         uint32_t i1;
> +         uint32_t i2;
> +      };
> +   } di;
> +
> +   di.d = v;
> +

You can declare a scalar builder here for convenience like:

| const fs_builder ubld = bld.exec_all().group(1, 0);

then use it below instead of 'bld' so you can get rid of the six inst
field assignments.

> +   fs_reg tmp = vgrf(glsl_type::uint_type);

On e.g. SIMD32 mode this will allocate 32 components worth of registers
even though you only need two.  Once you have a scalar builder at hand
you can do it as follows instead:

| const fs_reg tmp = ubld.vgrf(BRW_REGISTER_TYPE_UD, 2);

Other than that looks okay to me.

> +   fs_inst *inst = bld.MOV(tmp, brw_imm_ud(di.i1));
> +   inst->force_writemask_all = true;
> +   inst->exec_size = 1;
> +   inst->regs_written = 1;
> +
> +   inst = bld.MOV(horiz_offset(tmp, 1), brw_imm_ud(di.i2));
> +   inst->force_writemask_all = true;
> +   inst->exec_size = 1;
> +   inst->regs_written = 1;
> +
> +   return component(retype(tmp, BRW_REGISTER_TYPE_DF), 0);
> +}
> +
>  void
>  fs_visitor::nir_emit_alu(const fs_builder &bld, nir_alu_instr *instr)
>  {
> -- 
> 2.7.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160707/c3ad2238/attachment-0001.sig>