Skip to content

Conversation

@shushanhf
Copy link
Contributor

Add new ABI classifier for LoongArch64. #100744

The first patch is only the LoongArch64Classifier.
The CodeGen::genHomeRegisterParams will be considered later whether to uniform within the codegencommon.cpp.

@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Apr 18, 2024
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Apr 18, 2024
@dotnet-policy-service
Copy link
Contributor

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

@shushanhf shushanhf force-pushed the LA_ABI_classifier branch 2 times, most recently from 4447663 to c6bcb4f Compare April 18, 2024 06:47
@jakobbotsch
Copy link
Member

There are some formatting errors as well. Can you please fix those?

@shushanhf shushanhf force-pushed the LA_ABI_classifier branch 2 times, most recently from 5d9669c to 780e867 Compare April 18, 2024 08:55
Copy link
Member

@jakobbotsch jakobbotsch left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

@shushanhf
Copy link
Contributor Author

shushanhf commented Apr 18, 2024

Hi, @jakobbotsch
After LA64 using the CodeGen::genHomeRegisterParams within codegencommon.cpp, although I had some tests passed on LA64 and more tests are still in progressing.
I still am not sure here: the line-3335 and line-3041 within codegencommon.cpp.

Can you give some advices ?

Besides, althought the split args within the LoongArch64Classifier::Classify is ok, I also have to check the split args within CodeGen::genHomeRegisterParams.

@jakobbotsch
Copy link
Member

Hi, @jakobbotsch After LA64 using the CodeGen::genHomeRegisterParams within codegencommon.cpp, although I had some tests passed on LA64 and more tests are still in progressing. I still am not sure here: the line-3335 and line-3041 within codegencommon.cpp.

Can you give some advices ?

Line 3335: This is handling to support SIMD types. The JIT can enregister a SIMD type parameter in a single vector register while it may be passed in multiple registers. For example, on arm64:

public static float Test(float x, Vector3 v)
{
    return v.Length();
}
Parameter V00 ABI info: [00..04) reg d0
Parameter V01 ABI info: 3 segments
  [0] [00..04) reg d1
  [1] [04..08) reg d2
  [2] [08..12) reg d3
...
*************** In genHomeRegisterParams()
4 registers in register parameter interference graph
  d1
  d0
    <- d3 (offset: 8)
    <- d2 (offset: 4)
    <- d1
  d2
  d3
IN000b:             fmov    s0, s1
IN000c:             mov     v0.s[2], v3.s[0]
IN000d:             mov     v0.s[1], v2.s[0]

I think you don't need this yet for LA64.

Line 3041: This is purely an optimization for arm64 to make stores to the stack always the same size, so that we can use stp more often. It doesn't affect correctness.

@shushanhf
Copy link
Contributor Author

Thanks very much.

@jakobbotsch jakobbotsch merged commit 588b775 into dotnet:main Apr 18, 2024
@shushanhf shushanhf deleted the LA_ABI_classifier branch April 19, 2024 00:44
@shushanhf
Copy link
Contributor Author

Besides, althought the split args within the LoongArch64Classifier::Classify is ok, I also have to check the split args within CodeGen::genHomeRegisterParams.

#101340 (comment)

matouskozak pushed a commit to matouskozak/runtime that referenced this pull request Apr 30, 2024
* add new ABI classifier.

* share `CodeGen::genHomeRegisterParams` with XARCH-ARMARCH.
@github-actions github-actions bot locked and limited conversation to collaborators May 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

arch-loongarch64 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.

3 participants