Skip to content

Unify hardware feature detection between CoreCLR JIT and AOT#89342

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
jkotas merged 2 commits into from
Jul 23, 2023
Merged

Unify hardware feature detection between CoreCLR JIT and AOT #89342

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
539b8e5
Unify hardware feature detection between CoreCLR JIT and AOT
jkotas Jul 20, 2023
9b35d6d
FB
jkotas Jul 22, 2023
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
1 change: 0 additions & 1 deletion src/coreclr/CMakeLists.txt
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -191,7 +191,6 @@ include_directories("debug/inc/${ARCH_SOURCES_DIR}")
include_directories("debug/inc/dump")
include_directories("md/inc")
include_directories("classlibnative/bcltype")
include_directories("classlibnative/cryptography")
include_directories("classlibnative/inc")
include_directories("${GENERATED_INCLUDE_DIR}")
include_directories("hosts/inc")
Expand Down
4 changes: 1 addition & 3 deletions src/coreclr/classlibnative/bcltype/system.cpp
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -30,7 +30,7 @@
#include "array.h"
#include "eepolicy.h"


#include <minipal/cpuid.h>


FCIMPL0(UINT32, SystemNative::GetTickCount)
Expand All@@ -50,8 +50,6 @@ FCIMPL0(UINT64, SystemNative::GetTickCount64)
FCIMPLEND;




extern "C" VOID QCALLTYPE Environment_Exit(INT32 exitcode)
{
QCALL_CONTRACT;
Expand Down
24 changes: 1 addition & 23 deletions src/coreclr/gc/vxsort/isa_detection.cpp
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -19,28 +19,6 @@ enum class SupportedISA

#if defined(TARGET_AMD64) && defined(TARGET_WINDOWS)

static DWORD64 GetEnabledXStateFeaturesHelper()
{
// On Windows we have an api(GetEnabledXStateFeatures) to check if AVX is supported
typedef DWORD64(WINAPI* PGETENABLEDXSTATEFEATURES)();
PGETENABLEDXSTATEFEATURES pfnGetEnabledXStateFeatures = NULL;

HMODULE hMod = LoadLibraryExW(L"kernel32.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
if (hMod == NULL)
return 0;

pfnGetEnabledXStateFeatures = (PGETENABLEDXSTATEFEATURES)GetProcAddress(hMod, "GetEnabledXStateFeatures");

@jkotasjkotasJul 22, 2023

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

runtime/src/coreclr/vm/threadsuspend.cpp

Line 1972 in 08a6e06

DWORD64 FeatureMask = GetEnabledXStateFeatures();
links to GetEnabledXStateFeatures directly, so it is safe to delete the dynamic linking here.

@tannergoodingtannergoodingJul 22, 2023

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.

It's worth noting that the original reason for the dynamic linking was to support Windows 7 prior to SP1.

With Win7 support having been dropped and us just not regressing Win 7 SP1 and later, this is fine.


if (pfnGetEnabledXStateFeatures == NULL)
{
return 0;
}

DWORD64 FeatureMask = pfnGetEnabledXStateFeatures();

return FeatureMask;
}

SupportedISA DetermineSupportedISA()
{
// register definitions to make the following code more readable
Expand DownExpand Up@@ -78,7 +56,7 @@ SupportedISA DetermineSupportedISA()
DWORD64 xcr0 = _xgetbv(0);

// get OS XState info
DWORD64 FeatureMask = GetEnabledXStateFeaturesHelper();
DWORD64 FeatureMask = GetEnabledXStateFeatures();

// get processor extended feature flag info
__cpuidex(reg, 7, 0);
Expand Down
17 changes: 15 additions & 2 deletions src/coreclr/minipal/Unix/CMakeLists.txt
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,18 @@
add_library(coreclrminipal
STATIC
set(SOURCES
doublemapping.cpp
dn-u16.cpp
)

if(NOT CLR_CROSS_COMPONENTS_BUILD)
list(APPEND SOURCES
${CLR_SRC_NATIVE_DIR}/minipal/cpufeatures.c
)
endif()

add_library(coreclrminipal
STATIC
${SOURCES}
)

include(${CLR_SRC_NATIVE_DIR}/minipal/configure.cmake)
include_directories(${CMAKE_CURRENT_BINARY_DIR})
14 changes: 12 additions & 2 deletions src/coreclr/minipal/Windows/CMakeLists.txt
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,15 @@
add_library(coreclrminipal
STATIC
set(SOURCES
doublemapping.cpp
dn-u16.cpp
)

if(NOT CLR_CROSS_COMPONENTS_BUILD)
list(APPEND SOURCES
${CLR_SRC_NATIVE_DIR}/minipal/cpufeatures.c
)
endif()

add_library(coreclrminipal
STATIC
${SOURCES}
)
99 changes: 0 additions & 99 deletions src/coreclr/nativeaot/Runtime/amd64/MemClrForGC.asm
View file
Open in desktop

This file was deleted.

148 changes: 0 additions & 148 deletions src/coreclr/nativeaot/Runtime/i386/MemClrForGC.asm
View file
Open in desktop

This file was deleted.

Loading