Skip to content
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

Jump to bottom

tweaks to enable NativeAOT-LLVM on Linux with WASI-SDK 22 #2592

Merged
merged 2 commits into from
Jun 4, 2024
Merged

tweaks to enable NativeAOT-LLVM on Linux with WASI-SDK 22 #2592

merged 2 commits into from
Jun 4, 2024

Conversation

dicej
Copy link

@dicej dicej commented May 23, 2024

Some of these changes are borrowed from #2569.

Note that I had to manually copy pthread.h from the wasi-sdk-22/share/wasi-sysroot/include/wasm32-wasi-threads directory to the wasi-sdk-22/share/wasi-sysroot/include/wasm32-wasi directory as a workaround until WebAssembly/wasi-libc#501 is addressed.

@dicej dicej marked this pull request as ready for review May 23, 2024 19:08
@dicej
Copy link
Author

dicej commented May 23, 2024

@dotnet-policy-service agree company="Fermyon Technologies"

@jkotas jkotas added the area-NativeAOT-LLVM LLVM generation for Native AOT compilation (including Web Assembly) label May 23, 2024
@jkotas
Copy link
Member

jkotas commented May 23, 2024

cc @dotnet/nativeaot-llvm

SingleAccretion
SingleAccretion reviewed May 24, 2024
View reviewed changes
Copy link

@SingleAccretion SingleAccretion left a comment

Choose a reason for hiding this comment

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

The pthread situation will need to be resolved, s.t. an unmodified SDK can be used, by pulling the stubs from PalRedhawkWasm.cpp to PalRedhawkWasm.h.

eng/native/gen-buildsys.sh Outdated Show resolved Hide resolved
src/coreclr/build-runtime.sh Outdated Show resolved Hide resolved
src/coreclr/jit/CMakeLists.txt Outdated Show resolved Hide resolved
src/coreclr/jit/llvmcodegen.cpp Outdated Show resolved Hide resolved
src/native/libs/CMakeLists.txt Outdated Show resolved Hide resolved
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets Show resolved Hide resolved
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets Outdated Show resolved Hide resolved
@dicej
Copy link
Author

dicej commented May 24, 2024

Thanks for the detailed feedback, @SingleAccretion. I'm planning to address it when I get a chance early next week.

yowl
yowl reviewed May 26, 2024
View reviewed changes
eng/native/gen-buildsys.sh Outdated Show resolved Hide resolved
@dicej
Copy link
Author

dicej commented May 28, 2024

The pthread situation will need to be resolved, s.t. an unmodified SDK can be used, by pulling the stubs from PalRedhawkWasm.cpp to PalRedhawkWasm.h.

I tried pursuing that, but it got ugly fast. I had to edit code under e.g. src/coreclr/gc and sprinkle new #ifdefs in a lot of places, which felt like overkill for a temporary workaround.

Then I took a step back and decided to add the following to install-wasi-sdk.ps1:

# Temporary WASI-SDK 22 workaround: Until
# https://github.com/WebAssembly/wasi-libc/issues/501 is addressed, we copy
# pthread.h from the wasm32-wasi-threads include directory to the wasm32-wasi
# include directory:

cp wasi-sdk/share/wasi-sysroot/include/wasm32-wasi-threads/pthread.h wasi-sdk/share/wasi-sysroot/include/wasm32-wasi/

If that's not acceptable, I'd vote we stick with WASI-SDK 21 for the time being, then upgrade to WASI-SDK 23, which should include a fix for WebAssembly/wasi-libc#501.

@dicej
Copy link
Author

dicej commented May 28, 2024

FYI, this should address the missing pthread.h issue, once merged and included in a release: WebAssembly/wasi-libc#504

SingleAccretion
SingleAccretion reviewed May 28, 2024
View reviewed changes
src/coreclr/jit/CMakeLists.txt Outdated Show resolved Hide resolved
src/coreclr/jit/CMakeLists.txt Outdated Show resolved Hide resolved
eng/pipelines/runtimelab/install-wasi-sdk.ps1 Show resolved Hide resolved
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets Outdated Show resolved Hide resolved
eng/native/gen-buildsys.sh Outdated Show resolved Hide resolved
@dicej dicej mentioned this pull request May 29, 2024
Open
@dicej
Copy link
Author

dicej commented May 29, 2024

Hm, I see cl.exe is not happy with the #pragma clang ... directives I added to suppress warnings in llvmlssa.cpp. Is there a better way to approach that?

@SingleAccretion
Copy link

Is there a better way to approach that?

It would be ideal if there was a way to propagate the warning upwards, but it looks like there isn't... Presumably, the directives can be guarded with #ifdef __clang__.

SingleAccretion
SingleAccretion approved these changes May 30, 2024
View reviewed changes
Copy link

@SingleAccretion SingleAccretion left a comment

Choose a reason for hiding this comment

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

LGTM modulo one comment and assuming green CI. Thank you!

src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets Outdated Show resolved Hide resolved
@jkotas
Copy link
Member

jkotas commented May 31, 2024

Could you please resolve the conflict?

@dicej
tweaks to enable NativeAOT-LLVM on Linux with WASI-SDK 22
df0f98d 
Some of these changes are borrowed from
dotnet#2569.

Note that I had to manually copy pthread.h from the
wasi-sdk-22/share/wasi-sysroot/include/wasm32-wasi-threads directory to the
wasi-sdk-22/share/wasi-sysroot/include/wasm32-wasi directory as a workaround
until WebAssembly/wasi-libc#501 is addressed.

Signed-off-by: Joel Dice <joel.dice@fermyon.com>
@dicej @yowl
Restrict WASI SDK version check to WASI target
b7e0f94 
Co-authored-by: yowl <scott.waye@hubse.com>
@dicej
Copy link
Author

dicej commented Jun 4, 2024

I've resolved the conflict and everything's green. Are we good to merge this?

@jkotas
Copy link
Member

jkotas commented Jun 4, 2024

Thanks!

@jkotas jkotas merged commit 658d777 into dotnet:feature/NativeAOT-LLVM Jun 4, 2024
11 checks passed
This was referenced Jun 5, 2024
Closed
Closed
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yowl yowl yowl left review comments

@SingleAccretion SingleAccretion SingleAccretion approved these changes

Assignees
No one assigned
Labels
area-NativeAOT-LLVM LLVM generation for Native AOT compilation (including Web Assembly)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dicej @jkotas @SingleAccretion @yowl