[NO REVIEW] Update the CPUID and XSAVE logics for APX #103019

Ruihan-Yin · 2024-06-04T01:12:34Z

[No need to review this PR for now]

Overview on the changes:

Infrastructural changes on XArchIntrinsicConstants: Compress all the Avx512 related flags into 1 - Avx512f+bw+cd+dq+vl to Avx512, this saves more space in XArchIntrinsicConstants so that we can hold more x86 ISAs, like here, APX.

Edit: remove unnecessary Avx512VL ISA flags. #103144 and Cleanup some handling around Avx10v1 #103241 have resolved this part.

CPUID check updates for APX: CR4[XSAVE](existing) -> XCR0[APX_F] -> CPUID(7,1).EDX[APX_F] - the current status is that due to the missing macro definition for APX on the OS level, the second check will fail anyways, and it may break the build on CI (to be verified).
APX introduced 16 EGPRs, which are XSTATE enabled, this PR provides the corresponding changes for XSAVE behaviors.

Update the asm code in context2.S hard coded the memory offset with an assumption that we are using standrad form Seems the logic in context2.S is for custom stack, we may not need to follow the standard XSAVE buffer layout. Directly adding the APX section after AVX512. Updates in threadsuspend.cpp script-gen changes Extending CPUID flag from int to long to hold more ISAs. Update the cpuid check logic, make sure CR4.OSXSAVE and XCR0.APX_F are checked. resolve comments improve the logics in isa_detection in gc. add missing method definitions under unix context. code clean up

2. merge Avx512F, BW, CD, DQ to a converged ISA flag, Avx512. 3. introduce APX cpuid detection.

tannergooding · 2024-06-04T03:06:22Z

I've gone ahead and marked this as NO-MERGE for now. If it is ready for merge prior to main opening for .NET 10 changes, we would want to go through some additional considerations as to the impact it would have on .NET 9 and whether or not we want it excluded from the .NET 9 build such as via an #if defined(FEATURE_APX) flag.

Ruihan-Yin · 2024-06-04T03:10:33Z

I've gone ahead and marked this as NO-MERGE for now. If it is ready for merge prior to main opening for .NET 10 changes, we would want to go through some additional considerations as to the impact it would have on .NET 9 and whether or not we want it excluded from the .NET 9 build such as via an #if defined(FEATURE_APX) flag.

Yes, please mark it as NO-MERGE, thanks.

tannergooding · 2024-06-04T03:12:31Z

apx-doc/README.md

+# APX Integration in .NET
+
+Let's keep documentation on APX integration and notes on things here. I will evolve this as necessary.


We should move this under docs/design/features/xarch-apx.md or maybe under docs/design/coreclr/jit/xarch-apx.md, just to keep it in the same general place as our other docs.

I like the idea of having a documentation covering how the feature is used and integrates with the rest of the JIT.

jkotas · 2024-06-04T04:58:32Z

src/coreclr/tools/Common/Compiler/HardwareIntrinsicHelpers.cs

-            public const int Avx10v1 = 0x4000000;
-            public const int Avx10v1_v256 = 0x8000000;
-            public const int Avx10v1_v512 = 0x10000000;
+            public const int Avx512 = 0x8000;


If you would like to make these bits more compact, it would be better to submit it as separate PR. There are free bits now so you do not need it to add the Apx bit.

It may be better to avoid folding the features together at the PAL level:

public const int Avx512f = ...; public const int Avx512f_vl = ...; public const int Avx512bw = ...; public const int Avx512cd = ...; public const int Avx512dq = ...; public const int Avx512Vbmi = ...;

I understand that it does not save as many bits, but it is cleaner (avoids applying JIT policy at the PAL level).

Thanks for the inputs, I can work on this part soon

jkotas · 2024-06-04T05:01:39Z

src/coreclr/gc/vxsort/isa_detection.cpp

