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 <kenn...@whitecape.org>
> + 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 mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
