RISC-V: Fix riscv_modes_tieable_p

  Integer values and floating-point values need to be converted
by fmv series instructions. So if mode1 is MODE_INT and mode2
is MODE_FLOAT, we should return false in riscv_modes_tieable_p,
and vice versa.

gcc/ChangeLog:

        * config/riscv/riscv.cc (riscv_modes_tieable_p):

gcc/testsuite/ChangeLog:

        * gcc.target/riscv/fwprop1-modes-tieable.c: New test.
---
 gcc/config/riscv/riscv.cc                     |  5 ++
 .../gcc.target/riscv/fwprop1-modes-tieable.c  | 80 +++++++++++++++++++
 2 files changed, 85 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/riscv/fwprop1-modes-tieable.c

--
2.25.1


This message and any attachment are confidential and may be privileged or otherwise protected from disclosure. If you are not an intended recipient of this message, please delete it and any attachment from your system and notify the sender immediately by reply e-mail. Unintended recipients should not use, copy, disclose or take any action based on this message or any information contained in this message. Emails cannot be guaranteed to be secure or error free as they can be intercepted, amended, lost or destroyed, and you should take full responsibility for security checking. 
 
本邮件及其任何附件具有保密性质，并可能受其他保护或不允许被披露给第三方。如阁下误收到本邮件，敬请立即以回复电子邮件的方式通知发件人，并将本邮件及其任何附件从阁下系统中予以删除。如阁下并非本邮件写明之收件人，敬请切勿使用、复制、披露本邮件或其任何内容，亦请切勿依本邮件或其任何内容而采取任何行动。电子邮件无法保证是一种安全和不会出现任何差错的通信方式，可能会被拦截、修改、丢失或损坏，收件人需自行负责做好安全检查。
  

> Integer values and floating-point values need to be converted
> by fmv series instructions. So if mode1 is MODE_INT and mode2
> is MODE_FLOAT, we should return false in riscv_modes_tieable_p,
> and vice versa.

I think that's on purpose because we can read and write float values
from/to integer registers.  Maybe it's a cost problem that we spill
at some point rather than access directly?

If I compile your test case I do see converting moves in the final
assembly - is there something you're concerned about in particular?
  

On 1/10/25 12:11 PM, Robin Dapp wrote:
>> Integer values and floating-point values need to be converted
>> by fmv series instructions. So if mode1 is MODE_INT and mode2
>> is MODE_FLOAT, we should return false in riscv_modes_tieable_p,
>> and vice versa.
> 
> I think that's on purpose because we can read and write float values
> from/to integer registers.  Maybe it's a cost problem that we spill
> at some point rather than access directly?
But even if you spill, as long as loads/stores don't modify the value 
then I think we're OK from a correctness standpoint.


> 
> If I compile your test case I do see converting moves in the final
> assembly - is there something you're concerned about in particular?
Which was my general question as well.  Under precisely what 
circumstances is this causing a problem?  The secondary question would 
be how does this change interact with the finx and related extensions?

Jeff
  

On Fri, 10 Jan 2025 12:21:15 PST (-0800), jeffreyalaw@gmail.com wrote:
>
>
> On 1/10/25 12:11 PM, Robin Dapp wrote:
>>> Integer values and floating-point values need to be converted
>>> by fmv series instructions. So if mode1 is MODE_INT and mode2
>>> is MODE_FLOAT, we should return false in riscv_modes_tieable_p,
>>> and vice versa.
>>
>> I think that's on purpose because we can read and write float values
>> from/to integer registers.  Maybe it's a cost problem that we spill
>> at some point rather than access directly?
> But even if you spill, as long as loads/stores don't modify the value
> then I think we're OK from a correctness standpoint.
>
>
>>
>> If I compile your test case I do see converting moves in the final
>> assembly - is there something you're concerned about in particular?

Which appears to be the glibc code (or very similar to it), and I don't 
think we've had users reporting incorrect results there.

> Which was my general question as well.  Under precisely what
> circumstances is this causing a problem?  The secondary question would
> be how does this change interact with the finx and related extensions?

FWIW I'm also a bit lost here: I'd expect riscv_hard_regno_mode_ok() to 
be sufficient to handle these X/F register mixing cases, and thus us not 
to need any more special handling in riscv_modes_tieable_p().

(I think we're safe for finx with the current code, as we can access the 
registers safely there.)

So maybe there's something else also needed to trigger this?

>
> Jeff
  

On 1/10/25 4:59 PM, Palmer Dabbelt wrote:
> On Fri, 10 Jan 2025 12:21:15 PST (-0800), jeffreyalaw@gmail.com wrote:
>>
>>
>> On 1/10/25 12:11 PM, Robin Dapp wrote:
>>>> Integer values and floating-point values need to be converted
>>>> by fmv series instructions. So if mode1 is MODE_INT and mode2
>>>> is MODE_FLOAT, we should return false in riscv_modes_tieable_p,
>>>> and vice versa.
>>>
>>> I think that's on purpose because we can read and write float values
>>> from/to integer registers.  Maybe it's a cost problem that we spill
>>> at some point rather than access directly?
>> But even if you spill, as long as loads/stores don't modify the value
>> then I think we're OK from a correctness standpoint.
>>
>>
>>>
>>> If I compile your test case I do see converting moves in the final
>>> assembly - is there something you're concerned about in particular?
> 
> Which appears to be the glibc code (or very similar to it), and I don't 
> think we've had users reporting incorrect results there.
There's certainly cases in glibc that very much want to just move a blob 
of data from an FP register into a GPR without any kind of interpretation.

> 
>> Which was my general question as well.  Under precisely what
>> circumstances is this causing a problem?  The secondary question would
>> be how does this change interact with the finx and related extensions?
> 
> FWIW I'm also a bit lost here: I'd expect riscv_hard_regno_mode_ok() to 
> be sufficient to handle these X/F register mixing cases, and thus us not 
> to need any more special handling in riscv_modes_tieable_p().
> 
> (I think we're safe for finx with the current code, as we can access the 
> registers safely there.)
> 
> So maybe there's something else also needed to trigger this?
I don't think you're lost, Robin and I have similar concerns as yours. 
At this point I don't think the patch is right/correct, but I'm also 
open to the possibility there's something more complex going on that 
hasn't been fully explained.  Hence my request for an explanation of the 
precise circumstances when Zhijin thinks this is necessary.


Jeff