On Friday, October 24, 2014 03:09:45 PM Matt Turner wrote:
> Three source instructions cannot directly source a packed vec4 (<0,4,1>
> regioning) like vec4 uniforms, so we emit a MOV that expands the vec4 to
> both halves of a register.
>
> If these uniform values are used by multiple three-source instructions,
> we'll emit multiple expansion moves, which we cannot combine in CSE
> (because CSE emits moves itself).
>
> So emit a virtual instruction that we can CSE.
>
> Sometimes we demote a uniform to to a pull constant after emitting an
> expansion move for it. In that case, recognize in opt_algebraic that if
> the .file of the new instruction is GRF then it's just a real move that
> we can copy propagate and such.
>
> total instructions in shared programs: 5509805 -> 5499940 (-0.18%)
> instructions in affected programs: 348923 -> 339058 (-2.83%)
> ---
> src/mesa/drivers/dri/i965/brw_defines.h | 1 +
> src/mesa/drivers/dri/i965/brw_shader.cpp | 2 ++
> src/mesa/drivers/dri/i965/brw_vec4.cpp | 7 +++++++
> src/mesa/drivers/dri/i965/brw_vec4_cse.cpp | 1 +
> src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 1 +
> src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 2 +-
> 6 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h
b/src/mesa/drivers/dri/i965/brw_defines.h
> index a68d607..a6807ae 100644
> --- a/src/mesa/drivers/dri/i965/brw_defines.h
> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> @@ -904,6 +904,7 @@ enum opcode {
> SHADER_OPCODE_GEN7_SCRATCH_READ,
>
> VEC4_OPCODE_PACK_BYTES,
> + VEC4_OPCODE_UNPACK_UNIFORM,
>
> FS_OPCODE_DDX,
> FS_OPCODE_DDY,
> diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp
b/src/mesa/drivers/dri/i965/brw_shader.cpp
> index 87f2681..ea73dd7 100644
> --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
> @@ -457,6 +457,8 @@ brw_instruction_name(enum opcode op)
>
> case VEC4_OPCODE_PACK_BYTES:
> return "pack_bytes";
> + case VEC$_OPCODE_UNPACK_UNIFORM:MONEY! ^^^^^^^ I'm really not crazy about this patch...it's just another MOV operation, which can be CSE'd, but isn't considered everywhere else we handle MOV. Such as copy propagation, for example. I suppose this is OK, because you're only using it for "the following instruction can't handle <whatever>" MOVs, where you're not likely to be able to do things like copy propagation anyway. Speaking of, you could probably use it for other workarounds too: - resolve_ud_negate - fix_math_operand - emit_math1_gen6 - emit_math2_gen6 But I don't know if that would be helpful. Although I'm not a fan, I don't have anything better to suggest, and your statistics clearly show a benefit. It's probably fine. Reviewed-by: Kenneth Graunke <[email protected]> > + return "unpack_uniform"; > > case FS_OPCODE_DDX: > return "ddx"; > diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp b/src/mesa/drivers/dri/i965/brw_vec4.cpp > index 5029495..cb8658d 100644 > --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp > +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp > @@ -717,6 +717,13 @@ vec4_visitor::opt_algebraic() > > foreach_block_and_inst(block, vec4_instruction, inst, cfg) { > switch (inst->opcode) { > + case VEC4_OPCODE_UNPACK_UNIFORM: > + if (inst->src[0].file != UNIFORM) { > + inst->opcode = BRW_OPCODE_MOV; > + progress = true; > + } > + break; > + > case BRW_OPCODE_ADD: > if (inst->src[1].is_zero()) { > inst->opcode = BRW_OPCODE_MOV; > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp b/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp > index 28c69ca..6ff6902 100644 > --- a/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp > +++ b/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp > @@ -69,6 +69,7 @@ is_expression(const vec4_instruction *const inst) > case BRW_OPCODE_PLN: > case BRW_OPCODE_MAD: > case BRW_OPCODE_LRP: > + case VEC4_OPCODE_UNPACK_UNIFORM: > return true; > case SHADER_OPCODE_RCP: > case SHADER_OPCODE_RSQ: > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp > index 37c0806..6150d1d 100644 > --- a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp > +++ b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp > @@ -1183,6 +1183,7 @@ vec4_generator::generate_code(const cfg_t *cfg) > } > > switch (inst->opcode) { > + case VEC4_OPCODE_UNPACK_UNIFORM: > case BRW_OPCODE_MOV: > brw_MOV(p, dst, src[0]); > break; > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp > index 3b279dc..e10972b 100644 > --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp > +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp > @@ -302,7 +302,7 @@ vec4_visitor::fix_3src_operand(src_reg src) > > dst_reg expanded = dst_reg(this, glsl_type::vec4_type); > expanded.type = src.type; > - emit(MOV(expanded, src)); > + emit(VEC4_OPCODE_UNPACK_UNIFORM, expanded, src); > return src_reg(expanded); > } > >
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
