-
Notifications
You must be signed in to change notification settings - Fork 216
[NativeAOT-LLVM] Wasm managed threads - build new nupkg #3060
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.
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
Changes from 12 commits
494d4c5
46699a7
7fc1d34
e8e2de4
301c775
456f600
4099b10
c3ed443
3261466
900df40
8ccf2ec
d494b8e
06f4e2d
2672e75
3b7cfbc
ddf7606
9f681c6
384c77e
f52f534
3f6f0ed
0a72ac1
f39a0cb
539edc4
f49ca4d
521afe1
1b4daba
5be5163
44eba75
1e6d545
9073092
f9597f7
e47224c
024e938
9f213e9
c2c2c37
6483fb1
0a8d87e
1ecfb04
fae5d14
fb468b9
250b2a6
06dacf0
d6ccf55
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -65,6 +65,11 @@ if /i "%__Arch%" == "wasm" ( | |
set "WASI_SDK_PATH=!WASI_SDK_PATH:\=/!" | ||
if not "!WASI_SDK_PATH:~-1!" == "/" set "WASI_SDK_PATH=!WASI_SDK_PATH!/" | ||
set __CmakeGenerator=Ninja | ||
if defined __CMakeArgs ( | ||
if not "!__CMakeArgs:multithread=!" == "!__CMakeArgs!" ( | ||
set __ExtraCmakeParams=%__ExtraCmakeParams% -D_WASI_EMULATED_PTHREAD | ||
) | ||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Needed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. removed |
||
set __ExtraCmakeParams=%__ExtraCmakeParams% -DCLR_CMAKE_TARGET_OS=wasi -DCLR_CMAKE_TARGET_ARCH=wasm "-DWASI_SDK_PREFIX=!WASI_SDK_PATH!" "-DCMAKE_TOOLCHAIN_FILE=!WASI_SDK_PATH!/share/cmake/wasi-sdk-p2.cmake" "-DCMAKE_SYSROOT=!WASI_SDK_PATH!share/wasi-sysroot" "-DCMAKE_CROSSCOMPILING_EMULATOR=node --experimental-wasm-bigint --experimental-wasi-unstable-preview1" | ||
) | ||
) else ( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -489,6 +489,7 @@ jobs: | |
helixQueuesTemplate: ${{ parameters.helixQueuesTemplate }} | ||
variables: ${{ parameters.variables }} | ||
osGroup: wasi | ||
osSubGroup: ${{ parameters.osSubGroup }} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Needed (also below)? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have added it to some more naot wasm places. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, I meant that it is not clear it is needed... Since we already have a name suffix (the current name for the jobs has There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I went this way (OsSubGroup) is that is what I saw from mono. We could just use the namesuffix though, that would be fine with me, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed now.
yowl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
archType: wasm | ||
targetRid: wasi-wasm | ||
platform: wasi_wasm_win | ||
|
@@ -611,6 +612,7 @@ jobs: | |
helixQueuesTemplate: ${{ parameters.helixQueuesTemplate }} | ||
variables: ${{ parameters.variables }} | ||
osGroup: browser | ||
osSubGroup: ${{ parameters.osSubGroup }} | ||
archType: wasm | ||
targetRid: browser-wasm | ||
platform: browser_wasm_win | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -71,6 +71,28 @@ extends: | |
parameters: | ||
librariesConfiguration: Debug | ||
|
||
# | ||
# Build and test Wasm Debug multithreaded libraries and Debug runtime | ||
# | ||
- template: /eng/pipelines/common/platform-matrix.yml | ||
parameters: | ||
jobTemplate: /eng/pipelines/common/global-build-job.yml | ||
helixQueuesTemplate: /eng/pipelines/coreclr/templates/helix-queues-setup.yml | ||
buildConfig: debug | ||
osSubgroup: multithread | ||
yowl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
platforms: | ||
- browser_wasm_win | ||
- wasi_wasm_win | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How is this going to work with WASI, considering we don't have a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. removed. |
||
jobParameters: | ||
timeoutInMinutes: 300 | ||
buildArgs: -s clr.aot+libs -c debug -rc $(_BuildConfig) '/p:WasmEnableThreads=true' | ||
nameSuffix: Multithreaded | ||
postBuildSteps: | ||
- template: /eng/pipelines/runtimelab/runtimelab-post-build-steps.yml | ||
parameters: | ||
librariesConfiguration: Debug | ||
wasmEnableThreadsArg: /p:WasmEnableThreads=true | ||
|
||
# | ||
# Build and test with Debug libraries and Checked runtime | ||
# | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,7 @@ parameters: | |
runtimeVariant: '' | ||
isOfficialBuild: false | ||
librariesConfiguration: Debug | ||
wasmEnableThreadsArgg: '' | ||
yowl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
steps: | ||
# For NativeAOT-LLVM, we have just built the Wasm-targeting native artifacts (the runtime and libraries). | ||
|
@@ -17,21 +18,25 @@ steps: | |
displayName: Build the ILC and RyuJit cross-compilers | ||
|
||
# Build target packages (note: target libs already built). | ||
- script: $(Build.SourcesDirectory)/build$(scriptExt) nativeaot.packages -os ${{ parameters.osGroup }} -a wasm -c $(buildConfigUpper) $(_officialBuildParameter) -ci | ||
displayName: Build target packages | ||
- ${{ if ne(parameters.nameSuffix, 'Multithreaded') }}: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can use |
||
- script: $(Build.SourcesDirectory)/build$(scriptExt) nativeaot.packages -os ${{ parameters.osGroup }} -a wasm -c $(buildConfigUpper) $(_officialBuildParameter) -ci | ||
displayName: Build target packages | ||
- ${{ else }}: | ||
- script: $(Build.SourcesDirectory)/build$(scriptExt) nativeaot.packages -os ${{ parameters.osGroup }} -a wasm -c $(buildConfigUpper) $(_officialBuildParameter) -ci /p:WasmEnableThreads=true | ||
displayName: Build target multithread packages | ||
|
||
# Build host packages. | ||
- script: $(Build.SourcesDirectory)/build$(scriptExt) libs+nativeaot.packages -a $(hostedTargetArch) -c $(buildConfigUpper) -cross $(_officialBuildParameter) -ci | ||
displayName: Build host packages | ||
|
||
# Build coreclr native test output outside of official build | ||
- ${{ if ne(parameters.isOfficialBuild, true) }}: | ||
- ${{ if and(eq(parameters.archType, 'wasm'), ne(parameters.nameSuffix, '')) }}: | ||
- ${{ if and(eq(parameters.archType, 'wasm'), and(ne(parameters.nameSuffix, ''), ne(parameters.nameSuffix, 'Multithreaded'))) }}: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Better to use |
||
- script: pwsh $(Build.SourcesDirectory)/eng/pipelines/runtimelab/set-ilc-emulation-environment.ps1 -Arch $(hostedTargetArch) | ||
displayName: Set up ILC emulation environment | ||
|
||
- ${{ if eq(parameters.archType, 'wasm') }}: | ||
- script: $(Build.SourcesDirectory)/src/tests/build$(scriptExt) nativeaot $(buildConfigUpper) ${{ parameters.osGroup }} $(crossArg) $(_officialBuildParameter) ci tree nativeaot /p:LibrariesConfiguration=${{ parameters.librariesConfiguration }} | ||
- script: $(Build.SourcesDirectory)/src/tests/build$(scriptExt) nativeaot $(buildConfigUpper) ${{ parameters.osGroup }} $(crossArg) $(_officialBuildParameter) ci tree nativeaot /p:LibrariesConfiguration=${{ parameters.librariesConfiguration }} ${{ parameters.wasmEnableThreadsArg }} | ||
displayName: Build runtime tests | ||
- ${{ else }}: | ||
- ${{ if eq(parameters.osGroup, 'windows') }}: | ||
|
@@ -49,7 +54,7 @@ steps: | |
displayName: Run runtime tests | ||
|
||
# Don't compile/run the libraries tests with emulated ILC to save CI time/resources. | ||
- ${{ if and(eq(parameters.archType, 'wasm'), eq(parameters.nameSuffix, '')) }}: | ||
- ${{ if and(and(eq(parameters.archType, 'wasm'), eq(parameters.nameSuffix, '')), not(eq(parameters.nameSuffix, 'Multithreaded'))) }}: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto with |
||
- script: $(Build.SourcesDirectory)/build$(scriptExt) libs.tests -test -a ${{ parameters.archType }} -os ${{ parameters.osGroup }} -lc ${{ parameters.librariesConfiguration }} -rc $(buildConfigUpper) /p:TestNativeAot=true /p:RunSmokeTestsOnly=true | ||
displayName: Build and run WebAssembly libraries tests | ||
|
||
|
@@ -61,3 +66,4 @@ steps: | |
- template: /eng/pipelines/common/upload-intermediate-artifacts-step.yml | ||
parameters: | ||
name: ${{ parameters.platform }}${{ parameters.nameSuffix }} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -438,6 +438,7 @@ The .NET Foundation licenses this file to you under the MIT license. | |
<CompileWasmArgs Condition="'$(NativeDebugSymbols)' == 'true'">$(CompileWasmArgs) -g3</CompileWasmArgs> | ||
<CompileWasmArgs Condition="'$(WasmEnableNonTrappingFloatToIntConversions)' == 'true'">$(CompileWasmArgs) -mnontrapping-fptoint</CompileWasmArgs> | ||
<CompileWasmArgs Condition="'$(IlcLlvmExceptionHandlingModel)' == 'wasm'">$(CompileWasmArgs) -fwasm-exceptions</CompileWasmArgs> | ||
<CompileWasmArgs Condition="'$(WasmEnableThreads)' == 'true'">$(CompileWasmArgs) -pthread -matomics -mbulk-memory</CompileWasmArgs> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You also need to modify the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't think there were any triples for threads, at least not yet. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, I will comment it. The problem is that cmake fails for zlib-ng with pthread as the libc is not build with threads (atomics, bulk-memory) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just pthread is left now, thanks. |
||
</PropertyGroup> | ||
|
||
<PropertyGroup Condition="'$(_targetOS)' == 'browser'"> | ||
|
@@ -608,6 +609,7 @@ The .NET Foundation licenses this file to you under the MIT license. | |
<CustomLinkerArg Include="-s MAXIMUM_MEMORY=$(EmccMaximumHeapSize)" Condition="'$(EmccMaximumHeapSize)' != ''" /> | ||
<CustomLinkerArg Include="-s INITIAL_MEMORY=$(EmccInitialHeapSize)" Condition="'$(EmccInitialHeapSize)' != ''" /> | ||
<CustomLinkerArg Condition="'$(EmccEnableAssertions)' == 'true'" Include="-s ASSERTIONS=1" /> | ||
<CustomLinkerArg Condition="'$(WasmEnableThreads)' == 'true'" Include="-pthread" /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I saw earlier Emscripten gives some warning like "JS will be slow with memory growth + threads", seems we should disable it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. disabled growth and There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, I only meant to disable the warning. I don't think we can disable memory growth... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lol, I was thinking we would block users from running slow code. |
||
</ItemGroup> | ||
|
||
<!-- wasm-ld only supports listing exports on the command line --> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1169,6 +1169,8 @@ REDHAWK_PALEXPORT bool PalGetMaximumStackBounds(_Out_ void** ppStackLowOut, _Out | |
status = pthread_attr_get_np(thread, &attr); | ||
#elif HAVE_PTHREAD_GETATTR_NP | ||
status = pthread_getattr_np(thread, &attr); | ||
#elif defined(HOST_WASM) && defined(FEATURE_WASM_MANAGED_THREADS) | ||
// We dont have a pthread_getattr_np, but so far we don't need it. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we not? Could you test whether our Emscripten version already has a working version of it, it needs There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As we are not enabling a wasi build now we can remove this. |
||
#else | ||
#error Dont know how to get thread attributes on this platform! | ||
#endif | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -191,6 +191,186 @@ extern "C" int __cxa_thread_atexit(Dtor dtor, void* obj, void*) | |
#endif // TARGET_WASI | ||
#endif // !FEATURE_WASM_MANAGED_THREADS | ||
|
||
// TODO-LLVM: For now, a copy of the single threaded implementation. | ||
#ifdef FEATURE_WASM_MANAGED_THREADS | ||
int __cxa_thread_atexit(void (*func)(), void *obj, void *dso_symbol) | ||
{ | ||
return 0; | ||
} | ||
|
||
// | ||
// Note that we return the native stack bounds here, not shadow stack ones. Currently this functionality is mainly | ||
// used for RuntimeHelpers.TryEnsureSufficientExecutionStack, and we do use the native stack in codegen, so this | ||
// is an acceptable approximation. | ||
// | ||
extern "C" unsigned char __stack_low; | ||
extern "C" unsigned char __stack_high; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a lot of code copied here and it's not clear what part of it is needed and what part is there because WASI doesn't actually have There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. removed |
||
#ifdef TARGET_WASI | ||
// TODO-LLVM: No-op stubs, maybe when threads are implemented in WASI, we wont have to provide all of these. | ||
int pthread_mutex_init(pthread_mutex_t *, const pthread_mutexattr_t *) | ||
{ | ||
return 0; | ||
} | ||
|
||
int pthread_mutexattr_init(pthread_mutexattr_t *) | ||
{ | ||
return 0; | ||
} | ||
|
||
int pthread_mutexattr_settype(pthread_mutexattr_t *, int) | ||
{ | ||
return 0; | ||
} | ||
|
||
int pthread_mutex_destroy(pthread_mutex_t *) | ||
{ | ||
return 0; | ||
} | ||
|
||
int pthread_mutexattr_destroy(pthread_mutexattr_t *) | ||
{ | ||
return 0; | ||
} | ||
|
||
int pthread_cond_init(pthread_cond_t *, const pthread_condattr_t *) | ||
{ | ||
return 0; | ||
} | ||
|
||
int pthread_cond_destroy(pthread_cond_t *) | ||
{ | ||
return 0; | ||
} | ||
|
||
int pthread_cond_wait(pthread_cond_t *, pthread_mutex_t *) | ||
{ | ||
return 0; | ||
} | ||
|
||
int pthread_cond_timedwait(pthread_cond_t *, pthread_mutex_t *, const struct timespec *) | ||
{ | ||
return 0; | ||
} | ||
|
||
int pthread_condattr_init(pthread_condattr_t *) | ||
{ | ||
return 0; | ||
} | ||
|
||
int pthread_mutex_lock(pthread_mutex_t *) | ||
{ | ||
return 0; | ||
} | ||
|
||
int pthread_mutex_unlock(pthread_mutex_t *) | ||
{ | ||
return 0; | ||
} | ||
|
||
pthread_t pthread_self(void) | ||
{ | ||
return (pthread_t)0; | ||
} | ||
|
||
int pthread_equal(pthread_t, pthread_t) | ||
{ | ||
return 1; | ||
} | ||
|
||
int pthread_attr_init(pthread_attr_t *) | ||
{ | ||
// See https://github.com/emscripten-core/emscripten/pull/18057 and https://reviews.llvm.org/D135910. | ||
unsigned char* pStackLow = &__stack_low; | ||
unsigned char* pStackHigh = &__stack_high; | ||
|
||
// Sanity check that we have the expected memory layout. | ||
ASSERT((pStackHigh - pStackLow) >= 64 * 1024); | ||
if (pStackLow >= pStackHigh) | ||
{ | ||
PalPrintFatalError("\nFatal error. Unexpected stack layout.\n"); | ||
RhFailFast(); | ||
} | ||
|
||
return 0; | ||
} | ||
|
||
int pthread_attr_getstack(pthread_attr_t *, void **stackaddr, size_t *stacksize) | ||
{ | ||
unsigned char* pStackLow = &__stack_low; | ||
unsigned char* pStackHigh = &__stack_high; | ||
|
||
*stackaddr = pStackLow; | ||
*stacksize = pStackHigh - pStackLow; | ||
return 0; | ||
} | ||
|
||
int pthread_attr_destroy(pthread_attr_t *) | ||
{ | ||
return 0; | ||
} | ||
|
||
int pthread_condattr_destroy(pthread_condattr_t *) | ||
{ | ||
return 0; | ||
} | ||
|
||
int pthread_cond_broadcast(pthread_cond_t *) | ||
{ | ||
return 0; | ||
} | ||
|
||
int pthread_attr_setdetachstate(pthread_attr_t *, int) | ||
{ | ||
return 0; | ||
} | ||
|
||
using Dtor = void(*)(void*); | ||
|
||
// TODO=LLVM: This is a copy of the single threaded implementation. | ||
extern "C" int __cxa_thread_atexit(Dtor dtor, void* obj, void*) | ||
{ | ||
struct DtorList | ||
{ | ||
Dtor dtor; | ||
void* obj; | ||
DtorList* next; | ||
}; | ||
|
||
struct DtorsManager | ||
{ | ||
DtorList* m_dtors = nullptr; | ||
|
||
~DtorsManager() | ||
{ | ||
while (DtorList* head = m_dtors) | ||
{ | ||
m_dtors = head->next; | ||
head->dtor(head->obj); | ||
free(head); | ||
} | ||
} | ||
}; | ||
|
||
// The linked list of "thread-local" destructors to run. | ||
static DtorsManager s_dtorsManager; | ||
|
||
DtorList* head = static_cast<DtorList*>(malloc(sizeof(DtorList))); | ||
if (head == nullptr) | ||
{ | ||
return -1; | ||
} | ||
|
||
head->dtor = dtor; | ||
head->obj = obj; | ||
head->next = s_dtorsManager.m_dtors; | ||
s_dtorsManager.m_dtors = head; | ||
|
||
return 0; | ||
} | ||
#endif // TARGET_WASI | ||
#endif // FEATURE_WASM_MANAGED_THREADS | ||
|
||
// Recall that WASM's model is extremely simple: we have one linear memory, which can only be grown, in chunks | ||
// of 64K pages. Thus, "mmap"/"munmap" fundamentally cannot be faithfully recreated and the Unix emulators we | ||
// layer on top of (Emscripten/WASI libc) reflect this by not supporting the scenario. Fortunately, the current | ||
|
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -1,6 +1,8 @@ | ||||||||||
// Licensed to the .NET Foundation under one or more agreements. | ||||||||||
// The .NET Foundation licenses this file to you under the MIT license. | ||||||||||
|
||||||||||
#ifndef FEATURE_WASM_MANAGED_THREADS | ||||||||||
#ifdef FEATURE_WASM_MANAGED_THREADS | ||||||||||
int __cxa_thread_atexit(void (*func)(), void *obj, void *dso_symbol); | ||||||||||
#else | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Unused. |
||||||||||
void PalGetMaximumStackBounds_SingleThreadedWasm(void** ppStackLowOut, void** ppStackHighOut); | ||||||||||
#endif // !FEATURE_WASM_MANAGED_THREADS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or is it needed for some reason?