Skip to content

Simplify platform macro definitions in PAL#73530

Merged
janvorli merged 6 commits into
dotnet:mainfrom
am11:feature/consolidation/platform-definitions
Aug 26, 2022
Merged

Simplify platform macro definitions in PAL#73530
janvorli merged 6 commits into
dotnet:mainfrom
am11:feature/consolidation/platform-definitions

Conversation

@am11

@am11 am11 commented Aug 7, 2022

Copy link
Copy Markdown
Member
  • Delete _M_{ARCH} definitions and replace usage with HOST_{ARCH} and TARGET_{ARCH}.
  • Remove redundant ${arch} from filenames in architecture specific directories.
  • Remove Arm from PAL_ArmInterlockedOperationBarrier and ArmInterlockedOperationBarrier as they are not ARM-specific anymore.
  • Add a few empty stubs for riscv64 so we can run src/coreclr/build-runtime.sh -cross -a riscv64 -component paltests (without running into missing files errors).

@ghost

ghost commented Aug 7, 2022

Copy link
Copy Markdown

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Aug 7, 2022
@am11 am11 added the area-PAL-coreclr only for closed issues label Aug 7, 2022
Comment thread src/coreclr/pal/inc/pal.h Outdated
@am11 am11 requested review from janvorli and jkotas August 7, 2022 15:27
@am11 am11 marked this pull request as ready for review August 7, 2022 15:27
@am11 am11 requested a review from MichalStrehovsky as a code owner August 7, 2022 15:27
Comment thread src/coreclr/jit/hashbv.h Outdated
@am11 am11 force-pushed the feature/consolidation/platform-definitions branch from 16839f6 to 28204ea Compare August 9, 2022 12:42
@am11

am11 commented Aug 9, 2022

Copy link
Copy Markdown
Member Author

@janvorli, the CI issue was fixed after the rebase. PTAL.

@janvorli

Copy link
Copy Markdown
Member

@am11 I am sorry for the delay, I was OOF for the last 7 days.

@janvorli janvorli left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thank you!

Comment thread src/coreclr/pal/inc/pal.h Outdated
Comment thread src/coreclr/unwinder/arm64/unwinder.h
@am11

am11 commented Aug 19, 2022

Copy link
Copy Markdown
Member Author

@EgorBo, any idea why superpmi leg would fail the osx-arm64 checks when I updated PAL_CS_NATIVE_DATA_SIZE only for linux-arm64 to 104 (osx-arm64 is 120 which hasn't changed):

[16:48:11] Invoking: C:\h\w\9D87090C\p\artifacts\spmi\pintools\1.0\windows\pin.exe -follow_execv -t C:\h\w\9D87090C\p\artifacts\spmi\pintools\1.0\windows\clrjit_inscount_x64\clrjit_inscount.dll -quiet -- C:\h\w\9D87090C\p\superpmi.exe -applyDiff -baseMetricsSummary C:\h\w\9D87090C\t\tmpsfrtx1z4\libraries_tests.pmi.OSX.arm64.checked.mch_base_metrics.csv -diffMetricsSummary C:\h\w\9D87090C\t\tmpsfrtx1z4\libraries_tests.pmi.OSX.arm64.checked.mch_diff_metrics.csv -target arm64 -p -failureLimit 100 C:\h\w\9D87090C\p\base\release\clrjit_universal_arm64_x64.dll C:\h\w\9D87090C\p\diff\release\clrjit_universal_arm64_x64.dll C:\h\w\9D87090C\p\artifacts\spmi\mch\1b9551b8-21f4-4233-9c90-f3eabd6a322b.OSX.arm64\libraries_tests.pmi.OSX.arm64.checked.mch

[16:48:15] ERROR: Unexpected exception c0000005 was thrown.

[16:48:15] ERROR: Unexpected exception c0000005 was thrown.

[16:48:15] ERROR: Method 1 of size 88 failed to load and compile correctly by JIT2 (C:\h\w\9D87090C\p\diff\release\clrjit_universal_arm64_x64.dll).

[16:48:15] ERROR: Method 1 of size 88 failed to load and compile correctly by JIT1 (C:\h\w\9D87090C\p\base\release\clrjit_universal_arm64_x64.dll).

The CI was green before PAL_CS_NATIVE_DATA_SIZE/DAC_CS_NATIVE_DATA_SIZE 116->104 change for linux-arm64.

@EgorBo

EgorBo commented Aug 19, 2022

Copy link
Copy Markdown
Member

@am11 we had some issues with it and fixed them yesterday, can you rebase your PR?

@am11 am11 closed this Aug 19, 2022
@am11 am11 reopened this Aug 19, 2022
@janvorli

Copy link
Copy Markdown
Member

@am11 let me retry the superpmi tests one more time and then I'll merge it.

@janvorli janvorli closed this Aug 25, 2022
@janvorli janvorli reopened this Aug 25, 2022
@janvorli janvorli merged commit 536f34d into dotnet:main Aug 26, 2022
akoeplinger added a commit to akoeplinger/runtime that referenced this pull request Aug 30, 2022
It was missed in dotnet#73530, before that we defined `HOST_ARM` for armv6 in pal.h too because of `defined(__arm__)`
akoeplinger added a commit that referenced this pull request Aug 30, 2022
Add missing HOST_ARM define to armv6 in configurecompiler.cmake, it was missed in #73530, before that we defined `HOST_ARM` for armv6 in pal.h too because of `defined(__arm__)`

We also only need the `clr.iltools+clr.packages` subset, not all of CoreCLR.

Also fix some dependencies that weren't working for FreeBSD since we missed the local variables. We don't need installer subset conditions there but we need to trigger on rolling builds.
@ghost ghost locked as resolved and limited conversation to collaborators Sep 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-PAL-coreclr only for closed issues 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