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] Use zero register as argument for atomics #112693

Merged
merged 6 commits into from
Mar 10, 2025

Conversation

tomeksowi
Copy link
Contributor

A zero constant was unnecessarily synthesized, wasting a register.

Also fix a non-obvious bug in CompareExchange with unextended comparand register (see added test case).

Part of #84834, cc @dotnet/samsung

@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 Feb 19, 2025
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Feb 19, 2025
Copy link
Contributor

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

// Return Value:
// Whether the floating-point immediate can be synthesized with one instruction
//
static bool isSingleInstructionFpImm(double value, emitAttr size, int64_t* outBits)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@risc-vv
Copy link

risc-vv commented Feb 19, 2025

68730b3 is being scheduled for building and testing

GIT: 68730b307eb4aeba8c7f7afd045350ad4f2ffe95
REPO: dotnet/runtime
BRANCH: main

@lewing
Copy link
Member

lewing commented Feb 19, 2025

/azp run runtime

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@clamp03 clamp03 requested a review from jakobbotsch February 21, 2025 06:03
@jakobbotsch jakobbotsch closed this Mar 4, 2025
@jakobbotsch jakobbotsch reopened this Mar 4, 2025
@jakobbotsch
Copy link
Member

/azp run runtime

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jakobbotsch
Copy link
Member

/ba-g Failures are unrelated infra issues

@jakobbotsch jakobbotsch merged commit ff6474a into dotnet:main Mar 10, 2025
119 of 123 checks passed
// Return Value:
// Whether the floating-point immediate can be synthesized with one instruction
//
static bool isSingleInstructionFpImm(double value, emitAttr size, int64_t* outBits)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be more useful to return out some enum that indicates if we're doing, none, Simm12, or Simm20 instead of the bool?

There is significant coupling of the callers to this and them subsequently doing the same isValidSim12 and/or isValidSimm20 to emit the actual code. Might be really nice to return which and not test the bits there too...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about it but it complicated the signature of this simple helper, besides LSRA doesn't need to know which instruction will be used.

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