On 29/10/2024 09:39, Andrew Stubbs wrote:
On 28/10/2024 20:03, Robin Dapp wrote:
I'm not sure how this is different to just deleting the
zero-initializer, which is what I already tested and found some random
behaviour?

The difference is in the else-operand predicate.  So unless there are
more bugs we should only have added VCOND_EXPRs for the cases where
they are absolutely necessary and not unconditionally as currently done
in the gcn backend.

Except that the init_regs pass just puts it straight back in.... I'm testing this additional patch:

--- a/gcc/config/gcn/gcn-valu.md
+++ b/gcc/config/gcn/gcn-valu.md
@@ -4000,7 +4000,7 @@ (define_expand "maskload<mode>di"
      rtx v = gen_rtx_CONST_INT (VOIDmode, MEM_VOLATILE_P (operands[1]));


      emit_insn (gen_gather<mode>_expr_exec (operands[0], addr, as, v,
-                                          operands[0], exec));
+                                          gcn_gen_undef(<MODE>mode), exec));
      DONE;
    })

The tests with this patch (stacked on top of the original) also came back clean, and the init_regs pass didn't try to fix it up with the testcase I tried (gcc.dg/vect/vect-mask-load-1.c).

Just to be clear, gcn_gen_undef simply creates an unspec with the given mode that the insn predicates recognise as a valid input to an undefined vec_merge. The machine instruction simply leaves the previous contents of the masked lanes of the destination hardware register as it was.

The instruction requires that the input "else" value is in the same register as the destination, but at expand time you can obviously put any pseudo-registers you like in the vec_merge, and there's no need for a scratch register or any other dummy input if we use the unspec.

In principle, we can pre-initialize the destination register with any arbitrary value passed to maskload, but I think the "else" parameter does not actually work that way, as defined by the patch?

Andrew

Reply via email to