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

[API Proposal]: architecture definitions for RISC-V ISA #73437

Closed
am11 opened this issue Aug 5, 2022 · 29 comments
Closed

[API Proposal]: architecture definitions for RISC-V ISA #73437

am11 opened this issue Aug 5, 2022 · 29 comments
Labels
api-approved API was approved in API review, it can be implemented arch-riscv Related to the RISC-V architecture area-System.Runtime.InteropServices
Milestone

Comments

@am11
Copy link
Member

am11 commented Aug 5, 2022

Background and motivation

RISC-V is the fifth generation of RISC architecture, which is currently supported by mono; while the coreclr port is in progress. We should add the relevant identifiers visible for consumers and to the higher-level APIs.

API Proposal

Ref: https://docs.microsoft.com/en-us/windows/win32/debug/pe-format#machine-types

namespace System.Runtime.InteropServices;

public enum Architecture
{
    ...
+   Riscv64,
}

namespace System.Reflection.PortableExecutable;

public enum Machine : ushort
{
    ...
+   Riscv32 = 0x5032,
+   Riscv64 = 0x5064,
+   Riscv128 = 0x5128,
}

API Usage

int GetArchMagic(Architecture arch) => arch switch
{
    Architecture.Riscv64 => 42,
}

Alternative Designs

RISC-V is an open-source and modular architecture. Its support is often expressed in terms of 'extensions'.

We should provide System.Runtime.Intrinsics.X86Base.CpuId -like API to expose which extensions are supported.

Another twist is that the capability of RISC-V may differ by CPU core or hardware thread (ISA term is hart) on a given system, so knowing which extensions are supported by which hart (e.g. in order make decision on establishing affinity) maybe useful in certain types of multi-threading oriented workloads.

Risks

Yes, it literally is RISC-y.

Just kidding, it is low risk, high reward! 8-)

@am11 am11 added api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Runtime.InteropServices labels Aug 5, 2022
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Aug 5, 2022
@ghost
Copy link

ghost commented Aug 5, 2022

Tagging subscribers to this area: @dotnet/interop-contrib
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

RISC-V is the fifth generation of RISC architecture, which is currently supported by mono; while the coreclr port is in progress. We should add the relevant identifiers visible for consumers and to the higher-level APIs.

API Proposal

Ref: https://docs.microsoft.com/en-us/windows/win32/debug/pe-format#machine-types

namespace System.Runtime.InteropServices;

public enum Architecture
{
    ...
+   Riscv64,
}

namespace System.Reflection.PortableExecutable;

public enum Machine : ushort
{
    ...
+   Riscv32 = 0x5232,
+   Riscv64 = 0x5264
+   Riscv128 = 0x5128,
}

API Usage

int GetArchMagic(Architecture arch) => arch switch
{
    Architecture.Riscv64 => 42,
}

Alternative Designs

RISC-V is an open-source and modular architecture. Its support is often expressed in terms of 'extensions'.

We should provide System.Runtime.Intrinsics.X86Base.CpuId -like API to expose which extensions are supported.

Another twist is that the capability of RISC-V may differ by CPU core or hardware thread (ISA term is hart) on a given system, so knowing which extensions are supported by which hart (e.g. in order make decision on establishing affinity) maybe useful in certain types of multi-threading oriented workloads.

Risks

Yes, it literally is RISC-y.

Just kidding, it is low risk, high reward! 8-)

Author: am11
Assignees: -
Labels:

api-suggestion, area-System.Runtime.InteropServices

Milestone: -

@jkotas
Copy link
Member

jkotas commented Aug 5, 2022

+   Riscv32 = 0x5232,
+   Riscv64 = 0x5264
+   Riscv128 = 0x5128,

These numbers do not match the numbers in https://docs.microsoft.com/en-us/windows/win32/debug/pe-format#machine-types

@am11
Copy link
Member Author

am11 commented Aug 5, 2022

Fixed, thanks.

@vcsjones
Copy link
Member

vcsjones commented Aug 5, 2022

Riscv32

Which ISA base does this imply? RV32I or RV32E? Would it make sense to have one for each? If it's only for RV32I (or possibly RV32E) would it make sense to be explicit in the name?

Or do we imagine these values covering both base ISAs?

@alexrp
Copy link
Contributor

alexrp commented Aug 5, 2022

Which ISA base does this imply?

Unfortunately, it doesn't seem like that was considered when adding these machine types to the PE spec. It would probably be reasonable to assume that they effectively mean RV32I, considering RV32E hasn't been formally ratified yet.

If RV32E is ratified, there would likely need to be new machine types added for it, similar to how there are separate machine types for Arm, Arm Thumb, and Arm Thumb-2. But that's all up to whoever is in charge of the PE spec. The .NET Machine enum just reflects it.

