[Mesa-dev] [PATCH 06/10] i965: Lower the GLSL ES 3.00 pack/unpack operations

[Ian Romanick]

On 01/10/2013 12:10 AM, Chad Versace wrote:
> On gen < 7, we fully lower all operations to arithmetic and bitwise
> operations.
>
> On gen >= 7, we fully lower the Snorm2x16 and Unorm2x16 operations, and
> partially lower the Half2x16 operations.
>
> Signed-off-by: Chad Versace <chad.versace at linux.intel.com>
> ---
>   src/glsl/lower_packing_builtins.cpp      |  1 +
>   src/mesa/drivers/dri/i965/brw_shader.cpp | 32 ++++++++++++++++++++++++++++++++
>   2 files changed, 33 insertions(+)
>
> diff --git a/src/glsl/lower_packing_builtins.cpp b/src/glsl/lower_packing_builtins.cpp
> index cd84084..f965a27 100644
> --- a/src/glsl/lower_packing_builtins.cpp
> +++ b/src/glsl/lower_packing_builtins.cpp
> @@ -1013,6 +1013,7 @@ private:
>            new(mem_ctx) ir_variable(glsl_type::vec2_type,
>                                     "tmp_split_pack_half_2x16_v",
>                                     ir_var_temporary);
> +      insert_instruction(v);
>         insert_instruction(
>            new(mem_ctx) ir_assignment(
>               new(mem_ctx) ir_dereference_variable(v),

Shouldn't this hunk be in the previous patch?

> diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp b/src/mesa/drivers/dri/i965/brw_shader.cpp
> index 1e8d574..65f8e7d 100644
> --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
> @@ -75,6 +75,34 @@ brw_shader_precompile(struct gl_context *ctx, struct gl_shader_program *prog)
>      return true;
>   }
>
> +static void
> +brw_lower_packing_builtins(struct brw_context *brw,
> +                           gl_shader_type shader_type,
> +                           exec_list *ir)
> +{
> +   int ops = LOWER_PACK_SNORM_2x16
> +           | LOWER_UNPACK_SNORM_2x16
> +           | LOWER_PACK_UNORM_2x16
> +           | LOWER_UNPACK_UNORM_2x16;
> +
> +   if (brw->intel.gen >= 7) {
> +      switch (shader_type) {
> +      case MESA_SHADER_FRAGMENT:
> +         /* Scalarize the these operations. */
> +         ops |= LOWER_PACK_HALF_2x16_TO_SPLIT
> +             |  LOWER_UNPACK_HALF_2x16_TO_SPLIT;
> +         break;

Do we think other shader types are going to need similar treatment? 
Otherwise an if-statement would be better.

> +      default:
> +         break;
> +      }
> +   } else {
> +      ops |= LOWER_PACK_HALF_2x16
> +          |  LOWER_UNPACK_HALF_2x16;
> +   }
> +
> +   lower_packing_builtins(ir, ops);
> +}
> +
>   GLboolean
>   brw_link_shader(struct gl_context *ctx, struct gl_shader_program *shProg)
>   {
> @@ -113,6 +141,10 @@ brw_link_shader(struct gl_context *ctx, struct gl_shader_program *shProg)
>         shader->ir = new(shader) exec_list;
>         clone_ir_list(mem_ctx, shader->ir, shader->base.ir);
>
> +      /* lower_packing_builtins() inserts arithmetic instructions, so it
> +       * must precede lower_instructions().
> +       */
> +      brw_lower_packing_builtins(brw, (gl_shader_type) stage, shader->ir);
>         do_mat_op_to_vec(shader->ir);
>         lower_instructions(shader->ir,
>   			 MOD_TO_FRACT |
>