[Bug tdep/25489] New: RISC-V: NaN boxing not done for assignments to FPRs

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[Bug tdep/25489] New: RISC-V: NaN boxing not done for assignments to FPRs

glaubitz at physik dot fu-berlin.de
https://sourceware.org/bugzilla/show_bug.cgi?id=25489

            Bug ID: 25489
           Summary: RISC-V: NaN boxing not done for assignments to FPRs
           Product: gdb
           Version: HEAD
            Status: NEW
          Severity: normal
          Priority: P2
         Component: tdep
          Assignee: unassigned at sourceware dot org
          Reporter: [hidden email]
  Target Milestone: ---
            Target: riscv64-*-*

Observed as a failure in testing:

(gdb) FAIL: gdb.base/return-nodebug.exp: float: full width of the returned
result

The culprit is GDB fails to NaN-box floating-point data of the `float'
type when assigning it to a hardware register.  Storing an unboxed
datum into an FPR causes hardware to consider it a canonican NaN, e.g.
in this excerpt from `info float':

fa0            {float = -1,
  double = 1.5873523201947252e-314}     (raw 0x00000000bf800000)
fa1            {float = -1,
  double = -nan(0xfffffbf800000)}       (raw 0xffffffffbf800000)

we have a correctly boxed `-1.0f' value in $fa1, however the value in
in $fa0 is not boxed (i.e. the upper bits are not all-ones), having
been set by GDB rather than hardware (as with e.g. the FLW instruction),
in this case by requesting a function return with:

(gdb) return (float) -1

Consequently when an attempt is made to transfer the value to another
register with a hardware instruction, such as:

        fmv.s   fa5,fa0

as in the test case considered, a NaN is instead produced:

fa5            {float = nan(0x400000),
  double = -nan(0xfffff7fc00000)}       (raw 0xffffffff7fc00000)

which causes the failure reported.

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug tdep/25489] RISC-V: NaN boxing not done for assignments to FPRs

glaubitz at physik dot fu-berlin.de
https://sourceware.org/bugzilla/show_bug.cgi?id=25489

Andrew Burgess <andrew.burgess at embecosm dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |andrew.burgess at embecosm dot com
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at sourceware dot org   |andrew.burgess at embecosm dot com

--- Comment #1 from Andrew Burgess <andrew.burgess at embecosm dot com> ---
Created attachment 12373
  --> https://sourceware.org/bugzilla/attachment.cgi?id=12373&action=edit
Possible patch for this issue.

Attached patch should fix this issue.  I just need to run my local tests to
make sure I've not regressed anything then I'll post it to the mailing list.

Feel free to give it a test ahead of time and confirm this fixes your issue.

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug tdep/25489] RISC-V: NaN boxing not done for assignments to FPRs

glaubitz at physik dot fu-berlin.de
In reply to this post by glaubitz at physik dot fu-berlin.de
https://sourceware.org/bugzilla/show_bug.cgi?id=25489

Jim Wilson <wilson at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |wilson at gcc dot gnu.org

--- Comment #2 from Jim Wilson <wilson at gcc dot gnu.org> ---
I tried testing it, and hit the new assert you added.  This is hit about 198
times when running the gdb testsuite.

p/d check_arg_struct_01_01 (ref_val_struct_01_01)
../../binutils-gdb/gdb/riscv-tdep.c:2556: internal-error: CORE_ADDR
riscv_push_\
dummy_call(gdbarch*, value*, regcache*, CORE_ADDR, int, value**, CORE_ADDR,
fun\
ction_call_return_method, CORE_ADDR): Assertion `info->argloc[0].c_offset ==
0'\
 failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n) FAIL:
gdb.base/infcall-nested-structs.exp\
: l=c++: types-tfc: p/d check_arg_struct_01_01 (ref_val_struct_01_01) (GDB
inte\
rnal error)

I haven't tried to debug this yet.

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug tdep/25489] RISC-V: NaN boxing not done for assignments to FPRs

