[LoongArch64] add the coreclr/pal directory #62887

shushanhf · 2021-12-16T05:09:38Z

[LoongArch64] add the coreclr/pal and libunwind directory.

update from runtime.

am11 · 2021-12-16T08:03:59Z

@shushanhf, I assume you have just applied libunwind/libunwind@c5f1d12 on v1.6.2 and not copied the upstream master branch, right? If not please only apply the commit which is needed. Also, please update the version file so we can keep track of changes. This helps us when updating libunwind dependency. e.g. in src/coreclr/pal/src/libunwind/libunwind-version.txt, add a line:

Apply https://github.com/libunwind/libunwind/pull/316

jkotas · 2021-12-16T14:16:35Z

It may make sense to submit the libunwind update as separate PR.

shushanhf · 2021-12-17T07:55:38Z

@shushanhf, I assume you have just applied libunwind/libunwind@c5f1d12 on v1.6.2 and not copied the upstream master branch, right? If not please only apply the commit which is needed. Also, please update the version file so we can keep track of changes. This helps us when updating libunwind dependency. e.g. in src/coreclr/pal/src/libunwind/libunwind-version.txt, add a line:
Apply https://github.com/libunwind/libunwind/pull/316

In fact we used the first version libunwind for .NET6 and passed, then our company commits it to the upstream.
In this PR, I didn't pull the libunwind from the upstream.

shushanhf · 2021-12-17T07:55:54Z

@shushanhf, I assume you have just applied libunwind/libunwind@c5f1d12 on v1.6.2 and not copied the upstream master branch, right? If not please only apply the commit which is needed. Also, please update the version file so we can keep track of changes. This helps us when updating libunwind dependency. e.g. in src/coreclr/pal/src/libunwind/libunwind-version.txt, add a line:
Apply https://github.com/libunwind/libunwind/pull/316

am11 · 2021-12-17T16:26:18Z

