Skip to content

LoongArch is a NEW ISA which is different from MIPS #113564

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Mar 17, 2025
Merged

LoongArch is a NEW ISA which is different from MIPS #113564

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
269053c
LoongArch is a NEW ISA which is different from MIPS
4Darmygeometry Mar 15, 2025
29100be
Merge branch 'dotnet:main' into main
4Darmygeometry Mar 16, 2025
ec570a9
Update src/coreclr/jit/codegenlinear.cpp
jkotas Mar 17, 2025
1ad05ee
Update src/coreclr/jit/codegenlinear.cpp
jkotas Mar 17, 2025
1558fd4
Merge branch 'dotnet:main' into main
4Darmygeometry Mar 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/coreclr/jit/codegenlinear.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2484,8 +2484,8 @@ CodeGen::GenIntCastDesc::GenIntCastDesc(GenTreeCast* cast)
}

#if defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64)
// For LoongArch64's ISA which is same with the MIPS64 ISA, even the instructions of 32bits operation need
// the upper 32bits be sign-extended to 64 bits.
// For LoongArch64's ISA which is a RISC ISA which is different from any other existing ones,
// even the instructions of 32bits operation need the upper 32bits be sign-extended to 64 bits.
Copy link
Member

Choose a reason for hiding this comment

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

The comment should be saying "LoongArch64 is using the same mechanism with MIPS64 in this scenario"

Copy link
Member

Choose a reason for hiding this comment

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

@shushanhf Can you help to clarify the meaning of this comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can be change to:
LoongArch64 is using the similar mechanism with MIPS64 in this scenario

Copy link

Choose a reason for hiding this comment

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

Can be change to: LoongArch64 is using the similar mechanism with MIPS64 in this scenario

The problem is LoongArch64 does NOT have this mechanism (except for division instructions). So this would be still incorrect.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can be change to: LoongArch64 is using the similar mechanism with MIPS64 in this scenario

Yes, here LA64 has to keep similar mechanism with MIPS64.

Copy link
Contributor

@shushanhf shushanhf Mar 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Can be change to: LoongArch64 is using the similar mechanism with MIPS64 in this scenario

The problem is LoongArch64 does NOT have this mechanism (except for division instructions). So this would be still incorrect.

Although the LA64 is not same with MIPS64 compeletely, liking you said there are still some instructions needing the upper 32bits be sign-extended to 64 bits, we have to keep the similar mechanism for LA64.
If you delete these code it will introduce some errors when you running the test cases.

m_extendKind = SIGN_EXTEND_INT;
#else
m_extendKind = COPY;
Expand Down