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

[LoongArch64] Part-3:Add changes in ObjectWriter component about nativeaot. #104087

Merged
merged 1 commit into from
Jun 27, 2024

Conversation

sunlijun-610
Copy link
Contributor

This PR is the second part adding nativeaot support on LoongArch64.
It adds changes in ObjectWriter component.

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jun 27, 2024
Copy link
Contributor

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

@@ -161,6 +161,10 @@ public static int DwarfRegNum(TargetArchitecture architecture, int regNum)
_ => regNum - (int)RegNumX86.REGNUM_COUNT + 32 // FP registers
};

case TargetArchitecture.LoongArch64:
// Normal registers are directly mapped
Copy link
Member

Choose a reason for hiding this comment

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

Are all registers in LA64 are simply mapped, including FP and SIMD?
The comment here can be reworded.

Copy link
Member

Choose a reason for hiding this comment

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

The JIT mapping for FP regs starts at F0 == 32, the DWARF mapping starts at 64 AFAIK. Ideally this should be fixed by adding the code to correct the offset. Not sure about SIMD and if it's exposed at all.

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 think in LoongArch64, the DWARF mapping starts at 32, because UNW_LOONGARCH_F0 = 32 and UNW_LOONGARCH_F31 = 63. Not sure about SIMD, and it's not exposed yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

The JIT mapping for FP regs starts at F0 == 32, the DWARF mapping starts at 64 AFAIK.

Yes, the LA64's DWARF-document supporting the SIMD, it starts from 64.

Ideally this should be fixed by adding the code to correct the offset. Not sure about SIMD and if it's exposed at all.

we had planned to update these within the Intrinsic pushing but the LoongArch64's Intrinsic patch is pending for waiting the LA64's SIMD manual.
I think it is close to the time publishing SIMD manual, and we will modify these together.

Thanks for your advices.

Copy link
Member

Choose a reason for hiding this comment

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

You are right. I cross-checked the ABI docs. JIT maps the REG_F0 to 32, DWARF also maps it to 32 (la-abi-v2.30, section 6.1. DWARF Register Numbers).

However, the JIT uses DWARF register numbers that don't match the spec when generating unwinding information:

case REG_F0:
dwarfReg = 64;
break;
case REG_F1:
dwarfReg = 65;
break;
case REG_F2:
dwarfReg = 66;
break;
case REG_F3:
dwarfReg = 67;
break;
case REG_F4:
dwarfReg = 68;
break;
case REG_F5:
dwarfReg = 69;
break;
case REG_F6:
dwarfReg = 70;
break;
case REG_F7:
dwarfReg = 71;
break;
case REG_F8:
dwarfReg = 72;
break;
case REG_F9:
dwarfReg = 73;
break;
case REG_F10:
dwarfReg = 74;
break;
case REG_F11:
dwarfReg = 75;
break;
case REG_F12:
dwarfReg = 76;
break;
case REG_F13:
dwarfReg = 77;
break;
case REG_F14:
dwarfReg = 78;
break;
case REG_F15:
dwarfReg = 79;
break;
case REG_F16:
dwarfReg = 80;
break;
case REG_F17:
dwarfReg = 81;
break;
case REG_F18:
dwarfReg = 82;
break;
case REG_F19:
dwarfReg = 83;
break;
case REG_F20:
dwarfReg = 84;
break;
case REG_F21:
dwarfReg = 85;
break;
case REG_F22:
dwarfReg = 86;
break;
case REG_F23:
dwarfReg = 87;
break;
case REG_F24:
dwarfReg = 88;
break;
case REG_F25:
dwarfReg = 89;
break;
case REG_F26:
dwarfReg = 90;
break;
case REG_F27:
dwarfReg = 91;
break;
case REG_F28:
dwarfReg = 92;
break;
case REG_F29:
dwarfReg = 93;
break;
case REG_F30:
dwarfReg = 94;
break;
case REG_F31:
dwarfReg = 95;
break;

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, you are right.
These had been updated after LA merged into runtime, but the other places we didn't update, in fact you had told us and we planned to do this together with pushing the Intrinsic-LoongArch64 while the Intrinsic is pending temporarily.
We will do these in future.
Thanks for your advice!

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Thanks!

@jkotas jkotas merged commit aa34349 into dotnet:main Jun 27, 2024
81 of 87 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jul 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-loongarch64 area-NativeAOT-coreclr 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.

6 participants