I think if you could create a separate PR just for libunwind, as @jkotas suggested (collecting only the libunwind related changes from #62889 and this PR), i.e. everything under src/coreclr/pal/src/libunwind and nothing outside, that would make it quicker to review. Here is an example of such a PR for s390x architecture: #53286.

In terms of bash:

# start fresh
git remote add dotnet https://github.com/dotnet/runtime
git checkout -b feature/loongarch64/libunwind
git fetch --all
git reset --hard dotnet/main

# apply a clean patch
curl -sSL https://github.com/libunwind/libunwind/commit/c5f1d12.patch |\
    git apply - --directory src/coreclr/pal/src/libunwind

# manually update src/coreclr/pal/src/libunwind/libunwind-version.txt
# manually update CMakeLists.txt etc. (as you did in this PR: #62092)
git commit -m "Add loongarch64 support in libunwind"
git push -u origin feature/loongarch64/libunwind
# create a new PR from branch 'feature/loongarch64/libunwind'

The next time someone will update this lib, it will save them trouble to compare files and instead they will just follow our regular steps (#62092 (comment)). I am working on improving it a little bit so we don't modify CMakeLists.txt and friends in vendor's directory and changing libunwind's location to src/native/external; main...am11:feature/consolidations, it is actually ready, I'm just waiting for yours loongarch64 changes to go in first to avoid merge conflicts. :)

shushanhf · 2021-12-18T00:28:50Z

I think if you could create a separate PR just for libunwind, as @jkotas suggested (collecting only the libunwind related changes from #62889 and this PR), i.e. everything under src/coreclr/pal/src/libunwind and nothing outside, that would make it quicker to review. Here is an example of such a PR for s390x architecture: #53286.

In terms of bash:
# start fresh
git remote add dotnet https://github.com/dotnet/runtime
git checkout -b feature/loongarch64/libunwind
git fetch --all
git reset --hard dotnet/main

# apply a clean patch
curl -sSL https://github.com/libunwind/libunwind/commit/c5f1d12.patch |\
    git apply - --directory src/coreclr/pal/src/libunwind

# manually update src/coreclr/pal/src/libunwind/libunwind-version.txt
# manually update CMakeLists.txt etc. (as you did in this PR: #62092)
git commit -m "Add loongarch64 support in libunwind"
git push -u origin feature/loongarch64/libunwind
# create a new PR from branch 'feature/loongarch64/libunwind'
The next time someone will update this lib, it will save them trouble to compare files and instead they will just follow our regular steps (#62092 (comment)). I am working on improving it a little bit so we don't modify CMakeLists.txt and friends in vendor's directory and changing libunwind's location to src/native/external; main...am11:feature/consolidations, it is actually ready, I'm just waiting for yours loongarch64 changes to go in first to avoid merge conflicts. :)

Thanks for your review and advices,
I will update it later.

It will be commited within a new PR.

shushanhf · 2021-12-18T06:27:57Z

I think if you could create a separate PR just for libunwind, as @jkotas suggested (collecting only the libunwind related changes from #62889 and this PR), i.e. everything under src/coreclr/pal/src/libunwind and nothing outside, that would make it quicker to review. Here is an example of such a PR for s390x architecture: #53286.

In terms of bash:
# start fresh
git remote add dotnet https://github.com/dotnet/runtime
git checkout -b feature/loongarch64/libunwind
git fetch --all
git reset --hard dotnet/main

# apply a clean patch
curl -sSL https://github.com/libunwind/libunwind/commit/c5f1d12.patch |\
    git apply - --directory src/coreclr/pal/src/libunwind

# manually update src/coreclr/pal/src/libunwind/libunwind-version.txt
# manually update CMakeLists.txt etc. (as you did in this PR: #62092)
git commit -m "Add loongarch64 support in libunwind"
git push -u origin feature/loongarch64/libunwind
# create a new PR from branch 'feature/loongarch64/libunwind'
The next time someone will update this lib, it will save them trouble to compare files and instead they will just follow our regular steps (#62092 (comment)). I am working on improving it a little bit so we don't modify CMakeLists.txt and friends in vendor's directory and changing libunwind's location to src/native/external; main...am11:feature/consolidations, it is actually ready, I'm just waiting for yours loongarch64 changes to go in first to avoid merge conflicts. :)

Had pushed the libunwind separately by #62979

am11 · 2021-12-18T07:15:15Z

src/coreclr/pal/src/eventprovider/lttngprovider/CMakeLists.txt

+if(NOT CLR_CMAKE_TARGET_ARCH_LOONGARCH64)
+  target_link_libraries(coreclrtraceptprovider
+                        ${LTTNG}
+  )
+else()
+  target_link_libraries(coreclrtraceptprovider
+                        lttng-ust-tracepoint
+                        urcu
+                        urcu-bp
+                        urcu-cds
+                        urcu-common
+                        ${LTTNG}
+  )
+endif()


Curious, what is the error if we don't make this change? lttng-ust-tracepoint is actually a soft dependency, i.e. this library gets loaded via dlopen() during the run time of application. Even the existing target_link_libraries is not needed, it is a leftover from past refactoring, we can just delete it altogether.

Curious, what is the error if we don't make this change? lttng-ust-tracepoint is actually a soft dependency, i.e. this library gets loaded via dlopen() during the run time of application. Even the existing target_link_libraries is not needed, it is a leftover from past refactoring, we can just delete it altogether.

Yes, I thinks so.
I check it within my machine now.

Thanks for your review!
I had deleted the modification. (I guess that it's old code from the mips64 when we porting it.)

shushanhf · 2021-12-21T01:40:21Z

Now this PR can be compiled sucessfully on the native mode.
For example,
https://dev.azure.com/dnceng/public/_build/results?buildId=1522042&view=logs&j=86357178-b8b8-53d0-d37b-335a1e08c946&t=75d29754-3c44-5c16-fcdb-e0edbf440620

What should I do next ?

…2886.

shushanhf · 2021-12-21T09:55:41Z

Now the compiling errors are within the manged product.
Like this:
https://dev.azure.com/dnceng/public/_build/results?buildId=1522199&view=logs&j=11a68750-4cc0-50f6-a992-7042650640b8&t=26e8beda-8bda-5022-b907-3e9f228434d9

am11 · 2021-12-21T12:07:32Z

The native and managed jitinterfaces used by crossgen2 are not in synchronized state anymore. If int *flags argument is needed by loongarch64, maybe add it to the existing getArgType and leave it unused on the other architectures (#ifndef TARGET_LOONGARCH64\n(void)flags; // unused). Otherwise, add it with a new name like getArgTypeWithFlags. After that run src/coreclr/tools/Common/JitInterface/ThunkGenerator/gen.sh from root of the repo which will take a minute and synchronize the native and managed sides of JIT interface, and adjust the list ordinals in CorInfoBase.cs.

jkotas · 2021-12-21T14:32:48Z

The JIT interface changes should be excluded from this PR. This PR should be only touching the PAL.

am11 · 2021-12-21T15:21:43Z

Agreed with @jkotas. PAL changes in a separate PR as a first component during a platform port makes things much clearer for coreclr partition.

@shushanhf, could you please revert changes in files which are not under src/coreclr/pal directory? Also:

remove libunwind from this PR's title
build and run PAL tests to make sure this base layer is working as expected on linux loongarch64: https://github.com/dotnet/runtime/blob/3b40c7c/docs/workflow/testing/coreclr/unix-test-instructions.md#pal-tests

shushanhf · 2021-12-22T03:06:27Z

The JIT interface changes should be excluded from this PR. This PR should be only touching the PAL.

Agreed with @jkotas. PAL changes in a separate PR as a first component during a platform port makes things much clearer for coreclr partition.

@shushanhf, could you please revert changes in files which are not under src/coreclr/pal directory? Also:

remove libunwind from this PR's title

build and run PAL tests to make sure this base layer is working as expected on linux loongarch64: https://github.com/dotnet/runtime/blob/3b40c7c/docs/workflow/testing/coreclr/unix-test-instructions.md#pal-tests

Thanks, I will do it.

BruceForstall · 2022-02-14T18:16:10Z

@janvorli Is this something you could review?
cc @mangod9

@shushanhf Looks like some jobs failed. Maybe you need to make sure the branch is up to date and retrigger the testing.

janvorli · 2022-02-14T18:22:55Z

@janvorli Is this something you could review?

Yes, I will take a look today.

janvorli

Overall it looks good, I have just two comments.

janvorli · 2022-02-14T21:20:18Z

src/coreclr/pal/src/thread/context.cpp

        CONTEXT_CaptureContext(lpContext);
+#else
+        _ASSERT(!"unimplemented on LOONGARCH64 yet");


Should this be implemented in this PR?

OK.
I had delete the assert for having implements of the CONTEXT_CaptureContext.

janvorli · 2022-02-14T21:24:58Z

src/coreclr/pal/inc/pal.h

@@ -2453,6 +2601,9 @@ PALIMPORT BOOL PALAPI PAL_VirtualUnwindOutOfProc(CONTEXT *context, KNONVOLATILE_
 #define PAL_CS_NATIVE_DATA_SIZE 56
 #elif defined(__sun) && defined(__x86_64__)
 #define PAL_CS_NATIVE_DATA_SIZE 48
+#elif defined(__linux__) && defined(__loongarch64)
+////TODO for LOONGARCH64: should confirm !!!


It would be good to confirm this before merging this PR.

Thanks.
I have update it.

…_SIZE`

janvorli

LGTM, thank you!

BruceForstall · 2022-02-15T18:24:30Z

The CI failures look like unrelated infrastructure. Merging.

shushanhf added 8 commits April 27, 2021 16:32

Merge pull request #1 from dotnet/main

47d106f

update from runtime.

Merge branch 'dotnet:main' into master

411b59e

Merge branch 'dotnet:main' into master

7eed4eb

Merge branch 'dotnet:main' into master

0d7c9e4

Merge branch 'dotnet:main' into master

2c534c5

Merge branch 'dotnet:main' into master

6e14d76

Merge branch 'dotnet:main' into master

359875d

Merge branch 'dotnet:main' into main

192b095

shushanhf requested a review from MichalStrehovsky as a code owner December 16, 2021 05:09

dotnet-issue-labeler bot added the area-PAL-coreclr label Dec 16, 2021

ghost added the community-contribution Indicates that the PR has been added by a community member label Dec 16, 2021

[LoongArch64] add the coreclr/pal and libunwind directory. (#59561)

6117d07

shushanhf closed this Dec 17, 2021

shushanhf reopened this Dec 17, 2021

kant2002 mentioned this pull request Dec 18, 2021

[LoongArch64] add all the coreclr-build-config files #62889

Merged

[LoongArch64] delete the libunwind for LoongArch64.

4b1395d

It will be commited within a new PR.

[LoongArch64] add config files from #62889.

f70f07a

am11 reviewed Dec 18, 2021

View reviewed changes

qiaopengcheng and others added 2 commits December 18, 2021 16:39

[LoongArch64] revert some files when moved from #62889.

63cc4dd

[LoongArch64] compiling is sucessfully.

dbf71fc

[LoongArch64] delete the define IMAGE_FILE_MACHINE_LOONGARCH64 for #6…

c46968c

…2886.

shushanhf changed the title ~~[LoongArch64] add the coreclr/pal and libunwind directory~~ [LoongArch64] add the coreclr/pal directory Dec 22, 2021

[LoongArch64] exclude some files from pal directory.

d43f51b

jkotas added the arch-loongarch64 label Dec 22, 2021

shushanhf and others added 3 commits January 13, 2022 11:38

Merge branch 'main' into main_loongarch64_4

80c2143

[LoongArch64] rename the UNW_LOONGARCH_* with UNW_LOONGARCH64_*

80b233a

[LoongArch64] update the version of the LICENSE description.

422145b

BruceForstall added this to the 7.0.0 milestone Feb 14, 2022

BruceForstall requested a review from janvorli February 14, 2022 18:15

janvorli reviewed Feb 14, 2022

View reviewed changes

qiaopengcheng and others added 2 commits February 15, 2022 16:49

[LoongArch64] revert the pal/inc/rt/ntimage.h.

5647e5c

[LoongArch64] delete unused assert and update the `DAC_CS_NATIVE_DATA…

f6d3a93

…_SIZE`

janvorli approved these changes Feb 15, 2022

View reviewed changes

BruceForstall merged commit e7467a8 into dotnet:main Feb 15, 2022

ghost locked as resolved and limited conversation to collaborators Mar 17, 2022

shushanhf deleted the main_loongarch64_4 branch May 11, 2022 01:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[LoongArch64] add the coreclr/pal directory #62887

[LoongArch64] add the coreclr/pal directory #62887

shushanhf commented Dec 16, 2021

am11 commented Dec 16, 2021

jkotas commented Dec 16, 2021

shushanhf commented Dec 17, 2021 •

edited

Loading

shushanhf commented Dec 17, 2021

am11 commented Dec 17, 2021 •

edited

Loading

shushanhf commented Dec 18, 2021

shushanhf commented Dec 18, 2021

am11 Dec 18, 2021

shushanhf Dec 18, 2021

shushanhf Dec 18, 2021

shushanhf commented Dec 21, 2021

shushanhf commented Dec 21, 2021

am11 commented Dec 21, 2021 •

edited

Loading

jkotas commented Dec 21, 2021 •

edited

Loading

am11 commented Dec 21, 2021

shushanhf commented Dec 22, 2021

BruceForstall commented Feb 14, 2022

janvorli commented Feb 14, 2022

janvorli left a comment

janvorli Feb 14, 2022

shushanhf Feb 15, 2022

janvorli Feb 14, 2022

shushanhf Feb 15, 2022

janvorli left a comment

BruceForstall commented Feb 15, 2022

[LoongArch64] add the coreclr/pal directory #62887

[LoongArch64] add the coreclr/pal directory #62887

Conversation

shushanhf commented Dec 16, 2021

am11 commented Dec 16, 2021

jkotas commented Dec 16, 2021

shushanhf commented Dec 17, 2021 • edited Loading

shushanhf commented Dec 17, 2021

am11 commented Dec 17, 2021 • edited Loading

shushanhf commented Dec 18, 2021

shushanhf commented Dec 18, 2021

am11 Dec 18, 2021

Choose a reason for hiding this comment

shushanhf Dec 18, 2021

Choose a reason for hiding this comment

shushanhf Dec 18, 2021

Choose a reason for hiding this comment

shushanhf commented Dec 21, 2021

shushanhf commented Dec 21, 2021

am11 commented Dec 21, 2021 • edited Loading

jkotas commented Dec 21, 2021 • edited Loading

am11 commented Dec 21, 2021

shushanhf commented Dec 22, 2021

BruceForstall commented Feb 14, 2022

janvorli commented Feb 14, 2022

janvorli left a comment

Choose a reason for hiding this comment

janvorli Feb 14, 2022

Choose a reason for hiding this comment

shushanhf Feb 15, 2022

Choose a reason for hiding this comment

janvorli Feb 14, 2022

Choose a reason for hiding this comment

shushanhf Feb 15, 2022

Choose a reason for hiding this comment

janvorli left a comment

Choose a reason for hiding this comment

BruceForstall commented Feb 15, 2022

shushanhf commented Dec 17, 2021 •

edited

Loading

am11 commented Dec 17, 2021 •

edited

Loading

am11 commented Dec 21, 2021 •

edited

Loading

jkotas commented Dec 21, 2021 •

edited

Loading