dicej added a comment. Thanks for the review. I'll post an update shortly.
================ Comment at: clang/docs/ReleaseNotes.rst:705 +- The `wasm32-wasi` target now supports `Emscripten-style shared libraries + <https://github.com/WebAssembly/tool-conventions/blob/main/DynamicLinking.md>`_. ---------------- sbc100 wrote: > Would it not make more sense to simply say that all WebAssembly targets now > support shared libraries and PIC code generation? Perhaps we could phrase > it something like this: > > "Shared library support (and PIC code generation) for is no longer limited to > the Emscripten target OS and now works with other targets such as > wasm32-wasi." Agreed! I'll update it. ================ Comment at: clang/test/Driver/wasm-toolchain.c:38 + +// RUN: %clang -### -shared --target=wasm32-wasi --sysroot=/foo %s 2>&1 \ +// RUN: | FileCheck -check-prefix=LINK_KNOWN_SHARED %s ---------------- sbc100 wrote: > Perhaps this part could be enerat Sorry, I'm having trouble parsing this; did your comment get cut off? ================ Comment at: clang/test/Driver/wasm-toolchain.c:56 +// -fPIC should work for WASI + ---------------- sbc100 wrote: > The test above say `with known OS`.. which is perhaps more appropriate here. > > Also, shouldn't all these also work (and be tested with) unknown OS? Sounds reasonable. I'll edit the comment and add tests for the unknown case. ================ Comment at: llvm/utils/lit/lit/llvm/config.py:347 + return triple + m = re.match(r"(\w+)-(\w+)-(\w+)", triple) ---------------- sbc100 wrote: > Are the the changes to this file meant to be part of this CL? The `check-clang` target doesn't work at all if the target "triple" is a double, e.g.: ``` llvm-lit: /Users/dicej/p/wasi-sdk/src/llvm-project/llvm/utils/lit/lit/llvm/config.py:459: note: using clang: /Users/dicej/p/wasi-sdk/build/llvm/bin/clang llvm-lit: /Users/dicej/p/wasi-sdk/src/llvm-project/llvm/utils/lit/lit/llvm/config.py:324: fatal: Could not turn 'wasm32-wasi' into Itanium ABI triple ``` @sunfish had been using this patch locally to work around the issue, so I figured I'd include the patch here so it stops being a stumbling block. Perhaps there's a better way to address it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153293/new/ https://reviews.llvm.org/D153293 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits