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] Implement some of the NYI methods in unwindriscv64 #95128

Merged
merged 17 commits into from
Nov 28, 2023

Conversation

sirntar
Copy link
Member

@sirntar sirntar commented Nov 22, 2023

This PR contains implementation for some of the NYI methods in unwindriscv64.

About Compiler::mapRegNumToDwarfReg:
I've mapped only basic and floating pointer registers as we are not supporting V-extension right now.

About GetUnwindSizeFromUnwindHeader:
I'm not 100% certain whether this is a correct implementation, so I would be grateful if someone can check it.


Part of #84834
cc @wscho77 @HJLeee @clamp03 @JongHeonChoi @t-mustafin @gbalykov @ashaurtaev @yurai007 @tomeksowi @Bajtazar

@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 Nov 22, 2023
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Nov 22, 2023
@ghost
Copy link

ghost commented Nov 22, 2023

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

Issue Details

This PR contains implementation for some of the NYI methods in unwindriscv64.

About Compiler::mapRegNumToDwarfReg:
I've mapped only basic and floating pointer registers as we are not supporting V-extension right now.

About GetUnwindSizeFromUnwindHeader:
I'm not 100% certain whether this is a correct implementation, so I would be grateful if someone can check it.


Part of #84834
cc @wscho77 @HJLeee @clamp03 @JongHeonChoi @t-mustafin @gbalykov @ashaurtaev @yurai007 @tomeksowi @Bajtazar

Author: sirntar
Assignees: -
Labels:

area-CodeGen-coreclr, community-contribution

Milestone: -

@sirntar

This comment was marked as off-topic.

src/coreclr/jit/unwindriscv64.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/unwindriscv64.cpp Outdated Show resolved Hide resolved
@clamp03 clamp03 added the arch-riscv Related to the RISC-V architecture label Nov 22, 2023
@clamp03
Copy link
Member

clamp03 commented Nov 22, 2023

Noti: @shushanhf I checked dwarf register numbers for other archs as well as riscv. I think loongarch64 DWARF register number for floating-point register starts from 32 according to https://github.com/loongson/la-abi-specs/blob/release/ladwarf.adoc#loongarch-specific-dwarf-definitions. Please take a look. :)

case REG_F0:
dwarfReg = 64;
break;

@sirntar
Copy link
Member Author

sirntar commented Nov 23, 2023

Noti: @shushanhf I checked dwarf register numbers for other archs as well as riscv. I think loongarch64 DWARF register number for floating-point register starts from 32 according to https://github.com/loongson/la-abi-specs/blob/release/ladwarf.adoc#loongarch-specific-dwarf-definitions. Please take a look. :)

case REG_F0:
dwarfReg = 64;
break;

I sent an email about it to @shushanhf last week. If I remember correctly, he responded that it could have been a part of SIMD functionality that hasn't been pushed to upstream. I also made a small fix for loongarch64 (sirntar@58edd0b), but since I don't have a loongarch64 platform to test it, I didn't create any pull request.

@clamp03
Copy link
Member

clamp03 commented Nov 23, 2023

I sent an email about it to @shushanhf last week. If I remember correctly, he responded that it could have been a part of SIMD functionality that hasn't been pushed to upstream. I also made a small fix for loongarch64 (sirntar@58edd0b), but since I don't have a loongarch64 platform to test it, I didn't create any pull request.

Ah. You already sent an email to him. I think he will handle it. :)

@shushanhf
Copy link
Contributor

shushanhf commented Nov 23, 2023

Noti: @shushanhf I checked dwarf register numbers for other archs as well as riscv. I think loongarch64 DWARF register number for floating-point register starts from 32 according to https://github.com/loongson/la-abi-specs/blob/release/ladwarf.adoc#loongarch-specific-dwarf-definitions. Please take a look. :)

case REG_F0:
dwarfReg = 64;
break;

I sent an email about it to @shushanhf last week. If I remember correctly, he responded that it could have been a part of SIMD functionality that hasn't been pushed to upstream. I also made a small fix for loongarch64 (sirntar@58edd0b), but since I don't have a loongarch64 platform to test it, I didn't create any pull request.

Sorry, I forgot to reply to all.

Thanks very much!

I think that is for SIMD registers context. Although the main branch doesn't contain the LoongArch64-SIMD
In fact our local runtime had supported the LoongArch64-SIMD, of course the OS also supports the SIMD.

src/coreclr/jit/unwindriscv64.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/unwindriscv64.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/unwindriscv64.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/unwindriscv64.cpp Outdated Show resolved Hide resolved
src/coreclr/jit/unwindriscv64.cpp Outdated Show resolved Hide resolved
@clamp03 clamp03 requested review from jakobbotsch and removed request for yurai007 November 28, 2023 01:50
@jakobbotsch jakobbotsch merged commit 05bc701 into dotnet:main Nov 28, 2023
129 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Dec 29, 2023
@sirntar sirntar deleted the unwind-risc64 branch October 9, 2024 10:02
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