Skip to content

[RISC-V] Add pseudo instruction not and replace xori when immediate is -1 #117967

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

Merged
merged 3 commits into from
Aug 4, 2025

Conversation

credo-quia-absurdum
Copy link
Contributor

Add pseudo instruction not and replace xori when immediate is -1

As this is my first attempt at adding a new instruction, I might have overlooked certain parts (such as parts of the emitter implementation or validation in debug configuration). Any review or suggestion would be greatly appreciated.

@clamp03 @tomeksowi @SkyShield, @namu-lee
part of #84834, cc @dotnet/samsung

@Copilot Copilot AI review requested due to automatic review settings July 23, 2025 04:11
@github-actions github-actions bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jul 23, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a new pseudo instruction not for RISC-V64 architecture to replace the previous approach of using xori with immediate value -1 for bitwise NOT operations. This provides clearer semantics and potentially better code generation for bitwise NOT operations.

  • Adds not pseudo instruction definition with appropriate encoding
  • Updates emitter to handle the new instruction alongside existing register-to-register operations
  • Replaces xori reg, reg, -1 patterns with the more semantically clear not reg, reg instruction

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/coreclr/jit/instrsriscv64.h Adds the not pseudo instruction definition with encoding 0xFFF04013
src/coreclr/jit/emitriscv64.cpp Updates emitter condition to include INS_not in register-to-register instruction handling
src/coreclr/jit/codegenriscv64.cpp Replaces two instances of xori with immediate -1 with the new not instruction

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jul 23, 2025
Copy link
Contributor

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

@clamp03 clamp03 added the arch-riscv Related to the RISC-V architecture label Jul 23, 2025
@credo-quia-absurdum
Copy link
Contributor Author

@credo-quia-absurdum please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@dotnet-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@dotnet-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@dotnet-policy-service agree company="Microsoft"

Contributor License Agreement

@dotnet-policy-service agree

@credo-quia-absurdum
Copy link
Contributor Author

@sirntar

It seems that the risc-vv CI test hasn't been triggered on this PR, unlike previous ones. Could you kindly check if there might be a configuration issue?

@sirntar
Copy link
Member

sirntar commented Jul 25, 2025

@risc-vv /run

@risc-vv
Copy link

risc-vv commented Jul 25, 2025

RISC-V Release-CLR-QEMU: 9092 / 9122 (99.67%)
=======================
      passed: 9092
      failed: 2
     skipped: 596
      killed: 28
------------------------
 TOTAL tests: 9718
VIRTUAL time: 37h 33min 28s 523ms
   REAL time: 38min 17s 617ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst

RISC-V Release-CLR-VF2: 9093 / 9123 (99.67%)
=======================
      passed: 9093
      failed: 2
     skipped: 596
      killed: 28
------------------------
 TOTAL tests: 9719
VIRTUAL time: 11h 55min 4s 974ms
   REAL time: 48min 9s 970ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst

RISC-V Release-FX-QEMU: 276936 / 278063 (99.59%)
=======================
      passed: 276936
      failed: 1119
     skipped: 39
      killed: 8
------------------------
 TOTAL tests: 278102
VIRTUAL time: 31h 30min 25s 102ms
   REAL time: 1h 11min 46s 699ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst

RISC-V Release-FX-VF2: 301390 / 303141 (99.42%)
=======================
      passed: 301390
      failed: 1742
     skipped: 39
      killed: 9
------------------------
 TOTAL tests: 303180
VIRTUAL time: 21h 28min 46s 466ms
   REAL time: 2h 18min 2s 183ms
=======================

report.xml, report.md, failures.xml, testclr_details.tar.zst

Build information and commands

GIT: 0285c4a08a975029e2eb246ec91261fbc8695694
CI: 785da59dadeb491bca87651db4b40a68883f8a00
REPO: dotnet/runtime
BRANCH: main
CONFIG: Release
LIB_CONFIG: Release

@sirntar
Copy link
Member

sirntar commented Jul 25, 2025

@credo-quia-absurdum This wasn't a problem, rather a new feature introduced last week ;)
Now, instead of automatically running at every commit that might be rv64-related, the CI bot can only be scheduled by the @risc-vv /<command> <args> command (the command must be run by a user with public membership in dotnet org). Bot reads only comments in PRs, that are marked as being rv64-related (or have [RISC-V] in title).

@am11
Copy link
Member

am11 commented Jul 25, 2025

@sirntar thanks for making the bot on-demand. A few possible future enhancements:

@sirntar
Copy link
Member

sirntar commented Jul 25, 2025

@am11

/run jitdiffs to run the diffs which folks are running locally and showing posting results manually

I'm currently testing this feature, so I expect it to be launched next week

/run aot to run smoke AOT tests

Currently CI doesn't perform atoi tests at all... will add them in the future
We are also planning to add /run perf for dotnet/performance tests to benchmark commited code

@jakobbotsch
Copy link
Member

/azp run runtime

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jakobbotsch jakobbotsch merged commit e7e75c5 into dotnet:main Aug 4, 2025
109 of 113 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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