[Mesa-dev] [PATCH 05/12] i965/fs: Switch from GLSL IR to NIR for un/packHalf2x16 lowering.

[Iago Toral]

I think it would be a good idea to change the shortlog so it is clear
that we are only talking about the scalarization aspect. There is still
GLSL IR lowering for un/packHalf2x16 that remains in use after this
patch, as made clear by the last hunk in this patch.

On Mon, 2016-01-25 at 15:18 -0800, Matt Turner wrote:
> ---
>  src/mesa/drivers/dri/i965/brw_compiler.c                 |  2 ++
>  src/mesa/drivers/dri/i965/brw_fs_channel_expressions.cpp |  4 ++++
>  src/mesa/drivers/dri/i965/brw_link.cpp                   | 12 +-----------
>  3 files changed, 7 insertions(+), 11 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_compiler.c b/src/mesa/drivers/dri/i965/brw_compiler.c
> index dbd5ad2..21fff1d 100644
> --- a/src/mesa/drivers/dri/i965/brw_compiler.c
> +++ b/src/mesa/drivers/dri/i965/brw_compiler.c
> @@ -86,6 +86,8 @@ shader_perf_log_mesa(void *data, const char *fmt, ...)
>  
>  static const struct nir_shader_compiler_options scalar_nir_options = {
>     COMMON_OPTIONS,
> +   .lower_pack_half_2x16 = true,
> +   .lower_unpack_half_2x16 = true,
>  };
>  
>  static const struct nir_shader_compiler_options vector_nir_options = {
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_channel_expressions.cpp b/src/mesa/drivers/dri/i965/brw_fs_channel_expressions.cpp
> index 21f0b70..566257c 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_channel_expressions.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_channel_expressions.cpp
> @@ -72,6 +72,9 @@ channel_expressions_predicate(ir_instruction *ir)
>        return false;
>  
>     switch (expr->operation) {
> +      case ir_unop_pack_half_2x16:
> +         return false;
> +
>        /* these opcodes need to act on the whole vector,
>         * just like texturing.
>         */
> @@ -162,6 +165,7 @@ ir_channel_expressions_visitor::visit_leave(ir_assignment *ir)
>        return visit_continue;
>  
>     switch (expr->operation) {
> +      case ir_unop_pack_half_2x16:
>        case ir_unop_interpolate_at_centroid:
>        case ir_binop_interpolate_at_offset:
>        case ir_binop_interpolate_at_sample:

This opcode is still being handled below as:

...
   case ir_unop_pack_half_2x16:
...
      unreachable("should have been lowered");

so that should probably go away in this patch.

Also, don't we need to do all this same stuff for the unpack opcode?

> diff --git a/src/mesa/drivers/dri/i965/brw_link.cpp b/src/mesa/drivers/dri/i965/brw_link.cpp
> index 234afd5..8f2d760 100644
> --- a/src/mesa/drivers/dri/i965/brw_link.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_link.cpp
> @@ -87,17 +87,7 @@ brw_lower_packing_builtins(struct brw_context *brw,
>             | LOWER_PACK_SNORM_4x8;
>     }
>  
> -   if (brw->gen >= 7) {
> -      /* Gen7 introduced the f32to16 and f16to32 instructions, which can be
> -       * used to execute packHalf2x16 and unpackHalf2x16. For AOS code, no
> -       * lowering is needed. For SOA code, the Half2x16 ops must be
> -       * scalarized.
> -       */
> -      if (compiler->scalar_stage[shader_type]) {
> -         ops |= LOWER_PACK_HALF_2x16_TO_SPLIT
> -             |  LOWER_UNPACK_HALF_2x16_TO_SPLIT;
> -      }
> -   } else {
> +   if (brw->gen < 7) {
>        ops |= LOWER_PACK_HALF_2x16
>            |  LOWER_UNPACK_HALF_2x16;
>     }