On 16/02/2024 14:34, Thomas Schwinge wrote:
Hi!
On 2024-01-29T11:34:05+0100, Tobias Burnus <tbur...@baylibre.com> wrote:
Andrew wrote off list:
"Vector reductions don't work on RDNA, as is, but they're
supposed to be disabled by the insn condition"
This patch disables "fold_left_plus_<mode>", which is about
vectorization and in the code path shown in the backtrace.
I can also confirm manually that it fixes the ICE I saw and
also the ICE for the testfile that Richard's PR shows at the
end of his backtrace. (-O3 is needed to trigger the ICE.)
On top of that, OK to push the attached
"GCN: Conditionalize 'define_expand "reduc_<fexpander>_scal_<mode>"' on
'!TARGET_RDNA2_PLUS' [PR113615]"?
Which of the 'assert's are worth keeping?
Only tested 'vect.exp' for 'check-gcc-c' so far; full testing to run
later.
Please confirm I'm understanding this correctly:
Andrew's original commit c7ec7bd1c6590cf4eed267feab490288e0b8d691
"amdgcn: add -march=gfx1030 EXPERIMENTAL" did this:
(define_expand "reduc_<reduc_op>_scal_<mode>"
[(set (match_operand:<SCALAR_MODE> 0 "register_operand")
(unspec:<SCALAR_MODE>
[(match_operand:V_ALL 1 "register_operand")]
REDUC_UNSPEC))]
- ""
+ "!TARGET_RDNA2" [later '!TARGET_RDNA2_PLUS']
{
[...]
This conditional, however, does *not* govern any explicit
'gen_reduc_plus_scal_<mode>', and therefore Tobias in
commit 7cc2262ec9a410dc56d1c1c6b950c922e14f621d
"gcn/gcn-valu.md: Disable fold_left_plus for TARGET_RDNA2_PLUS [PR113615]"
had to replicate the '!TARGET_RDNA2_PLUS' condition:
@@ -4274,7 +4274,8 @@ (define_expand "fold_left_plus_<mode>"
[(match_operand:<SCALAR_MODE> 0 "register_operand")
(match_operand:<SCALAR_MODE> 1 "gcn_alu_operand")
(match_operand:V_FP 2 "gcn_alu_operand")]
- "can_create_pseudo_p ()
+ "!TARGET_RDNA2_PLUS
+ && can_create_pseudo_p ()
&& (flag_openacc || flag_openmp
|| flag_associative_math)"
{
| rtx dest = operands[0];
| rtx scalar = operands[1];
| rtx vector = operands[2];
| rtx tmp = gen_reg_rtx (<SCALAR_MODE>mode);
|
| emit_insn (gen_reduc_plus_scal_<mode> (tmp, vector));
| [...]
..., and I thus now have to do similar for
'gen_reduc_<expander>_scal_<mode>' use in here:
(define_expand "reduc_<fexpander>_scal_<mode>"
[(match_operand:<SCALAR_MODE> 0 "register_operand")
(fminmaxop:V_FP
(match_operand:V_FP 1 "register_operand"))]
- ""
+ "!TARGET_RDNA2_PLUS"
{
/* fmin/fmax are identical to smin/smax. */
emit_insn (gen_reduc_<expander>_scal_<mode> (operands[0],
operands[1]));
[...]
OK. I don't mind the asserts. Hopefully they're redundant, but I suppose
it's better than tracking down an unrecognised instruction in a later pass.
Andrew