RV32I or RV32E? Would it make sense to have one for each? If it's only for RV32I (or possibly RV32E) would it make sense to be explicit in the name?

RV32E is (roughly) to RISC-V what T32 is to Arm. It seems unlikely that mainline .NET would ever care about it. .NET nanoFramework might, but they have a completely separate and incompatible BCL API surface anyway.

Or do we imagine these values covering both base ISAs?

This would not really make sense since RV32I and RV32E (in its current draft form, anyway) have different calling conventions. There is a new EABI in development that is attempting to unify the calling conventions for the two, though, but who knows how much traction that will gain.

@am11
Copy link
Member Author

am11 commented Aug 5, 2022

Architecture.Riscv32 can be added to the proposal, so far I only added to the PE Machine values which (I think) implies "any extension" (or micro-architecture), included embedded E.

I will defer to @vargaz and @alexrp for accurate answer, but I think mono's implementation is adaptive to a certain extent that JIT keeps track of hwcaps and adapts to the target ISA extension when emitting the machine code. However, from the configuration viewpoint, the support seems to be represented with general-purpose extension G + the compressed instructions C in mind, so I think we can safely say that we support RISCV32GC and RISCV64GC. (GC is a short form of these extensions: IMAFDZicsrC), which most Linux distros (userlands) have chosen for the initial bringup.

In CoreCLR, we can also implement it in an adaptive manner; the prior art for ARM32 is at:

PAL_GetJitCpuCapabilityFlags(CORJIT_FLAGS *flags)

Since each core can have different capability in RISC-V and as time is passing on, more extensions are being introduced and getauxval(3) may not be sufficient (when we will have more than one alphabet per extension), so we can use mcpuid instruction (https://riscv.org/wp-content/uploads/2015/11/riscv-privileged-spec-v1.7.pdf) for precise identification of μ-arch for JITs and to provide API similar to System.Runtime.Intrinsics.X86Base.CpuId().

@alexrp
Copy link
Contributor

alexrp commented Aug 5, 2022

I will defer to @vargaz and @alexrp for accurate answer, but I think mono's implementation is adaptive to a certain extent that JIT keeps track of hwcaps and adapts to the target ISA extension when emitting the machine code. However, from the configuration viewpoint, the support seems to be represented with general-purpose extension G + the compressed instructions C in mind, so I think we can safely say that we support RISCV32GC and RISCV64GC. (GC is a short form of these extensions: IMAFDZicsrC), which most Linux distros (userlands) have chosen for the initial bringup.

It's been a while since I touched the RISC-V code in Mono, but I think Mono would be able to function with just I + A. Our experience on older Arm versions indicated that it's virtually impossible to make a .NET runtime function properly without atomic instructions, so the A extension is effectively mandatory.

M is just for faster multiplication; it can be emulated in a JIT helper. Mono has soft float support so F and D are also optional, but I'm not sure if dotnet/runtime cares about soft float support anymore, so it might be acceptable to require those. I didn't even consider C when writing riscv-codegen.h... I figured that would be a code size optimization that could potentially be added later down the line.

@AaronRobinsonMSFT AaronRobinsonMSFT removed the untriaged New issue has not been triaged by the area owner label Aug 5, 2022
@AaronRobinsonMSFT AaronRobinsonMSFT added this to the 8.0.0 milestone Aug 5, 2022
@lambdageek lambdageek added the arch-riscv Related to the RISC-V architecture label Aug 17, 2022
@alexrp
Copy link
Contributor

alexrp commented Sep 25, 2022

Naming nitpick: Do we want to spell it as Riscv64, RiscV64, or RISCV64? There could even be an argument for Rv64 or RV64 as that's a common abbreviation.

@am11
Copy link
Member Author

am11 commented Sep 25, 2022

Riscv64 casing is after the existing Armv6 in enum Architecture.

qemu, llvm and gnu toolchains etc. use the long abbreviations: riscv128, riscv64 and riscv32.
(empirically speaking) Short abbreviation is less popular than the long one.

@jkoritzinsky
Copy link
Member

@AaronRobinsonMSFT @lambdageek do we want to get this in for .NET 8? We're running out of runway here if we do.

@AaronRobinsonMSFT
Copy link
Member

I don't think we need this right now. @am11 do you have any concerns moving this to .NET 9?

@jkotas
Copy link
Member

jkotas commented Jul 20, 2023

cc @clamp03 @alpencolt

@am11
Copy link
Member Author

am11 commented Jul 20, 2023

do you have any concerns moving this to .NET 9?

No concern.

@am11 am11 modified the milestones: 8.0.0, 9.0.0 Jul 20, 2023
@gbalykov
Copy link
Member

We do not have strict requirement to add this to .NET 8 currently, we've just started investigation of libraries and work to be done in them, so this'll probably be done after .NET 8. I'll share more details about our status in #84834 in few days.

@AaronRobinsonMSFT AaronRobinsonMSFT added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Aug 22, 2023
@gbalykov
Copy link
Member

@AaronRobinsonMSFT @am11 is there still time to get this to .NET 8?

@am11
Copy link
Member Author

am11 commented Aug 30, 2023

@gbalykov, IMO, targeting riscv64 for .NET 9 is reasonable, since:

  • that will give us more time for libraries testing and porting some known community native libs to the new architrecture (e.g. https://github.com/ericsink/SQLitePCL.raw via https://github.com/ericsink/cb for 'sqlite-net on linux-riscv64' support)
  • there (still) aren't many consumer devices (desktop / laptop) widely available. That picture is supposedly change in the coming months (🤞 though).
  • Debian has recently changed riscv64 status from Port to Supported architecture. Some other distros will be doing the same (I think as soon as consumer / commercial devices are made available).

With that being said, if the enum member is backported to .NET 8 branch, it wouldn't hurt. 🙂

Do you think having this API in .NET 8 is important?

@MichalPetryka
Copy link
Contributor

Do you think having this API in .NET 8 is important?

I'd say that it'll help users prepare their libraries beforehand which would be useful.

@gbalykov
Copy link
Member

Do you think having this API in .NET 8 is important?

Yes, I think this would be useful for further porting

@clamp03
Copy link
Member

clamp03 commented Aug 31, 2023

Do you think having this API in .NET 8 is important?

We normally put .NET LTS version on our products. And actually, we should start to check .NET on the products in this year or in the beginning of next year at the latest. .NET 9 is not LTS and it is too late for us. If you don't mind, I also hope this API in the .NET 8 for further porting.

@am11
Copy link
Member Author

am11 commented Aug 31, 2023

@AaronRobinsonMSFT, @jkotas, is there a chance to include this in .NET 8 to unblock the porting efforts?

@jkotas
Copy link
Member

jkotas commented Aug 31, 2023

.NET 8 is done. All changes going into .NET 8 are treated as servicing. I do not think that this API meets the bar for a servicing change.

This API is not used very frequently and the few places that need it can use (Architecture)9 as a workaround for now. Is it really blocking any RISC-V porting efforts?

@AaronRobinsonMSFT
Copy link
Member

@AaronRobinsonMSFT, @jkotas, is there a chance to include this in .NET 8 to unblock the porting efforts?

I am doubtful, but we can try. We really needed this back in JULY, hence the question at #73437 (comment).

Let me see what we can do.

@AaronRobinsonMSFT
Copy link
Member

@am11 Sorry to say, but it really is too late for this sort of API addition. The bar is rather high and since this doesn't block an existing scenario, and there is an ugly, but feasible workaround, the new API will need to wait for .NET 9.

@bartonjs
Copy link
Member

We changed the casing from Riscv to RiscV since the V component is a different pronounced word.

namespace System.Runtime.InteropServices;

public enum Architecture
{
    ...
+   RiscV64,
}

namespace System.Reflection.PortableExecutable;

public enum Machine : ushort
{
    ...
+   RiscV32 = 0x5032,
+   RiscV64 = 0x5064,
+   RiscV128 = 0x5128,
}

@bartonjs bartonjs added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Sep 12, 2023
@AaronRobinsonMSFT
Copy link
Member

/cc @am11 @gbalykov

@am11
Copy link
Member Author

am11 commented Sep 13, 2023

Resolved by #90203.

@danmoseley
Copy link
Member

Are our rules quite deterministic? Eg S390x not S390X. Or Armv6 not ArmV6 or ARMV6.
(Not trying to be pedantic just wondering what rules are.)

@jkotas
Copy link
Member

jkotas commented Sep 21, 2023

.NET Framework design guidelines has a whole chapter on Capitalization Conventions.

@am11
Copy link
Member Author

am11 commented Sep 21, 2023

Armv6 not ArmV6 or ARMV6.

I think it is Arm because it has three letters (one or two letters use all upper-case letters), and small v as it literally stands for version to differentiate it from Arm (v7). In case of RISC-V, it is pronounced as "risk-five", fifth generation of RISC ISA, i.e. it only conceptually shares this lineage but has no compatibility relationship with its predecessors; it's its own thing, just like ARM is also a member of RISC ISA family.

@ghost ghost locked as resolved and limited conversation to collaborators Oct 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented arch-riscv Related to the RISC-V architecture area-System.Runtime.InteropServices
Projects
Archived in project
Development

No branches or pull requests