[PATCH] D153293: [clang][WebAssembly] support wasm32-wasi shared libraries

[Joel Dice via Phabricator via cfe-commits]

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