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

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added a comment.
Herald added a subscriber: ormris.

Nice!  lgtm with a couple of nits



================
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>`_.
 
----------------
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."


================
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
----------------
Perhaps this part could be enerat


================
Comment at: clang/test/Driver/wasm-toolchain.c:56
 
+// -fPIC should work for WASI
+
----------------
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?


================
Comment at: llvm/utils/lit/lit/llvm/config.py:347
+            return triple
+
         m = re.match(r"(\w+)-(\w+)-(\w+)", triple)
----------------
Are the the changes to this file meant to be part of this CL?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153293/new/

https://reviews.llvm.org/D153293