Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RISC-V] fix rm field in riscvd instruction #101908

Merged
merged 1 commit into from
May 6, 2024

Conversation

SzpejnaDawid
Copy link
Contributor

@SzpejnaDawid SzpejnaDawid commented May 6, 2024

When i was working on #101683 I found that R2RDump prints Decode Failure for those instructions:

  • fcvt.d.w
  • fcvt.d.wu
  • fcvt.d.s
    , when i ran R2RDump e.g. for System.Private.CoreLib.dll. I checked it is printed by libcoredistools because LLVM MC cannot decode instructions properly. The problem is with rm field. According to The RISC-V Instruction Set Manual

software should set their rm field to RNE (000)

and when i changed this field to 0 it fixed the problem. Below instructions also set rm to 1

  • fcvt.w.d
  • fcvt.wu.d
  • fcvt.s.d

I checked my changes running tests for coreclr with priority 1 and i didn't get any new error.

Part of #84834, cc @dotnet/samsung

Changed rm from 1 to 0.
Fixed instructions:
- fcvt.d.w
- fcvt.w.d
- fcvt.d.wu
- fcvt.wu.d
- fcvt.d.s
- fcvt.s.d
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label May 6, 2024
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label May 6, 2024
Copy link
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

@am11 am11 added the arch-riscv Related to the RISC-V architecture label May 6, 2024
@tomeksowi
Copy link
Contributor

Nit: s/riscvd/fcvt in title :)

@jkotas jkotas merged commit bbbb686 into dotnet:main May 6, 2024
107 checks passed
@tomeksowi
Copy link
Contributor

Congratulations on landing your first pull request!

@clamp03
Copy link
Member

clamp03 commented May 7, 2024

Thank you and congratulations!!! 🎆

michaelgsharp pushed a commit to michaelgsharp/runtime that referenced this pull request May 9, 2024
Changed rm from 1 to 0.
Fixed instructions:
- fcvt.d.w
- fcvt.w.d
- fcvt.d.wu
- fcvt.wu.d
- fcvt.d.s
- fcvt.s.d
Ruihan-Yin pushed a commit to Ruihan-Yin/runtime that referenced this pull request May 30, 2024
Changed rm from 1 to 0.
Fixed instructions:
- fcvt.d.w
- fcvt.w.d
- fcvt.d.wu
- fcvt.wu.d
- fcvt.d.s
- fcvt.s.d
@github-actions github-actions bot locked and limited conversation to collaborators Jun 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-riscv Related to the RISC-V architecture area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants