On 2/18/25 4:12 AM, Jin Ma wrote:
We overlooked the side effects of the rounding mode in the pattern,
which can impact the result of float_extend and lead to incorrect
optimizations in the final program. This issue likely affects nearly
all similar patterns that involve rounding modes, and the tests in
this patch only highlight one example. It seems challenging to address,
and I only implemented a simple fix, which is not a good way to solve
the problem.
Any comments on this?
gcc/ChangeLog:
* config/riscv/vector-iterators.md (UNSPEC_VRM): New.
* config/riscv/vector.md: Use UNSPEC for float_extend.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/rvv/base/bug-11.c: New test.
So as Kito note, the insn you changed already has a reference to the FRM
it needs -- kept in operands[9]. It seems like your patch, while fixing
the bug, more likely does so by accident rather than by design.
What I see when I look at the dump files is a deeper issue.
In the .expand dump we have:
(insn 17 16 18 2 (set (reg:HF 147)
(const_double:HF 0.0 [0x0.0p+0])) "j.c":14:24 -1
(nil))
(insn 18 17 19 2 (set (reg/v:RVVMF2SF 141 [ vreg ])
(if_then_else:RVVMF2SF (unspec:RVVMF64BI [
(reg/v:RVVMF64BI 138 [ vmask ])
(const_int 1 [0x1])
(const_int 0 [0])
(const_int 2 [0x2])
(const_int 0 [0])
(const_int 2 [0x2])
(reg:SI 66 vl)
(reg:SI 67 vtype)
(reg:SI 69 frm)
] UNSPEC_VPREDICATE)
(minus:RVVMF2SF (reg/v:RVVMF2SF 140 [ vreg_memory ])
(float_extend:RVVMF2SF (vec_duplicate:RVVMF4HF (reg:HF 147))))
(reg/v:RVVMF2SF 140 [ vreg_memory ]))) "j.c":14:24 -1
(nil))
Insn 18 does the subtraction with the adjusted rounding mode. So far,
so good. Things look fine at the start of cse1. But if we look at the
end of cse1 we have:
(insn 17 16 18 2 (set (reg:HF 147)
(const_double:HF 0.0 [0x0.0p+0])) "j.c":14:24 136 {*movhf_hardfloat}
(nil))
(insn 18 17 19 2 (set (reg/v:RVVMF2SF 141 [ vreg ])
(reg/v:RVVMF2SF 140 [ vreg_memory ])) "j.c":14:24 2786
{*movrvvmf2sf_fract}
(expr_list:REG_DEAD (reg:HF 147)
(expr_list:REG_DEAD (reg/v:RVVMF2SF 140 [ vreg_memory ])
(expr_list:REG_DEAD (reg/v:RVVMF64BI 138 [ vmask ])
(expr_list:REG_DEAD (reg:SI 69 frm)
(nil))))))
Note how CSE replace the arithmetic with a simple copy. At this point
things are broken.
I don't see how CSE can make the right decision here; we don't expose
rounding modes this early and thus CSE has no way to know it can't make
that kind of replacement.
You patch kindof works, but it seems to me it's more accident than
design and that we need to fix this in a more general manner.
The natural question is what do other targets do when the rounding mode
gets changed. I'm guessing its exposed as a unspec set before the RTL
optimizers run.
jeff
