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; > } _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev