[v1] RISC-V: Let fcvt.* recognize rounding mode != 0

  For those floating point convert instructions that convert from lower
precisions to higher precisions, although the rounding mode has no
effects, the spec doesn’t forbid them to be values other than zero.

fcvt.q.l and fcvt.q.lu were fixed by 1d1595b. This patch also adds
test for them.

gas/ChangeLog:

	* testsuite/gas/riscv/fp-zfh-insns.d: New test for rounding mode.
	* testsuite/gas/riscv/fp-zfh-insns.s: Likewise.
	* testsuite/gas/riscv/zdinx.d: Likewise.
	* testsuite/gas/riscv/zdinx.s: Likewise.
	* testsuite/gas/riscv/zfbfmin.d: Likewise.
	* testsuite/gas/riscv/zfbfmin.s: Likewise.
	* testsuite/gas/riscv/zqinx.d: Likewise.
	* testsuite/gas/riscv/zqinx.s: Likewise.

opcode/ChangeLog:
	* riscv-opc.c (riscv_opcodes): Add opcodes for fcvt.* with
	rounding mode.

---
 gas/testsuite/gas/riscv/fp-zfh-insns.d |  3 +++
 gas/testsuite/gas/riscv/fp-zfh-insns.s |  3 +++
 gas/testsuite/gas/riscv/zdinx.d        |  2 ++
 gas/testsuite/gas/riscv/zdinx.s        |  2 ++
 gas/testsuite/gas/riscv/zfbfmin.d      |  1 +
 gas/testsuite/gas/riscv/zfbfmin.s      |  1 +
 gas/testsuite/gas/riscv/zqinx.d        |  6 ++++++
 gas/testsuite/gas/riscv/zqinx.s        |  6 ++++++
 opcodes/riscv-opc.c                    | 11 +++++++++++
 9 files changed, 35 insertions(+)
  

On 15.10.2024 11:30, Hau Hsu wrote:
> For those floating point convert instructions that convert from lower
> precisions to higher precisions, although the rounding mode has no
> effects, the spec doesn’t forbid them to be values other than zero.

Yet what's the point of permitting the use of misleading operands? In the
commit you refer to below I specifically said that the non-standard 3-
operand forms are left merely to avoid breaking existing code. At some
point I think use of them should actually be warned about, to allow their
removal at a yet later point.

Or is this just for the sake of disassembly, when some of the RM bits are
set? In that cases I'm inclined to request that measures be taken to avoid
their use when assembling code.

> fcvt.q.l and fcvt.q.lu were fixed by 1d1595b. This patch also adds
> test for them.

You add tests for the 3-operand forms, whereas the mentioned commit fixed
the 2-operand ones.

Jan
  

On Tue, Oct 15, 2024 at 8:42 PM Jan Beulich <jbeulich@suse.com> wrote:

> On 15.10.2024 11:30, Hau Hsu wrote:
> > For those floating point convert instructions that convert from lower
> > precisions to higher precisions, although the rounding mode has no
> > effects, the spec doesn’t forbid them to be values other than zero.
>
> Yet what's the point of permitting the use of misleading operands? In the
> commit you refer to below I specifically said that the non-standard 3-
> operand forms are left merely to avoid breaking existing code. At some
> point I think use of them should actually be warned about, to allow their
> removal at a yet later point.
>

Hey Jan,

Personally, I agree and support that permitting the use of misleading
operands looks weird, even though the random test generator people are
always not happy about it.  Anway, I found this old discussion,
https://github.com/riscv/riscv-isa-manual/issues/603.  So...

Hey Andrew,

Seems that this patch is trying to do the things that the above link
mentioned - allow the unaffected rm field of fp widening conversion
instructions.  Is this still valid now?  If that is so, then not only
instructions in this patch need to be fixed.

Thanks
Nelson
  

On 10/15/24 9:17 AM, Nelson Chu wrote:
> 
> 
> On Tue, Oct 15, 2024 at 8:42 PM Jan Beulich <jbeulich@suse.com 
> <mailto:jbeulich@suse.com>> wrote:
> 
>     On 15.10.2024 11:30, Hau Hsu wrote:
>      > For those floating point convert instructions that convert from lower
>      > precisions to higher precisions, although the rounding mode has no
>      > effects, the spec doesn’t forbid them to be values other than zero.
> 
>     Yet what's the point of permitting the use of misleading operands?
>     In the
>     commit you refer to below I specifically said that the non-standard 3-
>     operand forms are left merely to avoid breaking existing code. At some
>     point I think use of them should actually be warned about, to allow
>     their
>     removal at a yet later point.
> 
> 
> Hey Jan,
> 
> Personally, I agree and support that permitting the use of misleading 
> operands looks weird, even though the random test generator people are 
> always not happy about it.  Anway, I found this old discussion, https:// 
> github.com/riscv/riscv-isa-manual/issues/603 <https://github.com/riscv/ 
> riscv-isa-manual/issues/603>.  So...
Funny, I've got a complaint from our DV folks for the same issue.  I've 
punted it so far, but I don't think they're particularly happy about it.

jeff
  

On Tue, 15 Oct 2024 08:24:25 PDT (-0700), jeffreyalaw@gmail.com wrote:
>
>
> On 10/15/24 9:17 AM, Nelson Chu wrote:
>>
>>
>> On Tue, Oct 15, 2024 at 8:42 PM Jan Beulich <jbeulich@suse.com
>> <mailto:jbeulich@suse.com>> wrote:
>>
>>     On 15.10.2024 11:30, Hau Hsu wrote:
>>      > For those floating point convert instructions that convert from lower
>>      > precisions to higher precisions, although the rounding mode has no
>>      > effects, the spec doesn’t forbid them to be values other than zero.
>>
>>     Yet what's the point of permitting the use of misleading operands?
>>     In the
>>     commit you refer to below I specifically said that the non-standard 3-
>>     operand forms are left merely to avoid breaking existing code. At some
>>     point I think use of them should actually be warned about, to allow
>>     their
>>     removal at a yet later point.
>>
>>
>> Hey Jan,
>>
>> Personally, I agree and support that permitting the use of misleading
>> operands looks weird, even though the random test generator people are
>> always not happy about it.  Anway, I found this old discussion, https://
>> github.com/riscv/riscv-isa-manual/issues/603 <https://github.com/riscv/
>> riscv-isa-manual/issues/603>.  So...
> Funny, I've got a complaint from our DV folks for the same issue.  I've
> punted it so far, but I don't think they're particularly happy about it.

I thought we just told them to use `.insn` for this sort of stuff?  Did 
that not work out?
  

On 10/15/24 9:56 AM, Palmer Dabbelt wrote:
> On Tue, 15 Oct 2024 08:24:25 PDT (-0700), jeffreyalaw@gmail.com wrote:
>>
>>
>> On 10/15/24 9:17 AM, Nelson Chu wrote:
>>>
>>>
>>> On Tue, Oct 15, 2024 at 8:42 PM Jan Beulich <jbeulich@suse.com
>>> <mailto:jbeulich@suse.com>> wrote:
>>>
>>>     On 15.10.2024 11:30, Hau Hsu wrote:
>>>      > For those floating point convert instructions that convert 
>>> from lower
>>>      > precisions to higher precisions, although the rounding mode 
>>> has no
>>>      > effects, the spec doesn’t forbid them to be values other than 
>>> zero.
>>>
>>>     Yet what's the point of permitting the use of misleading operands?
>>>     In the
>>>     commit you refer to below I specifically said that the non- 
>>> standard 3-
>>>     operand forms are left merely to avoid breaking existing code. At 
>>> some
>>>     point I think use of them should actually be warned about, to allow
>>>     their
>>>     removal at a yet later point.
>>>
>>>
>>> Hey Jan,
>>>
>>> Personally, I agree and support that permitting the use of misleading
>>> operands looks weird, even though the random test generator people are
>>> always not happy about it.  Anway, I found this old discussion, https://
>>> github.com/riscv/riscv-isa-manual/issues/603 <https://github.com/riscv/
>>> riscv-isa-manual/issues/603>.  So...
>> Funny, I've got a complaint from our DV folks for the same issue.  I've
>> punted it so far, but I don't think they're particularly happy about it.
> 
> I thought we just told them to use `.insn` for this sort of stuff?  Did 
> that not work out?
I think that's what they're using.  But I don't think they're 
particularly happy about it.

jeff
  

On Tue, Oct 15, 2024 at 8:17 AM Nelson Chu <nelson@rivosinc.com> wrote:
>
>
>
> On Tue, Oct 15, 2024 at 8:42 PM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 15.10.2024 11:30, Hau Hsu wrote:
>> > For those floating point convert instructions that convert from lower
>> > precisions to higher precisions, although the rounding mode has no
>> > effects, the spec doesn’t forbid them to be values other than zero.
>>
>> Yet what's the point of permitting the use of misleading operands? In the
>> commit you refer to below I specifically said that the non-standard 3-
>> operand forms are left merely to avoid breaking existing code. At some
>> point I think use of them should actually be warned about, to allow their
>> removal at a yet later point.
>
>
> Hey Jan,
>
> Personally, I agree and support that permitting the use of misleading operands looks weird, even though the random test generator people are always not happy about it.  Anway, I found this old discussion, https://github.com/riscv/riscv-isa-manual/issues/603.  So...
>
> Hey Andrew,
>
> Seems that this patch is trying to do the things that the above link mentioned - allow the unaffected rm field of fp widening conversion instructions.  Is this still valid now?  If that is so, then not only instructions in this patch need to be fixed.

Using a nonzero rm field is definitely legal at the ISA level.

I don't have a strong opinion on this one.  It's benign to allow
these, but they're only useful for DV, not application software.  If
that is (or isn't) deemed to be a good enough reason, fine by me, but
we should stay philosophically consistent going forward.

>
> Thanks
> Nelson
  

More context:
Previously LLVM generated instructions to use DYN rounding mode for fcvt.s.bf16.
We got errors when using gas to assemble the code generated by LLVM.
The LLVM codegen is mysterious and Craig had a PR to fix it:
https://github.com/llvm/llvm-project/pull/106948/commits/90aa61e0dbbf2442dfd49bb645c31f7debf0f4e0 
But still he reported this issue and I had this patch.

Hau






> On Oct 16, 2024, at 4:24 AM, Andrew Waterman <andrew@sifive.com> wrote:
> 
> On Tue, Oct 15, 2024 at 8:17 AM Nelson Chu <nelson@rivosinc.com <mailto:nelson@rivosinc.com>> wrote:
>> 
>> 
>> 
>> On Tue, Oct 15, 2024 at 8:42 PM Jan Beulich <jbeulich@suse.com> wrote:
>>> 
>>> On 15.10.2024 11:30, Hau Hsu wrote:
>>>> For those floating point convert instructions that convert from lower
>>>> precisions to higher precisions, although the rounding mode has no
>>>> effects, the spec doesn’t forbid them to be values other than zero.
>>> 
>>> Yet what's the point of permitting the use of misleading operands? In the
>>> commit you refer to below I specifically said that the non-standard 3-
>>> operand forms are left merely to avoid breaking existing code. At some
>>> point I think use of them should actually be warned about, to allow their
>>> removal at a yet later point.
>> 
>> 
>> Hey Jan,
>> 
>> Personally, I agree and support that permitting the use of misleading operands looks weird, even though the random test generator people are always not happy about it.  Anway, I found this old discussion, https://github.com/riscv/riscv-isa-manual/issues/603.  So...
>> 
>> Hey Andrew,
>> 
>> Seems that this patch is trying to do the things that the above link mentioned - allow the unaffected rm field of fp widening conversion instructions.  Is this still valid now?  If that is so, then not only instructions in this patch need to be fixed.
> 
> Using a nonzero rm field is definitely legal at the ISA level.
> 
> I don't have a strong opinion on this one.  It's benign to allow
> these, but they're only useful for DV, not application software.  If
> that is (or isn't) deemed to be a good enough reason, fine by me, but
> we should stay philosophically consistent going forward.
> 
>> 
>> Thanks
>> Nelson
  

Here's an older related LLVM patch from Alex Bradbury
https://github.com/llvm/llvm-project/pull/67555 for fcvt.d.s and other
instructions.

On Tue, Oct 15, 2024 at 7:06 PM Hau Hsu <hau.hsu@sifive.com> wrote:

> More context:
> Previously LLVM generated instructions to use DYN rounding mode for
> fcvt.s.bf16.
> We got errors when using gas to assemble the code generated by LLVM.
> The LLVM codegen is mysterious and Craig had a PR to fix it:
>
> https://github.com/llvm/llvm-project/pull/106948/commits/90aa61e0dbbf2442dfd49bb645c31f7debf0f4e0
>
> But still he reported this issue and I had this patch.
>
> Hau
>
>
>
>
>
>
> On Oct 16, 2024, at 4:24 AM, Andrew Waterman <andrew@sifive.com> wrote:
>
> On Tue, Oct 15, 2024 at 8:17 AM Nelson Chu <nelson@rivosinc.com> wrote:
>
>
>
>
> On Tue, Oct 15, 2024 at 8:42 PM Jan Beulich <jbeulich@suse.com> wrote:
>
>
> On 15.10.2024 11:30, Hau Hsu wrote:
>
> For those floating point convert instructions that convert from lower
> precisions to higher precisions, although the rounding mode has no
> effects, the spec doesn’t forbid them to be values other than zero.
>
>
> Yet what's the point of permitting the use of misleading operands? In the
> commit you refer to below I specifically said that the non-standard 3-
> operand forms are left merely to avoid breaking existing code. At some
> point I think use of them should actually be warned about, to allow their
> removal at a yet later point.
>
>
>
> Hey Jan,
>
> Personally, I agree and support that permitting the use of misleading
> operands looks weird, even though the random test generator people are
> always not happy about it.  Anway, I found this old discussion,
> https://github.com/riscv/riscv-isa-manual/issues/603.  So...
>
> Hey Andrew,
>
> Seems that this patch is trying to do the things that the above link
> mentioned - allow the unaffected rm field of fp widening conversion
> instructions.  Is this still valid now?  If that is so, then not only
> instructions in this patch need to be fixed.
>
>
> Using a nonzero rm field is definitely legal at the ISA level.
>
> I don't have a strong opinion on this one.  It's benign to allow
> these, but they're only useful for DV, not application software.  If
> that is (or isn't) deemed to be a good enough reason, fine by me, but
> we should stay philosophically consistent going forward.
>
>
> Thanks
> Nelson
>
>
>