glaubitz at physik dot fu-berlin.de
In reply to this post by glaubitz at physik dot fu-berlin.de
https://sourceware.org/bugzilla/show_bug.cgi?id=25489

--- Comment #3 from Andrew Burgess <andrew.burgess at embecosm dot com> ---
Thanks.  I had a few problems with my testing infrastructure, so I posted the
original patch with only minimal testing.  Hopefully I've got my local problems
fixed, and I'll update the patch.

I suspect the assertion will probably end up removed and some comments will be
updated - the assertion was based on a comment at the declaration of the
c_offset field, that the offset for the first piece was always 0, but I'm
guessing this is not always the case, and I'll just need to revise the comment.

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug tdep/25489] RISC-V: NaN boxing not done for assignments to FPRs

glaubitz at physik dot fu-berlin.de
In reply to this post by glaubitz at physik dot fu-berlin.de
https://sourceware.org/bugzilla/show_bug.cgi?id=25489

Andrew Burgess <andrew.burgess at embecosm dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #12373|0                           |1
        is obsolete|                            |

--- Comment #4 from Andrew Burgess <andrew.burgess at embecosm dot com> ---
Created attachment 12375
  --> https://sourceware.org/bugzilla/attachment.cgi?id=12375&action=edit
Updated patch.

Revised patch, the assertion is now removed, and I updated the comment that
originally motivated me to add the assertion.  Still needs full testing at my
end before I can post to the mailing list, but hopefully I'll get that done
over the weekend.

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug tdep/25489] RISC-V: NaN boxing not done for assignments to FPRs

glaubitz at physik dot fu-berlin.de
In reply to this post by glaubitz at physik dot fu-berlin.de
https://sourceware.org/bugzilla/show_bug.cgi?id=25489

--- Comment #5 from Jim Wilson <wilson at gcc dot gnu.org> ---
The latest patch works for me.  It fixes the return-nodebug failure, and the
small number of other differences look like the usual unrelated stability
problems.

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug tdep/25489] RISC-V: NaN boxing not done for assignments to FPRs

Sourceware - gdb-prs mailing list
In reply to this post by glaubitz at physik dot fu-berlin.de
https://sourceware.org/bugzilla/show_bug.cgi?id=25489

--- Comment #6 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Andrew Burgess <[hidden email]>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=dd8953924b0966e363c27ee38a0663c08f742fa0

commit dd8953924b0966e363c27ee38a0663c08f742fa0
Author: Andrew Burgess <[hidden email]>
Date:   Fri Mar 13 15:50:28 2020 +0000

    gdb/riscv: Apply NaN boxing when writing return values into registers

    When setting up function parameters we already perform NaN boxing, as
    required by the RISC-V ABI, however, we don't do this when writing
    values into registers as part of setting up a return value.

    This commit moves the NaN boxing code into a small helper function,
    and then makes use of this function when setting up function
    parameters, and also when setting up return values.

    This should resolve this failure:

      FAIL: gdb.base/return-nodebug.exp: float: full width of the returned
result

    gdb/ChangeLog:

            PR gdb/25489
            * riscv-tdep.c (riscv_arg_info::c_offset): Update comment.
            (riscv_regcache_cooked_write): New function.
            (riscv_push_dummy_call): Use new function.
            (riscv_return_value): Likewise.

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug tdep/25489] RISC-V: NaN boxing not done for assignments to FPRs

Sourceware - gdb-prs mailing list
In reply to this post by glaubitz at physik dot fu-berlin.de
https://sourceware.org/bugzilla/show_bug.cgi?id=25489

Andrew Burgess <andrew.burgess at embecosm dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #7 from Andrew Burgess <andrew.burgess at embecosm dot com> ---
This issue should now be resolved in current master branch.  Please feel free
to reopen if there are still problems in this area.

--
You are receiving this mail because:
You are on the CC list for the bug.