[Mesa-dev] [RFC] nir: compiler options for addressing modes

[Eric Anholt]

Rob Clark <robdclark at gmail.com> writes:

> From: Rob Clark <robclark at freedesktop.org>
>
> Add compiler options so driver can request the units for address value,
> for the various _indirect intrinsics.  This way, the front end can
> insert the appropriate multiply/shift operations to get the address into
> the units that the driver needs, avoiding need for driver to insert
> these on the backend (and loose out on some optimizing potential).
>
> The addressing modes are specified per var/input/output/uniform/ubo.
> I'm not sure about other drivers, but for freedreno we want byte
> addressing for ubo's and register addressing for the rest.
>
> NOTE: so far just updated ttn and freedreno, and included everything in
> one commit to make it easier to see how it fits together.  The driver
> patches would naturally be separate (since drivers not setting these
> options get the current/default behavior, ie. splitting out won't hurt
> bisectability).  Also the ttn part could be split out as long as it
> comes before freedreno or vc4 parts.

> diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
> index 6531237..e8bce9f 100644
> --- a/src/glsl/nir/nir.h
> +++ b/src/glsl/nir/nir.h
> @@ -1374,6 +1374,23 @@ typedef struct nir_function {
>     exec_node_data(nir_function_overload, \
>                    exec_list_get_head(&(func)->overload_list), node)
>  
> +/**
> + * Addressing mode used by the backend for various sorts of indirect
> + * access.  This allows the frontend to generate the appropriate
> + * multiply or shift operations to convert the address param for the
> + * corresponding _indirect intrinsic, rather than relying on the
> + * driver to insert them after the various other NIR optimization
> + * passes.
> + */
> +typedef enum {
> +   /** No address conversion, units in elements of array member: */
> +   nir_addressing_mode_none = 0,
> +   /** Address converted to units of registers (ie. float/int32): */
> +   nir_addressing_mode_register = 1,
> +   /** Address converted to units of bytes: */
> +   nir_addressing_mode_byte = 2,
> +} nir_addressing_mode;
> +
>  typedef struct nir_shader_compiler_options {
>     bool lower_ffma;
>     bool lower_flrp;
> @@ -1393,6 +1410,16 @@ typedef struct nir_shader_compiler_options {
>      * are simulated by floats.)
>      */
>     bool native_integers;
> +
> +   /**
> +    * Addressing mode for corresponding _indirect intrinsics:
> +    */
> +   nir_addressing_mode var_addressing_mode;
> +   nir_addressing_mode input_addressing_mode;
> +   nir_addressing_mode output_addressing_mode;
> +   nir_addressing_mode uniform_addressing_mode;
> +   nir_addressing_mode ubo_addressing_mode;
> +
>  } nir_shader_compiler_options;
>  
>  typedef struct nir_shader {
> -- 
> 2.1.0

This seems like a good idea, as it'll mean we can have passes that do
things like scalarizing uniform loads, which I think could be productive
for us.

However, I find the "register" size name confusing -- it seems you mean
a channel in a register, while on both the pieces of hardware I've
worked on recently, a register is 16 dwords wide.  I think I'd call it
"dword" or "float", with a slight preference for dword.

Also, should this apply to just the indirect variants? I'd think they
would apply to the const_index component, as well.  Either that, or we
make const_index always be bytes, maybe.

This patch should probably edit the comment in nir_intrinsics.h about
addressing, too.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150414/04f73722/attachment.sig>