[Mesa-dev] [PATCH] i965/fs: initialize src as reg_undef for texture opcodes

[Robert Bragg]

Hey Tapani,

I came across this issue the other day too and can at least confirm
that I wrote an almost identical patch.

I was a bit unsure whether this was the best place to fix this issue
since it seems a bit unobvious, in isolation, why we emit these
texture ops with a place holder register, but maybe for others more
experienced than me with the code, this is fine.

Something I wasn't clear about, was whether the
gen6_resolve_implied_move() call in brw_SAMPLE is really needed. It
looks like the fs and vec4 visitors (where we emit these ops) both
explicitly setup an MRF payload and before 07af0ab both would have
effectively passed reg_undef registers through to generate_tex() which
would just result in the _resolve_move() being a no-op that returns a
register for msg_reg_nr.

As another possible approach; I smoke tested the following change
which also seemed to fixed this issue for me:

diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c
b/src/mesa/drivers/dri/i965/brw_eu_emit.c
index 2fa65e9..899fe47 100644
--- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
+++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
@@ -2317,7 +2317,7 @@ void brw_SAMPLE(struct brw_compile *p,
    struct brw_instruction *insn;

    if (msg_reg_nr != -1)
-      gen6_resolve_implied_move(p, &src0, msg_reg_nr);
+      src0 = brw_message_reg(msg_reg_nr);

    insn = next_insn(p, BRW_OPCODE_SEND);
    insn->header.predicate_control = 0; /* XXX */

I'm not familiar enough with this to know if a change like that would
be right/acceptable though and haven't had a chance to try and run
piglit to see if that breaks something I'm not aware of.

--
Regards,
Robert


On Mon, Jun 9, 2014 at 10:30 AM, Tapani Pälli <tapani.palli at intel.com> wrote:
> Commit 07af0ab changed fs_inst to have 0 sources for texture opcodes
> in emit_texture_gen5 (Ironlake, Sandybrige) while fs_generator still
> uses a single source from brw_reg struct. Patch sets src as reg_undef
> which matches the behavior before the constructor got changed.
>
> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=79534
> ---
>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> index 0b4fbb7..3089932 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> @@ -1171,21 +1171,21 @@ fs_visitor::emit_texture_gen5(ir_texture *ir, fs_reg dst, fs_reg coordinate,
>     fs_inst *inst = NULL;
>     switch (ir->op) {
>     case ir_tex:
> -      inst = emit(SHADER_OPCODE_TEX, dst);
> +      inst = emit(SHADER_OPCODE_TEX, dst, reg_undef);
>        break;
>     case ir_txb:
>        mlen = MAX2(mlen, header_present + 4 * reg_width);
>        emit(MOV(fs_reg(MRF, base_mrf + mlen), lod));
>        mlen += reg_width;
>
> -      inst = emit(FS_OPCODE_TXB, dst);
> +      inst = emit(FS_OPCODE_TXB, dst, reg_undef);
>        break;
>     case ir_txl:
>        mlen = MAX2(mlen, header_present + 4 * reg_width);
>        emit(MOV(fs_reg(MRF, base_mrf + mlen), lod));
>        mlen += reg_width;
>
> -      inst = emit(SHADER_OPCODE_TXL, dst);
> +      inst = emit(SHADER_OPCODE_TXL, dst, reg_undef);
>        break;
>     case ir_txd: {
>        mlen = MAX2(mlen, header_present + 4 * reg_width); /* skip over 'ai' */
> @@ -1215,17 +1215,17 @@ fs_visitor::emit_texture_gen5(ir_texture *ir, fs_reg dst, fs_reg coordinate,
>     case ir_txs:
>        emit(MOV(fs_reg(MRF, base_mrf + mlen, BRW_REGISTER_TYPE_UD), lod));
>        mlen += reg_width;
> -      inst = emit(SHADER_OPCODE_TXS, dst);
> +      inst = emit(SHADER_OPCODE_TXS, dst, reg_undef);
>        break;
>     case ir_query_levels:
>        emit(MOV(fs_reg(MRF, base_mrf + mlen, BRW_REGISTER_TYPE_UD), fs_reg(0u)));
>        mlen += reg_width;
> -      inst = emit(SHADER_OPCODE_TXS, dst);
> +      inst = emit(SHADER_OPCODE_TXS, dst, reg_undef);
>        break;
>     case ir_txf:
>        mlen = header_present + 4 * reg_width;
>        emit(MOV(fs_reg(MRF, base_mrf + mlen - reg_width, BRW_REGISTER_TYPE_UD), lod));
> -      inst = emit(SHADER_OPCODE_TXF, dst);
> +      inst = emit(SHADER_OPCODE_TXF, dst, reg_undef);
>        break;
>     case ir_txf_ms:
>        mlen = header_present + 4 * reg_width;
> @@ -1235,13 +1235,13 @@ fs_visitor::emit_texture_gen5(ir_texture *ir, fs_reg dst, fs_reg coordinate,
>        /* sample index */
>        emit(MOV(fs_reg(MRF, base_mrf + mlen, BRW_REGISTER_TYPE_UD), sample_index));
>        mlen += reg_width;
> -      inst = emit(SHADER_OPCODE_TXF_CMS, dst);
> +      inst = emit(SHADER_OPCODE_TXF_CMS, dst, reg_undef);
>        break;
>     case ir_lod:
> -      inst = emit(SHADER_OPCODE_LOD, dst);
> +      inst = emit(SHADER_OPCODE_LOD, dst, reg_undef);
>        break;
>     case ir_tg4:
> -      inst = emit(SHADER_OPCODE_TG4, dst);
> +      inst = emit(SHADER_OPCODE_TG4, dst, reg_undef);
>        break;
>     default:
>        fail("unrecognized texture opcode");
> --
> 1.8.3.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev