Re: [SH] Check FPSCR.PR for fldi0, fldi1 insns in simulator

[Simon Marchi <simark@simark.ca>]

On 3/11/26 2:20 AM, Oleg Endo wrote:
> On Wed, 2026-03-11 at 00:27 -0400, Simon Marchi wrote:
>>
>> On 2026-03-10 23:02, Oleg Endo wrote:
>>> Hi,
>>>
>>> On SH variants with double-precision FPU the insns fli0 and flid1 require
>>> that FPSCR.PR must be set to 0, i.e. single-precision mode.  The attached
>>> patch fixes that.
>>>
>>> OK to apply?
>>>
>>> Best regards,
>>> Oleg Endo
>>
>> I looked for an ISA manual [1] and checked it for fun.  For the FLID0,
>> it says:
>>
>>   When FPSCR.PR = 0, this instruction loads floating-point 0.0 (0x00000000) into FRn.
>>
>> Curiously, it does not say what happens if `FPSCR.PR = 1`.  However,
>> the description for FPSCR.PR says:
>>
>>   PR: Precision mode
>>   PR = 0: Floating-point instructions are executed as single-precision operations.
>>   PR = 1: Floating-point instructions are executed as double-precision operations (the result of
>>   instructions for which double-precision is not supported is undefined).
>>
>> I guess that FLID0 and FLID1 fall into that "is undefined" category?
>> Do you know what the real hardware does in this case?
> 
> I have not checked what exactly happens on real hardware.  I have checked a
> few ISA manuals (Renesas, ST).  The instruction descriptions doesn't say
> that FLDI0 / FLID1 raises an exception (when in the wrong mode).  So I guess
> it will produce garbage quietly.  Maybe some silicon implementations have
> actually an undocumented but well defined behavior.

Ok, well since we're in "undefined behavior" territory we could argue
that the current sim behavior is fine.  But I think the patch is still
good, traping is a more useful behavior to catch something you don't
want to happen.

> 
> QEMU traps in this case.  We found this while chasing an GCC bug.
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117182
> 
> I find using sh-sim for compiler testing quite useful.  Hence this patch.

Good to know, thanks.  We often wonder "does anybody still use some old
architecture or feature X in gdb".

>> The patch LGTM, but I am not a maintainer of the sim, so:
>>
>> Reviewed-By: Simon Marchi <simon.marchi@efficios.com>
>>
>> One note, please put the relevant information (the body of your email)
>> into the commit log itself).
> 
> Yeah, sure I can expand the comment of the commit.  Updated patch/commit
> attached.

I would give it a week for Andrew or Mike to review and approve, but
otherwise I would feel comfortable merging it.

Simon