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

Fix the ARM's double register name displayed in JitDisasm/JitDump #79949

Merged
merged 2 commits into from
Dec 28, 2022

Conversation

kunalspathak
Copy link
Member

Fix the display of ARM double register name in JitDisasm and JitDump. We would just display the floating register name, but instead should be displaying the accurate double register name (that the encoding reflects).

Fixes: #8121

@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 Dec 24, 2022
@ghost ghost assigned kunalspathak Dec 24, 2022
@kunalspathak
Copy link
Member Author

@dotnet/jit-contrib

@ghost
Copy link

ghost commented Dec 24, 2022

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

Issue Details

Fix the display of ARM double register name in JitDisasm and JitDump. We would just display the floating register name, but instead should be displaying the accurate double register name (that the encoding reflects).

Fixes: #8121

Author: kunalspathak
Assignees: kunalspathak
Labels:

area-CodeGen-coreclr

Milestone: -

}
else
{
printf("s%s", emitFloatRegName(reg, attr) + 1);
Copy link
Member

Choose a reason for hiding this comment

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

Is this side also incorrect? I could easily have misread the chain of calls, but it looked like emitFloatRegName is going to return "f" so it would be "sf", which doesn't seem right.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, but since we do + 1, we remove the extra f character that gets printed. This portion is unchanged actually.

Copy link
Member Author

Choose a reason for hiding this comment

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

I will just use something manual version of _itoa to convert from int -> string.

Copy link
Member

@markples markples left a comment

Choose a reason for hiding this comment

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

Change looks good - there might be another change depending on whether my comment is correct or not, but this is an improvement either way.

@kunalspathak kunalspathak merged commit 2bd38ad into dotnet:main Dec 28, 2022
@kunalspathak kunalspathak deleted the arm-dbl-register branch December 28, 2022 05:18
@ghost ghost locked as resolved and limited conversation to collaborators Jan 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ARM32] Display wrong register number for double registers
3 participants