+
+    if (__builtin_cpu_supports("avx2"))
+    {
+        IsaFlags |= (int)SupportedISA::APX;


It is not necessary to detect APX here until the APX-specific implementation of the GC sort gets added.

tannergooding · 2024-06-05T03:37:11Z

src/coreclr/pal/src/include/pal/context.h

+// TODO-xarch-apx: the definition of XSTATE mask value for APX is now missing on the OS level, 
+//                 we are currently using bare value to hack it through the build process, and test the implementation through CI.
+//                 those changes will be removed when we have the OS support for APX.


Why not do this via the following so that things can still function as expected, even if built on a downlevel OS without the support?

#ifndef XSTATE_APX #define XSTATE_APX 19 #endif

I tried this way locally on my own dev machine with Windows OS, the build will fail with error report saying the macros are not defined, and it seems that, at least on Windows, XSTATE_APX and XSTATE_MASK_APX shall be defined in winnt.h (I am not 100% sure about the reason, is it because we are using Windows API in the CPUID checks?), I can fix the error by adding the definition there, but it might not be the solution in CI environment.

huoyaoyuan · 2024-06-05T03:30:59Z

src/coreclr/inc/corinfoinstructionset.h

-    InstructionSet_AVX10v1_X64=68,
-    InstructionSet_AVX10v1_V256_X64=69,
-    InstructionSet_AVX10v1_V512_X64=70,
+    InstructionSet_APX=36,


Does APX support 32bit at all?

Intel®Advanced Performance Extensions (Intel® APX) expands the Intel® 64 instruction set

or is everything just generated with both x86 and x64?

Thanks for bringing this up,

No, APX features is available only under 64-bit mode
Reference: https://www.intel.com/content/www/us/en/content-details/819797/intel-advanced-performance-extensions-intel-apx-architecture-specification.html

3.1.4 Intel® APX features are only available in IA-32e 64-bit Protected Mode, and are an XSAVE-enabled feature
which requires XCR0 enabling before using the new Intel® APX ISA, new Intel® APX prefixes (REX2) and
prefix extensions (EVEX extensions). See section 3.1.4.2 for details on XCR0-enabling for Intel® APX

Based on this fact, I understand APX could be different from existing ISAs and we are open to take suggestions on this part.

dotnet-policy-service · 2024-07-05T08:53:08Z

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

Ruihan-Yin · 2024-07-09T18:17:33Z

Hi @tannergooding

Since the XArchIntrinsicConstant has been updated, I will start to work on this PR again, can we reopen this PR? Thanks!

tannergooding · 2024-07-09T19:28:43Z

I will start to work on this PR again, can we reopen this PR? Thanks!

GitHub blocks it being reopened due to the branch having been force-pushed or recreated since the time the PR was closed

Ruihan-Yin · 2024-07-09T19:58:41Z

I will start to work on this PR again, can we reopen this PR? Thanks!

GitHub blocks it being reopened due to the branch having been force-pushed or recreated since the time the PR was closed

I updated my branch beforehand, my bad, I will create another draft PR.

anthonycanino and others added 5 commits June 3, 2024 16:26

Adding an APX doc.

f27618f

1. revert the int-to-long changes

38c5366

2. merge Avx512F, BW, CD, DQ to a converged ISA flag, Avx512. 3. introduce APX cpuid detection.

improve the Avx512 check logics.

ffb7a01

resolve merge conflict

1923060

Ruihan-Yin requested a review from MichalStrehovsky as a code owner June 4, 2024 01:12

dotnet-issue-labeler bot added the area-PAL-coreclr label Jun 4, 2024

dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jun 4, 2024

Ruihan-Yin marked this pull request as draft June 4, 2024 01:13

tannergooding added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Jun 4, 2024

tannergooding reviewed Jun 4, 2024

View reviewed changes

jkotas reviewed Jun 4, 2024

View reviewed changes

This was referenced Jun 4, 2024

Wasm test failure: System addon update list error SyntaxError: XMLHttpRequest.open #102955

Closed

Wasm test failure: Unable to evaluate script: target frame detached #102957

Closed

Ruihan-Yin added 8 commits June 4, 2024 12:58

revert changes in GC

a22937f

move apx doc to the proper place

8c7878d

Try to hack XSTATE_MASK_APX.

ddfa139

Update comments

8e2f79a

comment clean up

2eb6644

make sure the context size is correct with APX feature.

9877981

exclude OSX from APX changes.

37bc8a0

make sure the context size is correct on unix system.

ff7bae6

tannergooding reviewed Jun 5, 2024

View reviewed changes

huoyaoyuan reviewed Jun 5, 2024

View reviewed changes

Ruihan-Yin mentioned this pull request Jun 6, 2024

remove unnecessary Avx512VL ISA flags. #103144

Merged

dotnet-policy-service bot closed this Jul 5, 2024

Ruihan-Yin mentioned this pull request Jul 9, 2024

Update the CPUID and XSAVE logics for APX #104637

Merged

github-actions bot locked and limited conversation to collaborators Aug 9, 2024

BruceForstall added the apx Related to the Intel Advanced Performance Extensions (APX) label Sep 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[NO REVIEW] Update the CPUID and XSAVE logics for APX #103019

[NO REVIEW] Update the CPUID and XSAVE logics for APX #103019

Ruihan-Yin commented Jun 4, 2024 •

edited

Loading

tannergooding commented Jun 4, 2024

Ruihan-Yin commented Jun 4, 2024

tannergooding Jun 4, 2024

jkotas Jun 4, 2024

Ruihan-Yin Jun 4, 2024

jkotas Jun 4, 2024

tannergooding Jun 5, 2024

Ruihan-Yin Jun 5, 2024

huoyaoyuan Jun 5, 2024

Ruihan-Yin Jun 5, 2024

dotnet-policy-service bot commented Jul 5, 2024

Ruihan-Yin commented Jul 9, 2024

tannergooding commented Jul 9, 2024

Ruihan-Yin commented Jul 9, 2024

		# APX Integration in .NET

		Let's keep documentation on APX integration and notes on things here. I will evolve this as necessary.

[NO REVIEW] Update the CPUID and XSAVE logics for APX #103019

[NO REVIEW] Update the CPUID and XSAVE logics for APX #103019

Conversation

Ruihan-Yin commented Jun 4, 2024 • edited Loading

[No need to review this PR for now]

tannergooding commented Jun 4, 2024

Ruihan-Yin commented Jun 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dotnet-policy-service bot commented Jul 5, 2024

Ruihan-Yin commented Jul 9, 2024

tannergooding commented Jul 9, 2024

Ruihan-Yin commented Jul 9, 2024

Ruihan-Yin commented Jun 4, 2024 •

edited

Loading