[PATCH] D159383: [Headers] Remove musl-related comment about NULL

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

I believe wasi-sdk doesn't have this issue, because it has modifications to all 
headers which define NULL to always use the NULL defined in stddef.h, and it 
doesn't ship musl's stddef.h.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159383

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

This patch appears to be just the release notes and the test; is the actual 
driver change missing?




Comment at: clang/docs/ReleaseNotes.rst:706
+  limited to the Emscripten target OS and now works with other targets such as
+  wasm32-wasi.
 

We should mention that this format is not yet stable and may change between 
LLVM versions, and that WASI does not yet have any facilities for loading 
dynamic libraries.


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

[PATCH] D150803: Add a new `wasm_custom` clang attribute for marking functions.

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

In D150803#4419436 , @sbc100 wrote:

> Did you mean to comment on the old PR?  This new PR doesn't propose either 
> "jspi" or "async", but adds that ability to define custom attributes.. since 
> that was deemed more flexible and forward thinking.   The decision as to 
> which name emscripten will use for what can then be part of  different 
> discussion/PR we hope.

Ah, I think I somehow was looking at the old PR, and thought there was a test 
with `custom_section.func_attr.async` in it.

This patch looks ok, though my initial impression is that if we're going to 
have a fully general-purpose mechanism like this, we might want to plan ahead 
for attributes that have parameters. We don't need to implement it now though; 
if there's a reasonable way to evolve in that direction, then this LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150803

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150803: Add a new `wasm_custom` clang attribute for marking functions.

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

Would it make sense to call this "jspi" in practice, rather than "async"? Even 
though this isn't clang's decision per se anymore, it seems like the reasoning 
earlier in this review would still apply.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150803

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150803: [WebAssembly] Add a new `wasm_async` clang attribute for marking async functions.

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

In D150803#4401204 , @sbc100 wrote:

> In D150803#4401061 , @sunfish wrote:
>
>> wasm_jspi works for me.
>
> The problem with that is that we will also likely want to use this attribute 
> for asyncify (which is the current async approach we have today) as well as 
> JSPI (which is hopefully coming soon)

Ah, I see. Would it be accurate to say that asyncify is acting as a kind of a 
polyfill for JSPI in this situation, such that `wasm_jspi` is actually correct?

Alternatively, what would you think about `wasm_external_async`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150803

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150803: [WebAssembly] Add a new `wasm_async` clang attribute for marking async functions.

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

wasm_jspi works for me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150803

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150803: [WebAssembly] Add a new `wasm_async` clang attribute for marking async functions.

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

In D150803#4389040 , @dschuff wrote:

> Hm, this is interesting because in the long term we plan to have stack 
> switching in wasm, which could allow for similar async behavior that JSPI 
> has, and could be useful in non-web systems. But that's a ways off. The file 
> format we are generating with this CL will be used in emscripten sooner (and 
> we may want to try to stabilize it some point, possibly before pure wasm 
> stack switching is usable in non-web systems).
> So overall I kind of feel like I could go either way on this. Curious if 
> @sunfish has had any thoughts about async outside of emscripten.

When we do have stack switching, would you anticipate still using this 
`wasm_async` attribute, or would we switch to something else at that point?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150803

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125728: [WebAssembly] Update supported features in -mcpu=generic

[Dan Gohman via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1e4e2433bcd1: [WebAssembly] Update supported features in the 
generic CPU configuration (authored by sunfish).

Changed prior to commit:
  https://reviews.llvm.org/D125728?vs=431067=470584#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125728

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Basic/Targets/WebAssembly.cpp
  clang/test/Driver/wasm-features.c


Index: clang/test/Driver/wasm-features.c
===
--- /dev/null
+++ clang/test/Driver/wasm-features.c
@@ -0,0 +1,43 @@
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -fsyntax-only 2>&1 | 
FileCheck %s
+
+// CHECK: "-fvisibility=hidden"
+
+// RUN: %clang --target=wasm32-unknown-unknown -### %s 2>&1 | FileCheck %s 
-check-prefix=DEFAULT
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -mcpu=mvp 2>&1 | 
FileCheck %s -check-prefix=MVP
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -mcpu=bleeding-edge 
2>&1 | FileCheck %s -check-prefix=BLEEDING-EDGE
+
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -mbulk-memory 2>&1 | 
FileCheck %s -check-prefix=BULK-MEMORY
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -mno-bulk-memory 2>&1 | 
FileCheck %s -check-prefix=NO-BULK-MEMORY
+
+// BULK-MEMORY: "-target-feature" "+bulk-memory"
+// NO-BULK-MEMORY: "-target-feature" "-bulk-memory"
+// DEFAULT-NOT: "-target-feature" "-bulk-memory"
+// MVP-NOT: "-target-feature" "+bulk-memory"
+// BLEEDING-EDGE-NOT: "-target-feature" "-bulk-memory"
+
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -mmutable-globals 2>&1 
| FileCheck %s -check-prefix=MUTABLE-GLOBALS
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -mno-mutable-globals 
2>&1 | FileCheck %s -check-prefix=NO-MUTABLE-GLOBALS
+
+// MUTABLE-GLOBALS: "-target-feature" "+mutable-globals"
+// NO-MUTABLE-GLOBALS: "-target-feature" "-mutable-globals"
+// DEFAULT-NOT: "-target-feature" "-mutable-globals"
+// MVP-NOT: "-target-feature" "+mutable-globals"
+// BLEEDING-EDGE-NOT: "-target-feature" "-mutable-globals"
+
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -msign-ext 2>&1 | 
FileCheck %s -check-prefix=SIGN-EXT
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -mno-sign-ext 2>&1 | 
FileCheck %s -check-prefix=NO-SIGN-EXT
+
+// SIGN-EXT: "-target-feature" "+sign-ext"
+// NO-SIGN-EXT: "-target-feature" "-sign-ext"
+// DEFAULT-NOT: "-target-feature" "-sign-ext"
+// MVP-NOT: "-target-feature" "+sign-ext"
+// BLEEDING-EDGE-NOT: "-target-feature" "-sign-ext"
+
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -mnontrapping-fptoint 
2>&1 | FileCheck %s -check-prefix=NONTRAPPING-FPTOINT
+// RUN: %clang --target=wasm32-unknown-unknown -### %s 
-mno-nontrapping-fptoint 2>&1 | FileCheck %s 
-check-prefix=NO-NONTRAPPING-FPTOINT
+
+// NONTRAPPING-FPTOINT: "-target-feature" "+nontrapping-fptoint"
+// NO-NONTRAPPING-FPTOINT: "-target-feature" "-nontrapping-fptoint"
+// DEFAULT-NOT: "-target-feature" "-nontrapping-fptoint"
+// MVP-NOT: "-target-feature" "+nontrapping-fptoint"
+// BLEEDING-EDGE-NOT: "-target-feature" "-nontrapping-fptoint"
Index: clang/lib/Basic/Targets/WebAssembly.cpp
===
--- clang/lib/Basic/Targets/WebAssembly.cpp
+++ clang/lib/Basic/Targets/WebAssembly.cpp
@@ -147,6 +147,9 @@
 Features["mutable-globals"] = true;
 Features["tail-call"] = true;
 setSIMDLevel(Features, SIMD128, true);
+  } else if (CPU == "generic") {
+Features["sign-ext"] = true;
+Features["mutable-globals"] = true;
   }
 
   return TargetInfo::initFeatureMap(Features, Diags, CPU, FeaturesVec);
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -607,6 +607,12 @@
   * Support intrinsic of ``__cmpccxadd_epi32``.
   * Support intrinsic of ``__cmpccxadd_epi64``.
 
+WebAssembly Support in Clang
+
+
+The -mcpu=generic configuration now enables sign-ext and mutable-globals. These
+proposals are standardized and available in all major engines.
+
 DWARF Support in Clang
 --
 


Index: clang/test/Driver/wasm-features.c
===
--- /dev/null
+++ clang/test/Driver/wasm-features.c
@@ -0,0 +1,43 @@
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -fsyntax-only 2>&1 | FileCheck %s
+
+// CHECK: "-fvisibility=hidden"
+
+// RUN: %clang --target=wasm32-unknown-unknown -### %s 2>&1 | FileCheck %s -check-prefix=DEFAULT
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -mcpu=mvp 2>&1 | FileCheck %s -check-prefix=MVP
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -mcpu=bleeding-edge 

[PATCH] D75277: [WebAssembly] Remove restriction on main name mangling

[Dan Gohman via Phabricator via cfe-commits]

sunfish added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6360
+  auto *GA = llvm::GlobalAlias::create("__main_void", F);
+  GA->setVisibility(F->getVisibility());
+}

sbc100 wrote:
> sunfish wrote:
> > Can this use `GA->setVisibility(GlobalValue::HiddenVisibility)`? 
> > `__main_void` is an implementation detail, so in theory users shouldn't be 
> > setting interesting visibility attributes on it.
> The caller main / __main_void could live in different shared library the main 
> function itself.  
> 
> IIRC the caller of `main` is normally something like crt1.o and it should be 
> able to see `__main_void` if, and only if, it can see `main`.  i.e. if should 
> have the same visibility as main itself, no?
Typically crt1.o is linked into the same exe/dso as `main`, so it needs `main` 
to have external linkage, but it doesn't need `main` to have any particular 
visibility.

In Emscripten, is `main` being called from a different dso, or from JS 
following visibility rules for different dsos?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75277

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75277: [WebAssembly] Remove restriction on main name mangling

[Dan Gohman via Phabricator via cfe-commits]

sunfish added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6360
+  auto *GA = llvm::GlobalAlias::create("__main_void", F);
+  GA->setVisibility(F->getVisibility());
+}

Can this use `GA->setVisibility(GlobalValue::HiddenVisibility)`? `__main_void` 
is an implementation detail, so in theory users shouldn't be setting 
interesting visibility attributes on it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75277

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75277: [WebAssembly] Remove restriction on main name mangling

[Dan Gohman via Phabricator via cfe-commits]

sunfish added inline comments.



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyFixFunctionBitcasts.cpp:264
-  }
-}
   }

Why is this code going away? This isn't Emscripten-specific code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75277

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125728: [WebAssembly] Update supported features in -mcpu=generic

[Dan Gohman via Phabricator via cfe-commits]

sunfish updated this revision to Diff 431067.
sunfish added a comment.

Add tests for -mcpu=mvp and -mcpu=bleeding-edge.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125728

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Basic/Targets/WebAssembly.cpp
  clang/test/Driver/wasm-features.c


Index: clang/test/Driver/wasm-features.c
===
--- clang/test/Driver/wasm-features.c
+++ clang/test/Driver/wasm-features.c
@@ -0,0 +1,43 @@
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -fsyntax-only 2>&1 | 
FileCheck %s
+
+// CHECK: "-fvisibility" "hidden"
+
+// RUN: %clang --target=wasm32-unknown-unknown -### %s 2>&1 | FileCheck %s 
-check-prefix=DEFAULT
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -mcpu=mvp 2>&1 | 
FileCheck %s -check-prefix=MVP
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -mcpu=bleeding-edge 
2>&1 | FileCheck %s -check-prefix=BLEEDING-EDGE
+
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -mbulk-memory 2>&1 | 
FileCheck %s -check-prefix=BULK-MEMORY
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -mno-bulk-memory 2>&1 | 
FileCheck %s -check-prefix=NO-BULK-MEMORY
+
+// BULK-MEMORY: "-target-feature" "+bulk-memory"
+// NO-BULK-MEMORY: "-target-feature" "-bulk-memory"
+// DEFAULT-NOT: "-target-feature" "-bulk-memory"
+// MVP-NOT: "-target-feature" "+bulk-memory"
+// BLEEDING-EDGE-NOT: "-target-feature" "-bulk-memory"
+
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -mmutable-globals 2>&1 
| FileCheck %s -check-prefix=MUTABLE-GLOBALS
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -mno-mutable-globals 
2>&1 | FileCheck %s -check-prefix=NO-MUTABLE-GLOBALS
+
+// MUTABLE-GLOBALS: "-target-feature" "+mutable-globals"
+// NO-MUTABLE-GLOBALS: "-target-feature" "-mutable-globals"
+// DEFAULT-NOT: "-target-feature" "-mutable-globals"
+// MVP-NOT: "-target-feature" "+mutable-globals"
+// BLEEDING-EDGE-NOT: "-target-feature" "-mutable-globals"
+
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -msign-ext 2>&1 | 
FileCheck %s -check-prefix=SIGN-EXT
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -mno-sign-ext 2>&1 | 
FileCheck %s -check-prefix=NO-SIGN-EXT
+
+// SIGN-EXT: "-target-feature" "+sign-ext"
+// NO-SIGN-EXT: "-target-feature" "-sign-ext"
+// DEFAULT-NOT: "-target-feature" "-sign-ext"
+// MVP-NOT: "-target-feature" "+sign-ext"
+// BLEEDING-EDGE-NOT: "-target-feature" "-sign-ext"
+
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -mnontrapping-fptoint 
2>&1 | FileCheck %s -check-prefix=NONTRAPPING-FPTOINT
+// RUN: %clang --target=wasm32-unknown-unknown -### %s 
-mno-nontrapping-fptoint 2>&1 | FileCheck %s 
-check-prefix=NO-NONTRAPPING-FPTOINT
+
+// NONTRAPPING-FPTOINT: "-target-feature" "+nontrapping-fptoint"
+// NO-NONTRAPPING-FPTOINT: "-target-feature" "-nontrapping-fptoint"
+// DEFAULT-NOT: "-target-feature" "-nontrapping-fptoint"
+// MVP-NOT: "-target-feature" "+nontrapping-fptoint"
+// BLEEDING-EDGE-NOT: "-target-feature" "-nontrapping-fptoint"
Index: clang/lib/Basic/Targets/WebAssembly.cpp
===
--- clang/lib/Basic/Targets/WebAssembly.cpp
+++ clang/lib/Basic/Targets/WebAssembly.cpp
@@ -147,6 +147,11 @@
 Features["mutable-globals"] = true;
 Features["tail-call"] = true;
 setSIMDLevel(Features, SIMD128, true);
+  } else if (CPU == "generic") {
+Features["nontrapping-fptoint"] = true;
+Features["sign-ext"] = true;
+Features["bulk-memory"] = true;
+Features["mutable-globals"] = true;
   }
 
   return TargetInfo::initFeatureMap(Features, Diags, CPU, FeaturesVec);
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -437,6 +437,13 @@
 X86 Support in Clang
 
 
+WebAssembly Support in Clang
+
+
+The -mcpu=generic configuration now enables nontrapping-fptoint, sign-ext,
+bulk-memory, and mutable-globals. These proposals are standardized and
+available in all major engines.
+
 DWARF Support in Clang
 --
 


Index: clang/test/Driver/wasm-features.c
===
--- clang/test/Driver/wasm-features.c
+++ clang/test/Driver/wasm-features.c
@@ -0,0 +1,43 @@
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -fsyntax-only 2>&1 | FileCheck %s
+
+// CHECK: "-fvisibility" "hidden"
+
+// RUN: %clang --target=wasm32-unknown-unknown -### %s 2>&1 | FileCheck %s -check-prefix=DEFAULT
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -mcpu=mvp 2>&1 | FileCheck %s -check-prefix=MVP
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -mcpu=bleeding-edge 2>&1 | FileCheck %s -check-prefix=BLEEDING-EDGE
+
+// RUN: %clang --target=wasm32-unknown-unknown -### %s 

[PATCH] D125728: [WebAssembly] Update supported features in -mcpu=generic

[Dan Gohman via Phabricator via cfe-commits]

sunfish updated this revision to Diff 431064.
sunfish added a comment.

Add a driver test, and add release notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125728

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Basic/Targets/WebAssembly.cpp
  clang/test/Driver/wasm-features.c


Index: clang/test/Driver/wasm-features.c
===
--- clang/test/Driver/wasm-features.c
+++ clang/test/Driver/wasm-features.c
@@ -0,0 +1,33 @@
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -fsyntax-only 2>&1 | 
FileCheck %s
+
+// CHECK: "-fvisibility" "hidden"
+
+// RUN: %clang --target=wasm32-unknown-unknown -### %s 2>&1 | FileCheck %s 
-check-prefix=DEFAULT
+
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -mbulk-memory 2>&1 | 
FileCheck %s -check-prefix=BULK-MEMORY
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -mno-bulk-memory 2>&1 | 
FileCheck %s -check-prefix=NO-BULK-MEMORY
+
+// BULK-MEMORY: "-target-feature" "+bulk-memory"
+// NO-BULK-MEMORY: "-target-feature" "-bulk-memory"
+// DEFAULT-NOT: "-target-feature" "-bulk-memory"
+
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -mmutable-globals 2>&1 
| FileCheck %s -check-prefix=MUTABLE-GLOBALS
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -mno-mutable-globals 
2>&1 | FileCheck %s -check-prefix=NO-MUTABLE-GLOBALS
+
+// MUTABLE-GLOBALS: "-target-feature" "+mutable-globals"
+// NO-MUTABLE-GLOBALS: "-target-feature" "-mutable-globals"
+// DEFAULT-NOT: "-target-feature" "-mutable-globals"
+
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -msign-ext 2>&1 | 
FileCheck %s -check-prefix=SIGN-EXT
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -mno-sign-ext 2>&1 | 
FileCheck %s -check-prefix=NO-SIGN-EXT
+
+// SIGN-EXT: "-target-feature" "+sign-ext"
+// NO-SIGN-EXT: "-target-feature" "-sign-ext"
+// DEFAULT-NOT: "-target-feature" "-sign-ext"
+
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -mnontrapping-fptoint 
2>&1 | FileCheck %s -check-prefix=NONTRAPPING-FPTOINT
+// RUN: %clang --target=wasm32-unknown-unknown -### %s 
-mno-nontrapping-fptoint 2>&1 | FileCheck %s 
-check-prefix=NO-NONTRAPPING-FPTOINT
+
+// NONTRAPPING-FPTOINT: "-target-feature" "+nontrapping-fptoint"
+// NO-NONTRAPPING-FPTOINT: "-target-feature" "-nontrapping-fptoint"
+// DEFAULT-NOT: "-target-feature" "-nontrapping-fptoint"
Index: clang/lib/Basic/Targets/WebAssembly.cpp
===
--- clang/lib/Basic/Targets/WebAssembly.cpp
+++ clang/lib/Basic/Targets/WebAssembly.cpp
@@ -147,6 +147,11 @@
 Features["mutable-globals"] = true;
 Features["tail-call"] = true;
 setSIMDLevel(Features, SIMD128, true);
+  } else if (CPU == "generic") {
+Features["nontrapping-fptoint"] = true;
+Features["sign-ext"] = true;
+Features["bulk-memory"] = true;
+Features["mutable-globals"] = true;
   }
 
   return TargetInfo::initFeatureMap(Features, Diags, CPU, FeaturesVec);
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -437,6 +437,13 @@
 X86 Support in Clang
 
 
+WebAssembly Support in Clang
+
+
+The -mcpu=generic configuration now enables nontrapping-fptoint, sign-ext,
+bulk-memory, and mutable-globals. These proposals are standardized and
+available in all major engines.
+
 DWARF Support in Clang
 --
 


Index: clang/test/Driver/wasm-features.c
===
--- clang/test/Driver/wasm-features.c
+++ clang/test/Driver/wasm-features.c
@@ -0,0 +1,33 @@
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -fsyntax-only 2>&1 | FileCheck %s
+
+// CHECK: "-fvisibility" "hidden"
+
+// RUN: %clang --target=wasm32-unknown-unknown -### %s 2>&1 | FileCheck %s -check-prefix=DEFAULT
+
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -mbulk-memory 2>&1 | FileCheck %s -check-prefix=BULK-MEMORY
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -mno-bulk-memory 2>&1 | FileCheck %s -check-prefix=NO-BULK-MEMORY
+
+// BULK-MEMORY: "-target-feature" "+bulk-memory"
+// NO-BULK-MEMORY: "-target-feature" "-bulk-memory"
+// DEFAULT-NOT: "-target-feature" "-bulk-memory"
+
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -mmutable-globals 2>&1 | FileCheck %s -check-prefix=MUTABLE-GLOBALS
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -mno-mutable-globals 2>&1 | FileCheck %s -check-prefix=NO-MUTABLE-GLOBALS
+
+// MUTABLE-GLOBALS: "-target-feature" "+mutable-globals"
+// NO-MUTABLE-GLOBALS: "-target-feature" "-mutable-globals"
+// DEFAULT-NOT: "-target-feature" "-mutable-globals"
+
+// RUN: %clang --target=wasm32-unknown-unknown -### %s -msign-ext 2>&1 | FileCheck %s -check-prefix=SIGN-EXT
+// RUN: 

[PATCH] D125728: [WebAssembly] Update supported features in -mcpu=generic

[Dan Gohman via Phabricator via cfe-commits]

sunfish created this revision.
Herald added subscribers: pmatos, asb, ecnelises, jgravelle-google, sbc100, 
dschuff.
Herald added a project: All.
sunfish requested review of this revision.
Herald added a subscriber: aheejin.
Herald added a project: clang.

Enable nontrapping-fptoint, sign-ext, bulk-memory, and mutable-globals in 
-mcpu=generic. This makes these features enabled by default.

These features are all [finished proposals], and all major wasm engines support 
them.

[finished proposals]: 
https://github.com/WebAssembly/proposals/blob/main/finished-proposals.md


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125728

Files:
  clang/lib/Basic/Targets/WebAssembly.cpp


Index: clang/lib/Basic/Targets/WebAssembly.cpp
===
--- clang/lib/Basic/Targets/WebAssembly.cpp
+++ clang/lib/Basic/Targets/WebAssembly.cpp
@@ -144,6 +144,11 @@
 Features["mutable-globals"] = true;
 Features["tail-call"] = true;
 setSIMDLevel(Features, SIMD128, true);
+  } else if (CPU == "generic") {
+Features["nontrapping-fptoint"] = true;
+Features["sign-ext"] = true;
+Features["bulk-memory"] = true;
+Features["mutable-globals"] = true;
   }
 
   return TargetInfo::initFeatureMap(Features, Diags, CPU, FeaturesVec);


Index: clang/lib/Basic/Targets/WebAssembly.cpp
===
--- clang/lib/Basic/Targets/WebAssembly.cpp
+++ clang/lib/Basic/Targets/WebAssembly.cpp
@@ -144,6 +144,11 @@
 Features["mutable-globals"] = true;
 Features["tail-call"] = true;
 setSIMDLevel(Features, SIMD128, true);
+  } else if (CPU == "generic") {
+Features["nontrapping-fptoint"] = true;
+Features["sign-ext"] = true;
+Features["bulk-memory"] = true;
+Features["mutable-globals"] = true;
   }
 
   return TargetInfo::initFeatureMap(Features, Diags, CPU, FeaturesVec);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121327: Lower `@llvm.global_dtors` using `__cxa_atexit` on MachO

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

In D121327#3381123 , @yln wrote:

> @sunfish 
> Hi Dan, I hope you are still happy with this change.  I didn't change any 
> WebAssembly tests, but rather added a new IR-level test, so all existing 
> WebAssembly behavior should stay the same.  Let me know if you have any 
> concerns.

Yes, the change looks good to me!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121327

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105749: WebAssembly: Update datalayout to match fp128 ABI change

[Dan Gohman via Phabricator via cfe-commits]

sunfish added inline comments.



Comment at: llvm/test/CodeGen/WebAssembly/varargs.ll:5
 
-target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
-target triple = "wasm32-unknown-unknown"
+target triple = "wasm32-unknown-emscripten"
 

It appears this change means that we're no longer testing varargs on 
wasm32-unknown-unknown. Please update this test so that it tests both triples.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105749

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104808: [clang][emscripten] Reduce alignof long double from 16 to 8 bytes

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

Do we still intend to unify Emscripten's ABI with wasm32-unknown-unknown or 
wasm32-wasi eventually? This is talking a step away from that.

One of the assumptions behind this is that it would be ok for malloc to be 
16-byte aligned anyway, because SIMD use cases benefit from being able to call 
`malloc` and get a buffer aligned for SIMD. Do we have more information on how 
much this matters in practice?

Another observation is that part of the reason for long double being 128-bit is 
to leave room for the possibility of 128-bit floats being added to wasm some 
day, and on x86-64, aarch64, and powerpc which all have 128-bit long double on 
Linux, the alignment is 16 bytes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104808

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101608: [WebAssembly] Support for WebAssembly globals in LLVM IR

[Dan Gohman via Phabricator via cfe-commits]

sunfish added inline comments.



Comment at: llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h:39
+  // pointers are lowered to global.get / global.set or local.get / local.set,
+  // as appropriate.
+  WASM_ADDRESS_SPACE_MANAGED = 1

tlively wrote:
> sunfish wrote:
> > tlively wrote:
> > > sunfish wrote:
> > > > Sorry to throw more paint at the bikeshed here, but as someone who's 
> > > > only following along at a high-level here, I found it confusing whether 
> > > > this is talking about the wasm globals themselves, or the objects 
> > > > referred to by reference values in the wasm globals. I think the 
> > > > feature here is talking about the wasm globals themselves, but 
> > > > "managed" initially made me think it might be talking about the objects 
> > > > they reference, which in a browser context especially are "managed" in 
> > > > every sense of the word.
> > > Fair point. What does everyone think about `INDEXED`, because it is used 
> > > to represent objects given static indexes (in the WebAssembly sense) in 
> > > the final binary.
> > Do I understand correctly that global variables and local variables are 
> > being assigned addresses within the same conceptual address space here?
> > 
> > How about `WASM_VARIABLES` or `WASM_VARS`? The wasm spec terms for these 
> > are global variables and local variables.
> > 
> > 
> Sure, that works for me. We may want to rename it in the future if we end up 
> using the same address space for tables and memories, but we can cross that 
> bridge when we get there.
Tables and memories have their own index spaces in wasm, so it seems like 
they'd want their own address spaces here, perhaps 2 and 3, respectively? I 
also agree that we can figure this out later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101608

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101608: [WebAssembly] Support for WebAssembly globals in LLVM IR

[Dan Gohman via Phabricator via cfe-commits]

sunfish added inline comments.



Comment at: llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h:39
+  // pointers are lowered to global.get / global.set or local.get / local.set,
+  // as appropriate.
+  WASM_ADDRESS_SPACE_MANAGED = 1

tlively wrote:
> sunfish wrote:
> > Sorry to throw more paint at the bikeshed here, but as someone who's only 
> > following along at a high-level here, I found it confusing whether this is 
> > talking about the wasm globals themselves, or the objects referred to by 
> > reference values in the wasm globals. I think the feature here is talking 
> > about the wasm globals themselves, but "managed" initially made me think it 
> > might be talking about the objects they reference, which in a browser 
> > context especially are "managed" in every sense of the word.
> Fair point. What does everyone think about `INDEXED`, because it is used to 
> represent objects given static indexes (in the WebAssembly sense) in the 
> final binary.
Do I understand correctly that global variables and local variables are being 
assigned addresses within the same conceptual address space here?

How about `WASM_VARIABLES` or `WASM_VARS`? The wasm spec terms for these are 
global variables and local variables.





Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:1279
+  if (const GlobalAddressSDNode *GA = dyn_cast(Op))
+return WebAssembly::isManagedAddressSpace(GA->getAddressSpace());
+

sbc100 wrote:
> sunfish wrote:
> > sbc100 wrote:
> > > sunfish wrote:
> > > > tlively wrote:
> > > > > tlively wrote:
> > > > > > Actually, should we enforce that these LLVM IR globals be 
> > > > > > thread_local, since the resulting Wasm globals will be 
> > > > > > thread_local? I don't know if that will affect any optimizations, 
> > > > > > but it seems like a more accurate modeling. If we do that, 
> > > > > > `CoalesceLocalsAndStripAtomics::stripThreadLocals` in 
> > > > > > WebAssemblyTargetMachine.cpp will have to be updated to not strip 
> > > > > > the thread locals corresponding to Wasm globals.
> > > > > I'd be fine handling this in a follow-up if you want to get this 
> > > > > landed.
> > > > Note that this will be true of Worker-based threads, but not of the 
> > > > expected future "wasm native threads". I wonder if this means that 
> > > > clang/llvm will (eventually) need to be aware of this difference.
> > > Don't you think is very likely that even in "wasm native threads" we 
> > > would want to opt into sharing like we do with wasm memories?   That path 
> > > would seem better for compatibility with existing worker-based threading. 
> > >  
> > > 
> > > Obviously once we do have shared wasm globals we will need to modify 
> > > llvm's assumptions, but for now I think it is a reasonable 
> > > assumption/assertion that wasm globals are TLS.
> > Yeah, I agree we'll likely want some way to opt into sharing. So the 
> > difference here is, worker-based threads won't have the option of sharing, 
> > so maybe requiring `thread_local` makes sense. But native threads could opt 
> > into sharing, so they could be `thread_local` or not.
> > 
> Yup.  And who know .. maybe by the time shared globals and native threads are 
> added, Workers might also add shared globals?  
It's possible :-). In any case, I don't think it's anything we need to do 
anything about right now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101608

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101608: [WebAssembly] Support for WebAssembly globals in LLVM IR

[Dan Gohman via Phabricator via cfe-commits]

sunfish added inline comments.



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:1279
+  if (const GlobalAddressSDNode *GA = dyn_cast(Op))
+return WebAssembly::isManagedAddressSpace(GA->getAddressSpace());
+

sbc100 wrote:
> sunfish wrote:
> > tlively wrote:
> > > tlively wrote:
> > > > Actually, should we enforce that these LLVM IR globals be thread_local, 
> > > > since the resulting Wasm globals will be thread_local? I don't know if 
> > > > that will affect any optimizations, but it seems like a more accurate 
> > > > modeling. If we do that, 
> > > > `CoalesceLocalsAndStripAtomics::stripThreadLocals` in 
> > > > WebAssemblyTargetMachine.cpp will have to be updated to not strip the 
> > > > thread locals corresponding to Wasm globals.
> > > I'd be fine handling this in a follow-up if you want to get this landed.
> > Note that this will be true of Worker-based threads, but not of the 
> > expected future "wasm native threads". I wonder if this means that 
> > clang/llvm will (eventually) need to be aware of this difference.
> Don't you think is very likely that even in "wasm native threads" we would 
> want to opt into sharing like we do with wasm memories?   That path would 
> seem better for compatibility with existing worker-based threading.  
> 
> Obviously once we do have shared wasm globals we will need to modify llvm's 
> assumptions, but for now I think it is a reasonable assumption/assertion that 
> wasm globals are TLS.
Yeah, I agree we'll likely want some way to opt into sharing. So the difference 
here is, worker-based threads won't have the option of sharing, so maybe 
requiring `thread_local` makes sense. But native threads could opt into 
sharing, so they could be `thread_local` or not.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101608

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101608: [WebAssembly] Support for WebAssembly globals in LLVM IR

[Dan Gohman via Phabricator via cfe-commits]

sunfish added inline comments.



Comment at: llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h:39
+  // pointers are lowered to global.get / global.set or local.get / local.set,
+  // as appropriate.
+  WASM_ADDRESS_SPACE_MANAGED = 1

Sorry to throw more paint at the bikeshed here, but as someone who's only 
following along at a high-level here, I found it confusing whether this is 
talking about the wasm globals themselves, or the objects referred to by 
reference values in the wasm globals. I think the feature here is talking about 
the wasm globals themselves, but "managed" initially made me think it might be 
talking about the objects they reference, which in a browser context especially 
are "managed" in every sense of the word.



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:1279
+  if (const GlobalAddressSDNode *GA = dyn_cast(Op))
+return WebAssembly::isManagedAddressSpace(GA->getAddressSpace());
+

tlively wrote:
> tlively wrote:
> > Actually, should we enforce that these LLVM IR globals be thread_local, 
> > since the resulting Wasm globals will be thread_local? I don't know if that 
> > will affect any optimizations, but it seems like a more accurate modeling. 
> > If we do that, `CoalesceLocalsAndStripAtomics::stripThreadLocals` in 
> > WebAssemblyTargetMachine.cpp will have to be updated to not strip the 
> > thread locals corresponding to Wasm globals.
> I'd be fine handling this in a follow-up if you want to get this landed.
Note that this will be true of Worker-based threads, but not of the expected 
future "wasm native threads". I wonder if this means that clang/llvm will 
(eventually) need to be aware of this difference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101608

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100411: [WebAssembly] Use standard intrinsics for f32x4 and f64x2 ops

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

Great, thanks! And yes, switching to roundeven for both scalar and SIMD ISel 
sounds right to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100411

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100411: [WebAssembly] Use standard intrinsics for f32x4 and f64x2 ops

[Dan Gohman via Phabricator via cfe-commits]

sunfish accepted this revision.
sunfish added a comment.
This revision is now accepted and ready to land.

This looks good to me! Could you briefly comment here on what the issue with 
`llvm.roundeven` is?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100411

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89274: [WebAssembly] Use the new crt1-command.o if present.

[Dan Gohman via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf9c05fc39145: [WebAssembly] Use the new crt1-command.o if 
present. (authored by sunfish).

Changed prior to commit:
  https://reviews.llvm.org/D89274?vs=297698=323164#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89274

Files:
  clang/lib/Driver/ToolChains/WebAssembly.cpp


Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -77,6 +77,16 @@
 
   const char *Crt1 = "crt1.o";
   const char *Entry = NULL;
+
+  // If crt1-command.o exists, it supports new-style commands, so use it.
+  // Otherwise, use the old crt1.o. This is a temporary transition measure.
+  // Once WASI libc no longer needs to support LLVM versions which lack
+  // support for new-style command, it can make crt1.o the same as
+  // crt1-command.o. And once LLVM no longer needs to support WASI libc
+  // versions before that, it can switch to using crt1-command.o.
+  if (ToolChain.GetFilePath("crt1-command.o") != "crt1-command.o")
+Crt1 = "crt1-command.o";
+
   if (const Arg *A = Args.getLastArg(options::OPT_mexec_model_EQ)) {
 StringRef CM = A->getValue();
 if (CM == "command") {


Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -77,6 +77,16 @@
 
   const char *Crt1 = "crt1.o";
   const char *Entry = NULL;
+
+  // If crt1-command.o exists, it supports new-style commands, so use it.
+  // Otherwise, use the old crt1.o. This is a temporary transition measure.
+  // Once WASI libc no longer needs to support LLVM versions which lack
+  // support for new-style command, it can make crt1.o the same as
+  // crt1-command.o. And once LLVM no longer needs to support WASI libc
+  // versions before that, it can switch to using crt1-command.o.
+  if (ToolChain.GetFilePath("crt1-command.o") != "crt1-command.o")
+Crt1 = "crt1-command.o";
+
   if (const Arg *A = Args.getLastArg(options::OPT_mexec_model_EQ)) {
 StringRef CM = A->getValue();
 if (CM == "command") {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89274: [WebAssembly] Use the new crt1-command.o if present.

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

I don't see a way to do this with weak symbols, and an install script would be 
yet-another moving part that we'd have to make on end-user systems.

How about this: once we reach a point where we don't support the old LLVM 
anymore, libc can make crt1.o be the same as crt1-command.o, and then once 
we're sure we don't support libc old than that, we can switch LLVM back to 
crt1.o :-).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89274

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89274: [WebAssembly] Use the new crt1-command.o if present.

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

It's to ensure that older LLVM works with newer WASI libc, and newer clang 
works with older WASI libc. New-style commands require [lld support]. We can 
assume that if clang is updated, lld has the requisite support.

That said, I'm open to other ideas here.

[lld support]: 
https://github.com/llvm/llvm-project/commit/6cd8511e5932e4a53b2bb7780f69489355fc7783


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89274

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89274: [WebAssembly] Use the new crt1-command.o if present.

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

Ping!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89274

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D96091: [WebAssembly] Use single-threaded mode when -matomics isn't enabled.

[Dan Gohman via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG95da64da23ac: [WebAssembly] Use single-threaded mode when 
-matomics isnt enabled. (authored by sunfish).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96091

Files:
  clang/lib/Basic/Targets/WebAssembly.cpp
  clang/lib/Basic/Targets/WebAssembly.h
  clang/test/Preprocessor/init.c


Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -1479,6 +1479,12 @@
 // RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 
-triple=wasm64-wasi \
 // RUN:   < /dev/null \
 // RUN:   | FileCheck -match-full-lines 
-check-prefixes=WEBASSEMBLY,WEBASSEMBLY64,WEBASSEMBLY-WASI %s
+// RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 
-triple=wasm32-unknown-unknown -x c++ \
+// RUN:   < /dev/null \
+// RUN:   | FileCheck -match-full-lines -check-prefixes=WEBASSEMBLY-CXX %s
+// RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 
-triple=wasm32-unknown-unknown -x c++ -pthread -target-feature +atomics \
+// RUN:   < /dev/null \
+// RUN:   | FileCheck -match-full-lines 
-check-prefixes=WEBASSEMBLY-CXX-ATOMICS %s
 //
 // WEBASSEMBLY32:#define _ILP32 1
 // WEBASSEMBLY32-NOT:#define _LP64
@@ -1847,6 +1853,10 @@
 // WEBASSEMBLY64-NEXT:#define __wasm64 1
 // WEBASSEMBLY64-NEXT:#define __wasm64__ 1
 // WEBASSEMBLY-NEXT:#define __wasm__ 1
+// WEBASSEMBLY-CXX-NOT:_REENTRANT
+// WEBASSEMBLY-CXX-NOT:__STDCPP_THREADS__
+// WEBASSEMBLY-CXX-ATOMICS:#define _REENTRANT 1
+// WEBASSEMBLY-CXX-ATOMICS:#define __STDCPP_THREADS__ 1
 
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple i686-windows-cygnus < 
/dev/null | FileCheck -match-full-lines -check-prefix CYGWIN-X32 %s
 // CYGWIN-X32: #define __USER_LABEL_PREFIX__ _
Index: clang/lib/Basic/Targets/WebAssembly.h
===
--- clang/lib/Basic/Targets/WebAssembly.h
+++ clang/lib/Basic/Targets/WebAssembly.h
@@ -138,6 +138,8 @@
   bool hasExtIntType() const override { return true; }
 
   bool hasProtectedVisibility() const override { return false; }
+
+  void adjust(LangOptions ) override;
 };
 
 class LLVM_LIBRARY_VISIBILITY WebAssembly32TargetInfo
Index: clang/lib/Basic/Targets/WebAssembly.cpp
===
--- clang/lib/Basic/Targets/WebAssembly.cpp
+++ clang/lib/Basic/Targets/WebAssembly.cpp
@@ -253,6 +253,15 @@
  Builtin::FirstTSBuiltin);
 }
 
+void WebAssemblyTargetInfo::adjust(LangOptions ) {
+  // If the Atomics feature isn't available, turn off POSIXThreads and
+  // ThreadModel, so that we don't predefine _REENTRANT or __STDCPP_THREADS__.
+  if (!HasAtomics) {
+Opts.POSIXThreads = false;
+Opts.setThreadModel(LangOptions::ThreadModelKind::Single);
+  }
+}
+
 void WebAssembly32TargetInfo::getTargetDefines(const LangOptions ,
MacroBuilder ) const {
   WebAssemblyTargetInfo::getTargetDefines(Opts, Builder);


Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -1479,6 +1479,12 @@
 // RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 -triple=wasm64-wasi \
 // RUN:   < /dev/null \
 // RUN:   | FileCheck -match-full-lines -check-prefixes=WEBASSEMBLY,WEBASSEMBLY64,WEBASSEMBLY-WASI %s
+// RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 -triple=wasm32-unknown-unknown -x c++ \
+// RUN:   < /dev/null \
+// RUN:   | FileCheck -match-full-lines -check-prefixes=WEBASSEMBLY-CXX %s
+// RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 -triple=wasm32-unknown-unknown -x c++ -pthread -target-feature +atomics \
+// RUN:   < /dev/null \
+// RUN:   | FileCheck -match-full-lines -check-prefixes=WEBASSEMBLY-CXX-ATOMICS %s
 //
 // WEBASSEMBLY32:#define _ILP32 1
 // WEBASSEMBLY32-NOT:#define _LP64
@@ -1847,6 +1853,10 @@
 // WEBASSEMBLY64-NEXT:#define __wasm64 1
 // WEBASSEMBLY64-NEXT:#define __wasm64__ 1
 // WEBASSEMBLY-NEXT:#define __wasm__ 1
+// WEBASSEMBLY-CXX-NOT:_REENTRANT
+// WEBASSEMBLY-CXX-NOT:__STDCPP_THREADS__
+// WEBASSEMBLY-CXX-ATOMICS:#define _REENTRANT 1
+// WEBASSEMBLY-CXX-ATOMICS:#define __STDCPP_THREADS__ 1
 
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple i686-windows-cygnus < /dev/null | FileCheck -match-full-lines -check-prefix CYGWIN-X32 %s
 // CYGWIN-X32: #define __USER_LABEL_PREFIX__ _
Index: clang/lib/Basic/Targets/WebAssembly.h
===
--- clang/lib/Basic/Targets/WebAssembly.h
+++ clang/lib/Basic/Targets/WebAssembly.h
@@ -138,6 +138,8 @@
   bool hasExtIntType() const override { return true; }
 
   

[PATCH] D96091: [WebAssembly] Use single-threaded mode when -matomics isn't enabled.

[Dan Gohman via Phabricator via cfe-commits]

sunfish created this revision.
sunfish added a reviewer: tlively.
Herald added subscribers: ecnelises, jfb, jgravelle-google, sbc100, dschuff.
sunfish requested review of this revision.
Herald added subscribers: cfe-commits, aheejin.
Herald added a project: clang.

When the -matomics feature is not enabled, disable POSIXThreads
mode and set the thread model to Single, so that we don't predefine
macros like `__STDCPP_THREADS__`.

This will help save code size (a thing I care about!) in programs which
use `__STDCPP_THREADS__` to determine whether to include support
for threads.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96091

Files:
  clang/lib/Basic/Targets/WebAssembly.cpp
  clang/lib/Basic/Targets/WebAssembly.h
  clang/test/Preprocessor/init.c


Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -1479,6 +1479,12 @@
 // RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 
-triple=wasm64-wasi \
 // RUN:   < /dev/null \
 // RUN:   | FileCheck -match-full-lines 
-check-prefixes=WEBASSEMBLY,WEBASSEMBLY64,WEBASSEMBLY-WASI %s
+// RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 
-triple=wasm32-unknown-unknown -x c++ \
+// RUN:   < /dev/null \
+// RUN:   | FileCheck -match-full-lines -check-prefixes=WEBASSEMBLY-CXX %s
+// RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 
-triple=wasm32-unknown-unknown -x c++ -pthread -target-feature +atomics \
+// RUN:   < /dev/null \
+// RUN:   | FileCheck -match-full-lines 
-check-prefixes=WEBASSEMBLY-CXX-ATOMICS %s
 //
 // WEBASSEMBLY32:#define _ILP32 1
 // WEBASSEMBLY32-NOT:#define _LP64
@@ -1847,6 +1853,10 @@
 // WEBASSEMBLY64-NEXT:#define __wasm64 1
 // WEBASSEMBLY64-NEXT:#define __wasm64__ 1
 // WEBASSEMBLY-NEXT:#define __wasm__ 1
+// WEBASSEMBLY-CXX-NOT:_REENTRANT
+// WEBASSEMBLY-CXX-NOT:__STDCPP_THREADS__
+// WEBASSEMBLY-CXX-ATOMICS:#define _REENTRANT 1
+// WEBASSEMBLY-CXX-ATOMICS:#define __STDCPP_THREADS__ 1
 
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple i686-windows-cygnus < 
/dev/null | FileCheck -match-full-lines -check-prefix CYGWIN-X32 %s
 // CYGWIN-X32: #define __USER_LABEL_PREFIX__ _
Index: clang/lib/Basic/Targets/WebAssembly.h
===
--- clang/lib/Basic/Targets/WebAssembly.h
+++ clang/lib/Basic/Targets/WebAssembly.h
@@ -138,6 +138,8 @@
   bool hasExtIntType() const override { return true; }
 
   bool hasProtectedVisibility() const override { return false; }
+
+  void adjust(LangOptions ) override;
 };
 
 class LLVM_LIBRARY_VISIBILITY WebAssembly32TargetInfo
Index: clang/lib/Basic/Targets/WebAssembly.cpp
===
--- clang/lib/Basic/Targets/WebAssembly.cpp
+++ clang/lib/Basic/Targets/WebAssembly.cpp
@@ -253,6 +253,15 @@
  Builtin::FirstTSBuiltin);
 }
 
+void WebAssemblyTargetInfo::adjust(LangOptions ) {
+  // If the Atomics feature isn't available, turn off POSIXThreads and
+  // ThreadModel, so that we don't predefine _REENTRANT or __STDCPP_THREADS__.
+  if (!HasAtomics) {
+Opts.POSIXThreads = false;
+Opts.setThreadModel(LangOptions::ThreadModelKind::Single);
+  }
+}
+
 void WebAssembly32TargetInfo::getTargetDefines(const LangOptions ,
MacroBuilder ) const {
   WebAssemblyTargetInfo::getTargetDefines(Opts, Builder);


Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -1479,6 +1479,12 @@
 // RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 -triple=wasm64-wasi \
 // RUN:   < /dev/null \
 // RUN:   | FileCheck -match-full-lines -check-prefixes=WEBASSEMBLY,WEBASSEMBLY64,WEBASSEMBLY-WASI %s
+// RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 -triple=wasm32-unknown-unknown -x c++ \
+// RUN:   < /dev/null \
+// RUN:   | FileCheck -match-full-lines -check-prefixes=WEBASSEMBLY-CXX %s
+// RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 -triple=wasm32-unknown-unknown -x c++ -pthread -target-feature +atomics \
+// RUN:   < /dev/null \
+// RUN:   | FileCheck -match-full-lines -check-prefixes=WEBASSEMBLY-CXX-ATOMICS %s
 //
 // WEBASSEMBLY32:#define _ILP32 1
 // WEBASSEMBLY32-NOT:#define _LP64
@@ -1847,6 +1853,10 @@
 // WEBASSEMBLY64-NEXT:#define __wasm64 1
 // WEBASSEMBLY64-NEXT:#define __wasm64__ 1
 // WEBASSEMBLY-NEXT:#define __wasm__ 1
+// WEBASSEMBLY-CXX-NOT:_REENTRANT
+// WEBASSEMBLY-CXX-NOT:__STDCPP_THREADS__
+// WEBASSEMBLY-CXX-ATOMICS:#define _REENTRANT 1
+// WEBASSEMBLY-CXX-ATOMICS:#define __STDCPP_THREADS__ 1
 
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple i686-windows-cygnus < /dev/null | FileCheck -match-full-lines -check-prefix CYGWIN-X32 %s
 // CYGWIN-X32: #define __USER_LABEL_PREFIX__ _
Index: 

[PATCH] D89274: [WebAssembly] Use the new crt1-command.o if present.

[Dan Gohman via Phabricator via cfe-commits]

sunfish created this revision.
sunfish added a reviewer: sbc100.
Herald added subscribers: cfe-commits, ecnelises, jgravelle-google, dschuff.
Herald added a project: clang.
sunfish requested review of this revision.
Herald added a subscriber: aheejin.

If crt1-command.o exists in the sysroot, the libc has new-style command
support, so use it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89274

Files:
  clang/lib/Driver/ToolChains/WebAssembly.cpp


Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -77,6 +77,12 @@
 
   const char *Crt1 = "crt1.o";
   const char *Entry = NULL;
+
+  // If crt1-command.o exists, it supports new-style commands, so use it.
+  // Otherwise, use the old crt1.o.
+  if (ToolChain.GetFilePath("crt1-command.o") != "crt1-command.o")
+Crt1 = "crt1-command.o";
+
   if (const Arg *A = Args.getLastArg(options::OPT_mexec_model_EQ)) {
 StringRef CM = A->getValue();
 if (CM == "command") {


Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -77,6 +77,12 @@
 
   const char *Crt1 = "crt1.o";
   const char *Entry = NULL;
+
+  // If crt1-command.o exists, it supports new-style commands, so use it.
+  // Otherwise, use the old crt1.o.
+  if (ToolChain.GetFilePath("crt1-command.o") != "crt1-command.o")
+Crt1 = "crt1-command.o";
+
   if (const Arg *A = Args.getLastArg(options::OPT_mexec_model_EQ)) {
 StringRef CM = A->getValue();
 if (CM == "command") {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85347: [WebAssembly] Fix types in wasm_simd128.h and add tests

[Dan Gohman via Phabricator via cfe-commits]

sunfish accepted this revision.
sunfish added a comment.
This revision is now accepted and ready to land.

Ah, it seems the reason I didn't see this is that I wasn't using 
`-flax-vector-conversions=none`. Thanks for fixing this!




Comment at: clang/test/Headers/wasm.cpp:1
+// RUN: %clang_cc1 -triple wasm32-unknown-unknown -target-feature +simd128 
-fsyntax-only -ffreestanding %s -verify
+// expected-no-diagnostics

This test currently passes on trunk without the bug fix here. Could you add a 
-flax-vector-conversions=none line, similar to the C test, which is the case 
that the patch here fixes?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85347

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85074: [WebAssembly] Use "signed char" instead of "char" in SIMD intrinsics.

[Dan Gohman via Phabricator via cfe-commits]

sunfish updated this revision to Diff 282608.
sunfish added a comment.

- Update clang/test/CodeGen/builtins-wasm.c.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85074

Files:
  clang/include/clang/Basic/BuiltinsWebAssembly.def
  clang/lib/Headers/wasm_simd128.h
  clang/test/CodeGen/builtins-wasm.c

Index: clang/test/CodeGen/builtins-wasm.c
===
--- clang/test/CodeGen/builtins-wasm.c
+++ clang/test/CodeGen/builtins-wasm.c
@@ -3,7 +3,7 @@
 // RUN: not %clang_cc1 -triple wasm64-unknown-unknown -target-feature +nontrapping-fptoint -target-feature +exception-handling -target-feature +bulk-memory -target-feature +atomics -flax-vector-conversions=none -O3 -emit-llvm -o - %s 2>&1 | FileCheck %s -check-prefixes MISSING-SIMD
 
 // SIMD convenience types
-typedef char i8x16 __attribute((vector_size(16)));
+typedef signed char i8x16 __attribute((vector_size(16)));
 typedef short i16x8 __attribute((vector_size(16)));
 typedef int i32x4 __attribute((vector_size(16)));
 typedef long long i64x2 __attribute((vector_size(16)));
@@ -201,7 +201,7 @@
   // WEBASSEMBLY-NEXT: ret
 }
 
-int extract_lane_u_i8x16(i8x16 v) {
+int extract_lane_u_i8x16(u8x16 v) {
   return __builtin_wasm_extract_lane_u_i8x16(v, 13);
   // WEBASSEMBLY: extractelement <16 x i8> %v, i32 13
   // WEBASSEMBLY-NEXT: zext
@@ -215,7 +215,7 @@
   // WEBASSEMBLY-NEXT: ret
 }
 
-int extract_lane_u_i16x8(i16x8 v) {
+int extract_lane_u_i16x8(u16x8 v) {
   return __builtin_wasm_extract_lane_u_i16x8(v, 7);
   // WEBASSEMBLY: extractelement <8 x i16> %v, i32 7
   // WEBASSEMBLY-NEXT: zext
@@ -291,7 +291,7 @@
   // WEBASSEMBLY-NEXT: ret
 }
 
-i8x16 add_saturate_u_i8x16(i8x16 x, i8x16 y) {
+u8x16 add_saturate_u_i8x16(u8x16 x, u8x16 y) {
   return __builtin_wasm_add_saturate_u_i8x16(x, y);
   // WEBASSEMBLY: call <16 x i8> @llvm.uadd.sat.v16i8(
   // WEBASSEMBLY-SAME: <16 x i8> %x, <16 x i8> %y)
@@ -305,7 +305,7 @@
   // WEBASSEMBLY-NEXT: ret
 }
 
-i16x8 add_saturate_u_i16x8(i16x8 x, i16x8 y) {
+u16x8 add_saturate_u_i16x8(u16x8 x, u16x8 y) {
   return __builtin_wasm_add_saturate_u_i16x8(x, y);
   // WEBASSEMBLY: call <8 x i16> @llvm.uadd.sat.v8i16(
   // WEBASSEMBLY-SAME: <8 x i16> %x, <8 x i16> %y)
@@ -319,7 +319,7 @@
   // WEBASSEMBLY-NEXT: ret
 }
 
-i8x16 sub_saturate_u_i8x16(i8x16 x, i8x16 y) {
+u8x16 sub_saturate_u_i8x16(u8x16 x, u8x16 y) {
   return __builtin_wasm_sub_saturate_u_i8x16(x, y);
   // WEBASSEMBLY: call <16 x i8> @llvm.wasm.sub.saturate.unsigned.v16i8(
   // WEBASSEMBLY-SAME: <16 x i8> %x, <16 x i8> %y)
@@ -357,7 +357,7 @@
   // WEBASSEMBLY-NEXT: ret <16 x i8> %1
 }
 
-i8x16 min_u_i8x16(i8x16 x, i8x16 y) {
+u8x16 min_u_i8x16(u8x16 x, u8x16 y) {
   return __builtin_wasm_min_u_i8x16(x, y);
   // WEBASSEMBLY: %0 = icmp ult <16 x i8> %x, %y
   // WEBASSEMBLY-NEXT: %1 = select <16 x i1> %0, <16 x i8> %x, <16 x i8> %y
@@ -371,7 +371,7 @@
   // WEBASSEMBLY-NEXT: ret <16 x i8> %1
 }
 
-i8x16 max_u_i8x16(i8x16 x, i8x16 y) {
+u8x16 max_u_i8x16(u8x16 x, u8x16 y) {
   return __builtin_wasm_max_u_i8x16(x, y);
   // WEBASSEMBLY: %0 = icmp ugt <16 x i8> %x, %y
   // WEBASSEMBLY-NEXT: %1 = select <16 x i1> %0, <16 x i8> %x, <16 x i8> %y
@@ -385,7 +385,7 @@
   // WEBASSEMBLY-NEXT: ret <8 x i16> %1
 }
 
-i16x8 min_u_i16x8(i16x8 x, i16x8 y) {
+u16x8 min_u_i16x8(u16x8 x, u16x8 y) {
   return __builtin_wasm_min_u_i16x8(x, y);
   // WEBASSEMBLY: %0 = icmp ult <8 x i16> %x, %y
   // WEBASSEMBLY-NEXT: %1 = select <8 x i1> %0, <8 x i16> %x, <8 x i16> %y
@@ -399,7 +399,7 @@
   // WEBASSEMBLY-NEXT: ret <8 x i16> %1
 }
 
-i16x8 max_u_i16x8(i16x8 x, i16x8 y) {
+u16x8 max_u_i16x8(u16x8 x, u16x8 y) {
   return __builtin_wasm_max_u_i16x8(x, y);
   // WEBASSEMBLY: %0 = icmp ugt <8 x i16> %x, %y
   // WEBASSEMBLY-NEXT: %1 = select <8 x i1> %0, <8 x i16> %x, <8 x i16> %y
@@ -413,7 +413,7 @@
   // WEBASSEMBLY-NEXT: ret <4 x i32> %1
 }
 
-i32x4 min_u_i32x4(i32x4 x, i32x4 y) {
+u32x4 min_u_i32x4(u32x4 x, u32x4 y) {
   return __builtin_wasm_min_u_i32x4(x, y);
   // WEBASSEMBLY: %0 = icmp ult <4 x i32> %x, %y
   // WEBASSEMBLY-NEXT: %1 = select <4 x i1> %0, <4 x i32> %x, <4 x i32> %y
@@ -427,7 +427,7 @@
   // WEBASSEMBLY-NEXT: ret <4 x i32> %1
 }
 
-i32x4 max_u_i32x4(i32x4 x, i32x4 y) {
+u32x4 max_u_i32x4(u32x4 x, u32x4 y) {
   return __builtin_wasm_max_u_i32x4(x, y);
   // WEBASSEMBLY: %0 = icmp ugt <4 x i32> %x, %y
   // WEBASSEMBLY-NEXT: %1 = select <4 x i1> %0, <4 x i32> %x, <4 x i32> %y
@@ -441,21 +441,21 @@
   // WEBASSEMBLY-NEXT: ret
 }
 
-i16x8 sub_saturate_u_i16x8(i16x8 x, i16x8 y) {
+u16x8 sub_saturate_u_i16x8(u16x8 x, u16x8 y) {
   return __builtin_wasm_sub_saturate_u_i16x8(x, y);
   // WEBASSEMBLY: call <8 x i16> @llvm.wasm.sub.saturate.unsigned.v8i16(
   // WEBASSEMBLY-SAME: <8 x i16> %x, <8 x i16> %y)
   // WEBASSEMBLY-NEXT: ret
 }
 
-i8x16 avgr_u_i8x16(i8x16 x, i8x16 y) {
+u8x16 avgr_u_i8x16(u8x16 x, u8x16 y) {
   return 

[PATCH] D85074: [WebAssembly] Use "signed char" instead of "char" in SIMD intrinsics.

[Dan Gohman via Phabricator via cfe-commits]

sunfish created this revision.
sunfish added a reviewer: tlively.
Herald added subscribers: jgravelle-google, sbc100, dschuff.
Herald added a project: clang.
sunfish requested review of this revision.
Herald added a subscriber: aheejin.

This allows people to use `int8_t` instead of `char`, -funsigned-char,
and generally decouples SIMD from the specialness of `char`.

And it makes intrinsics like `__builtin_wasm_add_saturate_s_i8x16`
and `__builtin_wasm_add_saturate_u_i8x16` use signed and unsigned
element types, respectively.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85074

Files:
  clang/include/clang/Basic/BuiltinsWebAssembly.def
  clang/lib/Headers/wasm_simd128.h

Index: clang/lib/Headers/wasm_simd128.h
===
--- clang/lib/Headers/wasm_simd128.h
+++ clang/lib/Headers/wasm_simd128.h
@@ -18,8 +18,7 @@
 
 // Internal types determined by clang builtin definitions
 typedef int32_t __v128_u __attribute__((__vector_size__(16), __aligned__(1)));
-typedef char __i8x16 __attribute__((__vector_size__(16), __aligned__(16)));
-typedef signed char __s8x16
+typedef signed char __i8x16
 __attribute__((__vector_size__(16), __aligned__(16)));
 typedef unsigned char __u8x16
 __attribute__((__vector_size__(16), __aligned__(16)));
@@ -340,17 +339,17 @@
 
 static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_i8x16_eq(v128_t __a,
   v128_t __b) {
-  return (v128_t)((__s8x16)__a == (__s8x16)__b);
+  return (v128_t)((__i8x16)__a == (__i8x16)__b);
 }
 
 static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_i8x16_ne(v128_t __a,
   v128_t __b) {
-  return (v128_t)((__s8x16)__a != (__s8x16)__b);
+  return (v128_t)((__i8x16)__a != (__i8x16)__b);
 }
 
 static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_i8x16_lt(v128_t __a,
   v128_t __b) {
-  return (v128_t)((__s8x16)__a < (__s8x16)__b);
+  return (v128_t)((__i8x16)__a < (__i8x16)__b);
 }
 
 static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_u8x16_lt(v128_t __a,
@@ -360,7 +359,7 @@
 
 static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_i8x16_gt(v128_t __a,
   v128_t __b) {
-  return (v128_t)((__s8x16)__a > (__s8x16)__b);
+  return (v128_t)((__i8x16)__a > (__i8x16)__b);
 }
 
 static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_u8x16_gt(v128_t __a,
@@ -370,7 +369,7 @@
 
 static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_i8x16_le(v128_t __a,
   v128_t __b) {
-  return (v128_t)((__s8x16)__a <= (__s8x16)__b);
+  return (v128_t)((__i8x16)__a <= (__i8x16)__b);
 }
 
 static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_u8x16_le(v128_t __a,
@@ -380,7 +379,7 @@
 
 static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_i8x16_ge(v128_t __a,
   v128_t __b) {
-  return (v128_t)((__s8x16)__a >= (__s8x16)__b);
+  return (v128_t)((__i8x16)__a >= (__i8x16)__b);
 }
 
 static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_u8x16_ge(v128_t __a,
@@ -602,7 +601,7 @@
 
 static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_i8x16_shr(v128_t __a,
int32_t __b) {
-  return (v128_t)((__s8x16)__a >> __b);
+  return (v128_t)((__i8x16)__a >> __b);
 }
 
 static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_u8x16_shr(v128_t __a,
Index: clang/include/clang/Basic/BuiltinsWebAssembly.def
===
--- clang/include/clang/Basic/BuiltinsWebAssembly.def
+++ clang/include/clang/Basic/BuiltinsWebAssembly.def
@@ -66,67 +66,67 @@
 TARGET_BUILTIN(__builtin_wasm_trunc_saturate_u_i64_f64, "LLid", "nc", "nontrapping-fptoint")
 
 // SIMD builtins
-TARGET_BUILTIN(__builtin_wasm_swizzle_v8x16, "V16cV16cV16c", "nc", "simd128")
+TARGET_BUILTIN(__builtin_wasm_swizzle_v8x16, "V16ScV16ScV16Sc", "nc", "simd128")
 
-TARGET_BUILTIN(__builtin_wasm_extract_lane_s_i8x16, "iV16cIi", "nc", "simd128")
-TARGET_BUILTIN(__builtin_wasm_extract_lane_u_i8x16, "iV16cIi", "nc", "simd128")
+TARGET_BUILTIN(__builtin_wasm_extract_lane_s_i8x16, "iV16ScIi", "nc", "simd128")
+TARGET_BUILTIN(__builtin_wasm_extract_lane_u_i8x16, "iV16UcIUi", "nc", "simd128")
 TARGET_BUILTIN(__builtin_wasm_extract_lane_s_i16x8, "iV8sIi", "nc", "simd128")
-TARGET_BUILTIN(__builtin_wasm_extract_lane_u_i16x8, "iV8sIi", "nc", "simd128")
+TARGET_BUILTIN(__builtin_wasm_extract_lane_u_i16x8, "iV8UsIUi", "nc", "simd128")
 TARGET_BUILTIN(__builtin_wasm_extract_lane_i32x4, "iV4iIi", "nc", "simd128")
 TARGET_BUILTIN(__builtin_wasm_extract_lane_i64x2, "LLiV2LLiIi", "nc", "simd128")
 TARGET_BUILTIN(__builtin_wasm_extract_lane_f32x4, "fV4fIi", "nc", "simd128")
 TARGET_BUILTIN(__builtin_wasm_extract_lane_f64x2, "dV2dIi", "nc", "simd128")
 
-TARGET_BUILTIN(__builtin_wasm_replace_lane_i8x16, 

[PATCH] D81688: [WebAssembly] WebAssembly doesn't support "protected" visibility

[Dan Gohman via Phabricator via cfe-commits]

sunfish closed this revision.
sunfish added a comment.

Landed in 66042959590d6db9d2a12803a16476d4e3508f3f 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81688



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81688: [WebAssembly] WebAssembly doesn't support "protected" visibility

[Dan Gohman via Phabricator via cfe-commits]

sunfish created this revision.
Herald added subscribers: llvm-commits, aheejin, jgravelle-google, sbc100, 
dschuff.
Herald added projects: clang, LLVM.

mplement the `hasProtectedVisibility()` hook to indicate that, like
Darwin, WebAssembly doesn't support "protected" visibility.

On ELF, "protected" visibility is intended to be an optimization, however
in practice it often [isn't], and ELF documentation generally ranges from
[not mentioning it at all] to [strongly discouraging its use].

[isn't]: https://www.airs.com/blog/archives/307
[not mentioning it at all]: https://gcc.gnu.org/wiki/Visibility
[strongly discouraging its use]: https://www.akkadia.org/drepper/dsohowto.pdf

While here, also mention the new Reactor support in the release notes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81688

Files:
  clang/lib/Basic/Targets/WebAssembly.h
  llvm/docs/ReleaseNotes.rst


Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -144,8 +144,14 @@
 Changes to the WebAssembly Target
 -
 
-During this release ...
-
+* Programs which don't have a "main" function, called "reactors" are now
+  properly supported, with a new `-mexec-model=reactor` flag. Programs which
+  previously used `-Wl,--no-entry` to avoid having a main function should
+  switch to this new flag, so that static initialization is properly
+  performed.
+
+* `__attribute__((visibility("protected")))` now evokes a warning, as
+  WebAssembly does not support "protected" visibility.
 
 Changes to the OCaml bindings
 -
Index: clang/lib/Basic/Targets/WebAssembly.h
===
--- clang/lib/Basic/Targets/WebAssembly.h
+++ clang/lib/Basic/Targets/WebAssembly.h
@@ -132,7 +132,14 @@
   }
 
   bool hasExtIntType() const override { return true; }
+
+  bool hasProtectedVisibility() const override {
+// For compatibility, continue to advertise "protected" support for
+// Emscripten targets.
+return getTriple().isOSEmscripten();
+  }
 };
+
 class LLVM_LIBRARY_VISIBILITY WebAssembly32TargetInfo
 : public WebAssemblyTargetInfo {
 public:


Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -144,8 +144,14 @@
 Changes to the WebAssembly Target
 -
 
-During this release ...
-
+* Programs which don't have a "main" function, called "reactors" are now
+  properly supported, with a new `-mexec-model=reactor` flag. Programs which
+  previously used `-Wl,--no-entry` to avoid having a main function should
+  switch to this new flag, so that static initialization is properly
+  performed.
+
+* `__attribute__((visibility("protected")))` now evokes a warning, as
+  WebAssembly does not support "protected" visibility.
 
 Changes to the OCaml bindings
 -
Index: clang/lib/Basic/Targets/WebAssembly.h
===
--- clang/lib/Basic/Targets/WebAssembly.h
+++ clang/lib/Basic/Targets/WebAssembly.h
@@ -132,7 +132,14 @@
   }
 
   bool hasExtIntType() const override { return true; }
+
+  bool hasProtectedVisibility() const override {
+// For compatibility, continue to advertise "protected" support for
+// Emscripten targets.
+return getTriple().isOSEmscripten();
+  }
 };
+
 class LLVM_LIBRARY_VISIBILITY WebAssembly32TargetInfo
 : public WebAssemblyTargetInfo {
 public:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62922: [WebAssembly] Implement "Reactor" mode

[Dan Gohman via Phabricator via cfe-commits]

sunfish marked an inline comment as done.
sunfish added inline comments.



Comment at: clang/test/Driver/wasm-toolchain.c:116
+// CHECK-COMMAND: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// CHECK-COMMAND: wasm-ld{{.*}}" "crt1.o" "[[temp]]" "-lc" 
"{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
+

bjope wrote:
> sunfish wrote:
> > bjope wrote:
> > > bjope wrote:
> > > > This isn't working for me on redhat 7 servers. I get
> > > > ```
> > > >  "wasm-ld" "-L/lib" "/lib/crt1.o" ...
> > > > ```
> > > > No idea why I get the full patch to crt1.o. But it's not matching with 
> > > > the FileCheck pattern.
> > > /full patch/full path/
> > Would you be able to test whether the following patch fixes it?
> > 
> > ```diff
> > diff --git a/clang/test/Driver/wasm-toolchain.c 
> > b/clang/test/Driver/wasm-toolchain.c
> > index 8300a81614e..332e6048cc5 100644
> > --- a/clang/test/Driver/wasm-toolchain.c
> > +++ b/clang/test/Driver/wasm-toolchain.c
> > @@ -110,11 +110,11 @@
> > 
> >  // Basic exec-model tests.
> > 
> > -// RUN: %clang %s -### -no-canonical-prefixes -target 
> > wasm32-unknown-unknown -mexec-model=command 2>&1 \
> > +// RUN: %clang %s -### -no-canonical-prefixes -target 
> > wasm32-unknown-unknown --sysroot=%s/no-sysroot-there -mexec-model=command 
> > 2>&1 \
> >  // RUN:   | FileCheck -check-prefix=CHECK-COMMAND %s
> >  // CHECK-COMMAND: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
> >  // CHECK-COMMAND: wasm-ld{{.*}}" "crt1.o" "[[temp]]" "-lc" 
> > "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
> > 
> > -// RUN: %clang %s -### -no-canonical-prefixes -target 
> > wasm32-unknown-unknown -mexec-model=reactor 2>&1 \
> > +// RUN: %clang %s -### -no-canonical-prefixes -target 
> > wasm32-unknown-unknown --sysroot=%s/no-sysroot-there -mexec-model=reactor 
> > 2>&1 \
> >  // RUN:   | FileCheck -check-prefix=CHECK-REACTOR %s
> >  // CHECK-REACTOR: wasm-ld{{.*}}" {{.*}} "--entry" "_initialize" {{.*}}
> > ```
> Yes, that is working.
Thanks for confirming! I've now committed this fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62922



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62922: [WebAssembly] Implement "Reactor" mode

[Dan Gohman via Phabricator via cfe-commits]

sunfish marked an inline comment as done.
sunfish added inline comments.



Comment at: clang/test/Driver/wasm-toolchain.c:116
+// CHECK-COMMAND: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// CHECK-COMMAND: wasm-ld{{.*}}" "crt1.o" "[[temp]]" "-lc" 
"{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
+

bjope wrote:
> bjope wrote:
> > This isn't working for me on redhat 7 servers. I get
> > ```
> >  "wasm-ld" "-L/lib" "/lib/crt1.o" ...
> > ```
> > No idea why I get the full patch to crt1.o. But it's not matching with the 
> > FileCheck pattern.
> /full patch/full path/
Would you be able to test whether the following patch fixes it?

```diff
diff --git a/clang/test/Driver/wasm-toolchain.c 
b/clang/test/Driver/wasm-toolchain.c
index 8300a81614e..332e6048cc5 100644
--- a/clang/test/Driver/wasm-toolchain.c
+++ b/clang/test/Driver/wasm-toolchain.c
@@ -110,11 +110,11 @@

 // Basic exec-model tests.

-// RUN: %clang %s -### -no-canonical-prefixes -target wasm32-unknown-unknown 
-mexec-model=command 2>&1 \
+// RUN: %clang %s -### -no-canonical-prefixes -target wasm32-unknown-unknown 
--sysroot=%s/no-sysroot-there -mexec-model=command 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-COMMAND %s
 // CHECK-COMMAND: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // CHECK-COMMAND: wasm-ld{{.*}}" "crt1.o" "[[temp]]" "-lc" 
"{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"

-// RUN: %clang %s -### -no-canonical-prefixes -target wasm32-unknown-unknown 
-mexec-model=reactor 2>&1 \
+// RUN: %clang %s -### -no-canonical-prefixes -target wasm32-unknown-unknown 
--sysroot=%s/no-sysroot-there -mexec-model=reactor 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-REACTOR %s
 // CHECK-REACTOR: wasm-ld{{.*}}" {{.*}} "--entry" "_initialize" {{.*}}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62922



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59520: [WebAssembly] Address review comments on r352930

[Dan Gohman via Phabricator via cfe-commits]

sunfish marked 3 inline comments as done.
sunfish added a comment.

I apologize again for the major delay. I've now updated the patch and addressed 
all of your comments.




Comment at: clang/lib/Sema/SemaDecl.cpp:2594-2597
+  else if (const auto *IMA = dyn_cast(Attr))
+NewAttr = S.mergeImportModuleAttr(D, *IMA);
+  else if (const auto *INA = dyn_cast(Attr))
+NewAttr = S.mergeImportNameAttr(D, *INA);

aaron.ballman wrote:
> You should probably have some tests for the redeclaration behavior somewhere. 
> The way you usually do this is to declare the function with the attribute, 
> then declare the function again without the attribute but see that it is in 
> fact inherited. We sometimes use AST dumping tests for this.
I've now added tests for this as you suggested.



Comment at: clang/test/Sema/attr-wasm.c:11
+
+__attribute__((import_name("foo"))) void name_e() {} //FIXME-expected-error 
{{import name cannot be applied to a function with a definition}}
+

aaron.ballman wrote:
> Are you intending to fix this as part of the patch?
I've made a few attempts at fixing this, but I'm not very familiar with these 
parts of clang, and I've been unable to find another attribute with a similar 
diagnostic.

The `weak_import` attribute seems like it should be one, with its 
`warn_attribute_invalid_on_definition` warning, and  there's code in clang to 
issue that warning for both functions and variables, however the function 
version of the diagnostic doesn't seem to work, and only the variable version 
has tests.

So for now I've just removed these tests, to at least unblock the rest of this 
patch. If anyone knows how to implement these diagnostics, I'd be interested.



Comment at: clang/test/Sema/attr-wasm.c:15
+
+void name_z() __attribute__((import_name("bar"))); //expected-warning {{import 
name does not match previous declaration}}
+

aaron.ballman wrote:
> Uncertain how important you think this may be: should we include the import 
> names in this diagnostic? e.g., `import name (%0) does not match the import 
> name (%1) of the previous declaration` or somesuch? I don't know how often 
> users will hide these attributes behind macros, which is the case I was 
> worried might be confusing.
I've now updated the patch to provide a more informative message, following 
your suggestion here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59520



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59520: [WebAssembly] Address review comments on r352930

[Dan Gohman via Phabricator via cfe-commits]

sunfish updated this revision to Diff 268329.
sunfish added a comment.

- Add tests for redeclaration behavior
- Remove disabled tests (previously marked with FIXMEs)
- Made the mismatch warning more informative.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59520

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/AST/ast-dump-wasm-attr-export.c
  clang/test/AST/ast-dump-wasm-attr-import.c
  clang/test/Sema/attr-wasm.c

Index: clang/test/Sema/attr-wasm.c
===
--- /dev/null
+++ clang/test/Sema/attr-wasm.c
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -triple wasm32-unknown-unknown -fsyntax-only -verify %s
+
+void name_a() __attribute__((import_name)); //expected-error {{'import_name' attribute takes one argument}}
+
+int name_b __attribute__((import_name("foo"))); //expected-error {{'import_name' attribute only applies to functions}}
+
+void name_c() __attribute__((import_name("foo", "bar"))); //expected-error {{'import_name' attribute takes one argument}}
+
+void name_d() __attribute__((import_name("foo", "bar", "qux"))); //expected-error {{'import_name' attribute takes one argument}}
+
+void name_z() __attribute__((import_name("foo"))); //expected-note {{previous attribute is here}}
+
+void name_z() __attribute__((import_name("bar"))); //expected-warning {{import name (bar) does not match the import name (foo) of the previous declaration}}
+
+void module_a() __attribute__((import_module)); //expected-error {{'import_module' attribute takes one argument}}
+
+int module_b __attribute__((import_module("foo"))); //expected-error {{'import_module' attribute only applies to functions}}
+
+void module_c() __attribute__((import_module("foo", "bar"))); //expected-error {{'import_module' attribute takes one argument}}
+
+void module_d() __attribute__((import_module("foo", "bar", "qux"))); //expected-error {{'import_module' attribute takes one argument}}
+
+void module_z() __attribute__((import_module("foo"))); //expected-note {{previous attribute is here}}
+
+void module_z() __attribute__((import_module("bar"))); //expected-warning {{import module (bar) does not match the import module (foo) of the previous declaration}}
+
+void both() __attribute__((import_name("foo"), import_module("bar")));
Index: clang/test/AST/ast-dump-wasm-attr-import.c
===
--- /dev/null
+++ clang/test/AST/ast-dump-wasm-attr-import.c
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -triple wasm32-unknown-unknown -ast-dump %s | FileCheck --strict-whitespace %s
+
+// Test that functions can be redeclared and they retain their attributes.
+
+__attribute__((import_name("import_red"), import_module("mod"))) void red(void);
+__attribute__((import_name("import_orange"), import_module("mod"))) void orange(void);
+__attribute__((import_name("import_yellow"), import_module("mod"))) void yellow(void);
+
+void red(void);
+void orange(void);
+void yellow(void);
+
+void calls(void) {
+red();
+orange();
+yellow();
+}
+
+// CHECK: |-FunctionDecl {{.+}} used red 'void (void)'
+// CHECK: | |-WebAssemblyImportNameAttr {{.+}} "import_red"
+// CHECK: | `-WebAssemblyImportModuleAttr {{.+}} "mod"
+// CHECK: |-FunctionDecl {{.+}} used orange 'void (void)'
+// CHECK: | |-WebAssemblyImportNameAttr {{.+}} "import_orange"
+// CHECK: | `-WebAssemblyImportModuleAttr {{.+}} "mod"
+// CHECK: |-FunctionDecl {{.+}} used yellow 'void (void)'
+// CHECK: | |-WebAssemblyImportNameAttr {{.+}} "import_yellow"
+// CHECK: | `-WebAssemblyImportModuleAttr {{.+}} "mod"
+// CHECK: |-FunctionDecl {{.+}} used red 'void (void)'
+// CHECK: | |-WebAssemblyImportNameAttr {{.+}} Inherited "import_red"
+// CHECK: | `-WebAssemblyImportModuleAttr {{.+}} Inherited "mod"
+// CHECK: |-FunctionDecl {{.+}} used orange 'void (void)'
+// CHECK: | |-WebAssemblyImportNameAttr {{.+}} Inherited "import_orange"
+// CHECK: | `-WebAssemblyImportModuleAttr {{.+}} Inherited "mod"
+// CHECK: |-FunctionDecl {{.+}} used yellow 'void (void)'
+// CHECK: | |-WebAssemblyImportNameAttr {{.+}} Inherited "import_yellow"
+// CHECK: | `-WebAssemblyImportModuleAttr {{.+}} Inherited "mod"
Index: clang/test/AST/ast-dump-wasm-attr-export.c
===
--- /dev/null
+++ clang/test/AST/ast-dump-wasm-attr-export.c
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -triple wasm32-unknown-unknown -ast-dump %s | FileCheck --strict-whitespace %s
+
+// Test that functions can be redeclared and they retain their attributes.
+
+__attribute__((export_name("export_red"))) void red(void) {}
+__attribute__((export_name("export_orange"))) void orange(void) {}
+__attribute__((export_name("export_yellow"))) void yellow(void) {}
+
+void red(void);
+void orange(void);
+void yellow(void);

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

In D70500#1999268 , @bernhard wrote:

> > It's for users who want smaller wasm binaries. It's not currently 
> > documented, though yes, it would be nice to document it.
>
> But how would a user even end up with wasm-opt in the same directory of clang 
> binaries?


wasi-sdk may package wasm-opt that way in the future. But, wasm-opt can also be 
found in the PATH.

>> Clang and wasm-ld are free to do anything wasm-opt does. Could you describe 
>> what you want to do in more detail? It may be possible to find alternative 
>> approaches, or to design a feature for it with a clear and documentable 
>> scope.
> 
> I'm generating additional data segments which while doing so increases the 
> initial memory pages count and shifts any globals that point to __heap_base 
> and global[0] (stack ptr init).

Would it work to increase the memory size, and then put your data in the new 
space this creates at the end of memory? This would avoid the need to shift 
anything around or depend on anything about the layout. It isn't yet documented 
anywhere, but the pattern of always allocating new memory at the end of the 
address space (as `memory.grow` does) is common enough and important enough 
that we probably should document it.

> Due to memory-packing the end of data becomes ambiguous (-export=__data_end 
> would mitigate that but it's nice to be compatible with any un-opt'ed wasm 
> file), and due to simplify-globals with remove-unused-module-elements the 
> global[0] can get copied/moved directly into the code section.
>  I am aware that this treads very much in undocumented assumptions, it is 
> still experimental but it works. This hidden execution of wasm-opt is mostly 
> making documentation on my end a bit harder.

`__data_end` and `__heap_base` aren't things you can move around once the 
linker has defined them. Their values can be baked in elsewhere in the wasm 
module, so it's not safe in general to insert new memory in between them and 
shift things around. And it happens that wasi-libc has some code that we hope 
to enable soon which allocates malloc memory using `__heap_base` which would 
likely break if things get shifted around after linking.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

In D70500#1998994 , @bernhard wrote:

> Are there plans to offer a way to disable this behavior (or have it optional 
> in the first place)?
>  We'd like to run some custom processing between wasm-ld and wasm-opt which 
> can't happen after the latter due to some of its one-way destructive 
> optimizations (i.e. memory-packing or simplify-globals passes).
>  The only way now is to tell our users to place wasm-opt somewhere where 
> clang can't find it. Or instead of using one clang super-command to manually 
> call -cc1 and wasm-ld separately which is disappointing.
>
> Also, is it even common to place wasm-opt next to the clang executable? Who 
> is this for? Is this documented?


It's for users who want smaller wasm binaries. It's not currently documented, 
though yes, it would be nice to document it.

That said, details about how LLVM lays out memory or uses globals are 
intentionally not documented. Just as we don't make any guarantees about 
exactly which optimizations we do at -O1 vs -O2, we're don't make any 
guarantees about which optimizations are done in clang vs wasm-ld vs wasm-opt. 
Clang and wasm-ld are free to do anything wasm-opt does. Could you describe 
what you want to do in more detail? It may be possible to find alternative 
approaches, or to design a feature for it with a clear and documentable scope.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62922: [WebAssembly] Implement "Reactor" mode

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

This addresses the review feedback from earlier. To answer this question:

> I assume that all he users -Wl args come after.. so its still possible to set 
> --entry on the command line?

Yes, that is what happens.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62922



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62922: [WebAssembly] Implement "Reactor" mode

[Dan Gohman via Phabricator via cfe-commits]

sunfish updated this revision to Diff 257171.
sunfish added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Rebase, update, add a test, and add basic error reporting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62922

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/WebAssembly.cpp
  clang/test/Driver/wasm-toolchain.c


Index: clang/test/Driver/wasm-toolchain.c
===
--- clang/test/Driver/wasm-toolchain.c
+++ clang/test/Driver/wasm-toolchain.c
@@ -107,3 +107,14 @@
 // RUN: %clang %s -### -fsanitize=address -target wasm32-unknown-emscripten 
2>&1 | FileCheck -check-prefix=CHECK-ASAN-EMSCRIPTEN %s
 // CHECK-ASAN-EMSCRIPTEN: "-fsanitize=address"
 // CHECK-ASAN-EMSCRIPTEN: "-fsanitize-address-globals-dead-stripping"
+
+// Basic exec-model tests.
+
+// RUN: %clang %s -### -no-canonical-prefixes -target wasm32-unknown-unknown 
-mexec-model=command 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-COMMAND %s
+// CHECK-COMMAND: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// CHECK-COMMAND: wasm-ld{{.*}}" "crt1.o" "[[temp]]" "-lc" 
"{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
+
+// RUN: %clang %s -### -no-canonical-prefixes -target wasm32-unknown-unknown 
-mexec-model=reactor 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-REACTOR %s
+// CHECK-REACTOR: wasm-ld{{.*}}" {{.*}} "--entry" "_initialize" {{.*}}
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -69,8 +69,26 @@
   Args.AddAllArgs(CmdArgs, options::OPT_u);
   ToolChain.AddFilePathLibArgs(Args, CmdArgs);
 
+  const char *Crt1 = "crt1.o";
+  const char *Entry = NULL;
+  if (const Arg *A = Args.getLastArg(options::OPT_mexec_model_EQ)) {
+StringRef CM = A->getValue();
+if (CM == "command") {
+  // Use default values.
+} else if (CM == "reactor") {
+  Crt1 = "crt1-reactor.o";
+  Entry = "_initialize";
+} else {
+  ToolChain.getDriver().Diag(diag::err_drv_invalid_argument_to_option)
+  << CM << A->getOption().getName();
+}
+  }
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles))
-CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crt1.o")));
+CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath(Crt1)));
+  if (Entry) {
+CmdArgs.push_back(Args.MakeArgString("--entry"));
+CmdArgs.push_back(Args.MakeArgString(Entry));
+  }
 
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
 
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -151,6 +151,10 @@
Group, DocName<"PowerPC">;
 def m_wasm_Features_Group : OptionGroup<"">,
 Group, DocName<"WebAssembly">;
+// The features added by this group will not be added to target features.
+// These are explicitly handled.
+def m_wasm_Features_Driver_Group : OptionGroup<"">,
+   Group, DocName<"WebAssembly 
Driver">;
 def m_x86_Features_Group : OptionGroup<"">,
Group, Flags<[CoreOption]>, DocName<"X86">;
 def m_riscv_Features_Group : OptionGroup<"">,
@@ -2416,6 +2420,9 @@
 def mno_tail_call : Flag<["-"], "mno-tail-call">, Group;
 def mreference_types : Flag<["-"], "mreference-types">, 
Group;
 def mno_reference_types : Flag<["-"], "mno-reference-types">, 
Group;
+def mexec_model_EQ : Joined<["-"], "mexec-model=">, 
Group,
+ Values<"command,reactor">,
+ HelpText<"Execution model (WebAssembly only)">;
 
 def mamdgpu_debugger_abi : Joined<["-"], "mamdgpu-debugger-abi=">,
   Flags<[HelpHidden]>,
Index: clang/docs/ClangCommandLineReference.rst
===
--- clang/docs/ClangCommandLineReference.rst
+++ clang/docs/ClangCommandLineReference.rst
@@ -3035,6 +3035,12 @@
 
 .. option:: -munimplemented-simd128, -mno-unimplemented-simd128
 
+.. option:: -mexec-model=
+
+Select between "command" and "reactor" executable models. Commands have a main
+function which scopes the lifetime of the program. Reactors are activated and
+remain active until explicitly terminated.
+
 X86
 ---
 .. option:: -m3dnow, -mno-3dnow


Index: clang/test/Driver/wasm-toolchain.c
===
--- clang/test/Driver/wasm-toolchain.c
+++ clang/test/Driver/wasm-toolchain.c
@@ -107,3 +107,14 @@
 // RUN: %clang %s -### -fsanitize=address -target wasm32-unknown-emscripten 2>&1 | FileCheck -check-prefix=CHECK-ASAN-EMSCRIPTEN %s
 // 

[PATCH] D77908: [WebAssembly] Enable nontrapping-fptoint for `default` cpu

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

Looks good! I don't have anything to add beyond Thomas' review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77908



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76959: [WebAssembly] Import wasm_simd128.h from Emscripten

[Dan Gohman via Phabricator via cfe-commits]

sunfish accepted this revision.
sunfish added a comment.
This revision is now accepted and ready to land.

Cool, LGTM, with optional suggestion for signed char below:




Comment at: clang/lib/Headers/wasm_simd128.h:30
+typedef long long __i64x2 __attribute__((__vector_size__(16), 
__aligned__(16)));
+typedef unsigned long long __u64x2
+__attribute__((__vector_size__(16), __aligned__(16)));

tlively wrote:
> sunfish wrote:
> > Since this file is already including , instead of `char`, 
> > `unsigned char`, `short`, `unsigned short`, etc., can these use `int8_t`, 
> > `uint8_t`, `int16_t`, `uint16_t`, and so on, for clarity? Also, this would 
> > avoid any possible behavior change under `-funsigned-char`.
> Unfortunately not :( This change gives errors like
> 
> ```
> error: cannot initialize a parameter of type 
> '__attribute__((__vector_size__(16 * sizeof(char char' (vector of 16 
> 'char' values) with an rvalue of type '__i8x16' (vector of 16 'int8_t' values)
>   return (v128_t)__builtin_wasm_abs_i8x16((__i8x16)a);
> ```
> 
> Even changing `char` to `signed char` doesn't work. I would change the 
> builtins to use the better types, but the builtin definition system does not 
> allow those more interesting types.
> 
> This is especially annoying because `-funsigned-char` does indeed change the 
> behavior of the comparison intrinsics. I will have to do extra work in those 
> functions to make it work.
Would changing the clang definitions of the builtins to use signed char (Sc) 
explicitly fix this, like:

-TARGET_BUILTIN(__builtin_wasm_swizzle_v8x16, "V16cV16cV16c", "nc", 
"unimplemented-simd128")
+TARGET_BUILTIN(__builtin_wasm_swizzle_v8x16, "V16ScV16ScV16Sc", "nc", 
"unimplemented-simd128")

and so on for all the wasm simd builtins?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76959



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76959: [WebAssembly] Import wasm_simd128.h from Emscripten

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

Very cool, thanks for putting this together!




Comment at: clang/lib/Headers/wasm_simd128.h:10
+
+#pragma once
+

Do you know why other clang headers, such as `lib/Headers/xmmintrin.h`, don't 
use `#pragma once`?



Comment at: clang/lib/Headers/wasm_simd128.h:30
+typedef long long __i64x2 __attribute__((__vector_size__(16), 
__aligned__(16)));
+typedef unsigned long long __u64x2
+__attribute__((__vector_size__(16), __aligned__(16)));

Since this file is already including , instead of `char`, `unsigned 
char`, `short`, `unsigned short`, etc., can these use `int8_t`, `uint8_t`, 
`int16_t`, `uint16_t`, and so on, for clarity? Also, this would avoid any 
possible behavior change under `-funsigned-char`.



Comment at: clang/lib/Headers/wasm_simd128.h:40
+#define __REQUIRE_CONSTANT(e)  
\
+  _Static_assert(__builtin_constant_p(e), "Expected constant")
+

In clang's xmmintrin.h, helper macros like these `__DEFAULT_FN_ATTRS` and 
`__REQUIRE_CONSTANT` are `#undef`ed at the end of the file.



Comment at: clang/lib/Headers/wasm_simd128.h:42
+
+// v128 wasm_v128_load(void* mem)
+static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_v128_load(const void *__mem) {

This comment (and similar throughout the file) don't seem to be adding any new 
information.



Comment at: clang/lib/Headers/wasm_simd128.h:87
+  struct __wasm_v64x2_load_splat_struct {
+long long __v;
+  } __attribute__((__packed__, __may_alias__));

Similar to above, these can use the fixed-size type names like `int64_t`.



Comment at: clang/lib/Headers/wasm_simd128.h:179
+int8_t c7, int8_t c8, int8_t c9, int8_t c10, int8_t c11, int8_t c12,
+int8_t c13, int8_t c14, int8_t c15) {
+  return (v128_t)(__i8x16){c0, c1, c2,  c3,  c4,  c5,  c6,  c7,

`c0`, `c1`, etc. aren't reserved identifiers, so the convention in other clang 
headers is to prefix them with `__`.



Comment at: clang/lib/Headers/wasm_simd128.h:293
+// v128_t wasm_i8x16_splat(int8_t a)
+static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_i8x16_splat(int8_t a) {
+  return (v128_t)(__i8x16){a, a, a, a, a, a, a, a, a, a, a, a, a, a, a, a};

`a`, `b`, etc. also aren't reserved identifiers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76959



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76547: [WebAssembly] Add wasm-exported function attribute

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

Instead of creating a new LLVM-IR-level attribute here, could you have clang 
translate the attribute to "wasm-export-name", to keep the LLVM-IR level 
simpler?

Also, I myself would be more comfortable with this change if it were restricted 
to Emscripten for now. `export_name` already exists and works in both 
Emscripten and non-Emscripten targets. If there's demand for this new syntax 
outside of Emscripten, I'm happy to reconsider, but until then it seems better 
to be conservative. Obviously it's not possible to completely prevent people 
from becoming dependent on C++ ABI details, but we can avoid giving them tools 
that make it easy to do the wrong thing. And we can keep the ecosystem simpler 
if we don't have multiple ways to do the same thing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76547



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70700: [WebAssembly] Mangle the argc/argv `main` as `__main_argc_argv`

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

@sbc100 Friendly ping :-).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70700



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59520: [WebAssembly] Address review comments on r352930

[Dan Gohman via Phabricator via cfe-commits]

sunfish marked 10 inline comments as done.
sunfish added a comment.

I apologize for the extraordinary delays here; at long last, I've now addressed 
your feedback.




Comment at: lib/Sema/SemaDeclAttr.cpp:5781-5783
+Sema::mergeImportNameAttr(Decl *D, SourceRange Range,
+  StringRef Name,
+  unsigned AttrSpellingListIndex) {

aaron.ballman wrote:
> I wonder if we want to generalize this with a template (for the attribute 
> type) if we could generalize the diagnostic text a bit more (or add an 
> additional parameter for it)?
The diagnostic name is now generalized, but it seems awkward to pass around the 
numbers to pick which variant of the string to use, so I didn't implement this 
yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59520



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59520: [WebAssembly] Address review comments on r352930

[Dan Gohman via Phabricator via cfe-commits]

sunfish updated this revision to Diff 235005.
sunfish added a comment.

Address review feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59520

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Sema/attr-wasm.c

Index: clang/test/Sema/attr-wasm.c
===
--- /dev/null
+++ clang/test/Sema/attr-wasm.c
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -triple wasm32-unknown-unknown -fsyntax-only -verify %s
+
+void name_a() __attribute__((import_name)); //expected-error {{'import_name' attribute takes one argument}}
+
+int name_b __attribute__((import_name("foo"))); //expected-error {{'import_name' attribute only applies to functions}}
+
+void name_c() __attribute__((import_name("foo", "bar"))); //expected-error {{'import_name' attribute takes one argument}}
+
+void name_d() __attribute__((import_name("foo", "bar", "qux"))); //expected-error {{'import_name' attribute takes one argument}}
+
+__attribute__((import_name("foo"))) void name_e() {} //FIXME-expected-error {{import name cannot be applied to a function with a definition}}
+
+void name_z() __attribute__((import_name("foo"))); //expected-note {{previous attribute is here}}
+
+void name_z() __attribute__((import_name("bar"))); //expected-warning {{import name does not match previous declaration}}
+
+void module_a() __attribute__((import_module)); //expected-error {{'import_module' attribute takes one argument}}
+
+int module_b __attribute__((import_module("foo"))); //expected-error {{'import_module' attribute only applies to functions}}
+
+void module_c() __attribute__((import_module("foo", "bar"))); //expected-error {{'import_module' attribute takes one argument}}
+
+void module_d() __attribute__((import_module("foo", "bar", "qux"))); //expected-error {{'import_module' attribute takes one argument}}
+
+void module_e() __attribute__((import_module("foo"))) {} //FIXME-expected-error {{import module cannot be applied to a function with a definition}}
+
+void module_z() __attribute__((import_module("foo"))); //expected-note {{previous attribute is here}}
+
+void module_z() __attribute__((import_module("bar"))); //expected-warning {{import module does not match previous declaration}}
+
+void both() __attribute__((import_name("foo"), import_module("bar")));
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -5776,45 +5776,73 @@
   D->addAttr(UsedAttr::CreateImplicit(S.Context));
 }
 
-static void handleWebAssemblyImportModuleAttr(Sema , Decl *D, const ParsedAttr ) {
-  if (!isFunctionOrMethod(D)) {
-S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)
-<< "'import_module'" << ExpectedFunction;
-return;
+WebAssemblyImportModuleAttr *
+Sema::mergeImportModuleAttr(Decl *D, const WebAssemblyImportModuleAttr ) {
+  auto *FD = cast(D);
+
+  if (const auto *ExistingAttr = FD->getAttr()) {
+if (ExistingAttr->getImportModule() == AL.getImportModule())
+  return nullptr;
+Diag(ExistingAttr->getLocation(), diag::warn_mismatched_import) << 0;
+Diag(AL.getLoc(), diag::note_previous_attribute);
+return nullptr;
+  }
+  if (FD->hasBody()) {
+Diag(AL.getLoc(), diag::warn_import_on_definition) << 0;
+return nullptr;
   }
+  return ::new (Context) WebAssemblyImportModuleAttr(Context, AL,
+ AL.getImportModule());
+}
 
+WebAssemblyImportNameAttr *
+Sema::mergeImportNameAttr(Decl *D, const WebAssemblyImportNameAttr ) {
   auto *FD = cast(D);
-  if (FD->isThisDeclarationADefinition()) {
-S.Diag(D->getLocation(), diag::err_alias_is_definition) << FD << 0;
-return;
+
+  if (const auto *ExistingAttr = FD->getAttr()) {
+if (ExistingAttr->getImportName() == AL.getImportName())
+  return nullptr;
+Diag(ExistingAttr->getLocation(), diag::warn_mismatched_import) << 1;
+Diag(AL.getLoc(), diag::note_previous_attribute);
+return nullptr;
+  }
+  if (FD->hasBody()) {
+Diag(AL.getLoc(), diag::warn_import_on_definition) << 1;
+return nullptr;
   }
+  return ::new (Context) WebAssemblyImportNameAttr(Context, AL,
+   AL.getImportName());
+}
+
+static void
+handleWebAssemblyImportModuleAttr(Sema , Decl *D, const ParsedAttr ) {
+  auto *FD = cast(D);
 
   StringRef Str;
   SourceLocation ArgLoc;
   if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, ))
 return;
+  if (FD->hasBody()) {
+S.Diag(AL.getLoc(), diag::warn_import_on_definition) << 0;
+return;
+  }
 
   FD->addAttr(::new (S.Context)
   WebAssemblyImportModuleAttr(S.Context, AL, Str));
 }
 
-static void handleWebAssemblyImportNameAttr(Sema , Decl 

[PATCH] D70700: [WebAssembly] Mangle the argc/argv `main` as `__wasm_argc_argv`

[Dan Gohman via Phabricator via cfe-commits]

sunfish updated this revision to Diff 234942.
sunfish added a comment.

To support a transition to the new system, temporarily define a `__main_void` 
alias for no-argument `main`. This allows libc to detect whether it's calling 
old-style or new-style main and do the right thing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70700

Files:
  clang/lib/AST/Mangle.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Frontend/InitHeaderSearch.cpp
  clang/test/CodeGen/wasm-call-main.c
  clang/test/CodeGen/wasm-main.c
  clang/test/CodeGen/wasm-main_argc_argv.c
  llvm/include/llvm/ADT/Triple.h

Index: llvm/include/llvm/ADT/Triple.h
===
--- llvm/include/llvm/ADT/Triple.h
+++ llvm/include/llvm/ADT/Triple.h
@@ -730,6 +730,11 @@
 return getArch() == Triple::riscv32 || getArch() == Triple::riscv64;
   }
 
+  /// Tests whether the target is wasm (32- and 64-bit).
+  bool isWasm() const {
+return getArch() == Triple::wasm32 || getArch() == Triple::wasm64;
+  }
+
   /// Tests whether the target supports comdat
   bool supportsCOMDAT() const {
 return !isOSBinFormatMachO();
Index: clang/test/CodeGen/wasm-main_argc_argv.c
===
--- /dev/null
+++ clang/test/CodeGen/wasm-main_argc_argv.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple wasm32 -o - -emit-llvm %s | FileCheck %s
+
+// Mangle the argc/argv form of main.
+
+int main(int argc, char **argv) {
+  return 0;
+}
+
+// CHECK-LABEL: define i32 @__main_argc_argv(i32 %argc, i8** %argv)
Index: clang/test/CodeGen/wasm-main.c
===
--- /dev/null
+++ clang/test/CodeGen/wasm-main.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple wasm32 -o - -emit-llvm %s | FileCheck %s
+
+// Don't mangle the no-arg form of main.
+
+int main(void) {
+  return 0;
+}
+
+// CHECK-LABEL: define i32 @main()
Index: clang/test/CodeGen/wasm-call-main.c
===
--- /dev/null
+++ clang/test/CodeGen/wasm-call-main.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -triple wasm32 -o - -emit-llvm %s | FileCheck %s
+
+// Mangle argc/argv main even when it's not defined in this TU.
+
+#include 
+
+int main(int argc, char *argv[]);
+
+int foo(void) {
+return main(0, NULL);
+}
+
+// CHECK: call i32 @__main_argc_argv(
Index: clang/lib/Frontend/InitHeaderSearch.cpp
===
--- clang/lib/Frontend/InitHeaderSearch.cpp
+++ clang/lib/Frontend/InitHeaderSearch.cpp
@@ -435,8 +435,7 @@
 break;
 
   case llvm::Triple::UnknownOS:
-if (triple.getArch() == llvm::Triple::wasm32 ||
-triple.getArch() == llvm::Triple::wasm64)
+if (triple.isWasm())
   return;
 break;
   }
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -1012,6 +1012,9 @@
   /// for the uninstrumented functions.
   void EmitDeferredUnusedCoverageMappings();
 
+  /// Emit an alias for "main" if it has no arguments (needed for wasm).
+  void EmitMainVoidAlias();
+
   /// Tell the consumer that this variable has been instantiated.
   void HandleCXXStaticMemberVarInstantiation(VarDecl *VD);
 
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -447,6 +447,10 @@
 CodeGenFunction(*this).EmitCfiCheckStub();
   }
   emitAtAvailableLinkGuard();
+  if (Context.getTargetInfo().getTriple().isWasm() &&
+  !Context.getTargetInfo().getTriple().isOSEmscripten()) {
+EmitMainVoidAlias();
+  }
   emitLLVMUsed();
   if (SanStats)
 SanStats->finish();
@@ -5579,6 +5583,17 @@
   }
 }
 
+void CodeGenModule::EmitMainVoidAlias() {
+  // In order to transition away from "__original_main" gracefully, emit an
+  // alias for "main" in the no-argument case so that libc can detect when
+  // new-style no-argument main is in used.
+  if (llvm::Function *F = getModule().getFunction("main")) {
+if (!F->isDeclaration() && F->arg_size() == 0 && !F->isVarArg() &&
+F->getReturnType()->isIntegerTy(Context.getTargetInfo().getIntWidth()))
+  addUsedGlobal(llvm::GlobalAlias::create("__main_void", F));
+  }
+}
+
 /// Turns the given pointer into a constant.
 static llvm::Constant *GetPointerConstant(llvm::LLVMContext ,
   const void *Ptr) {
Index: clang/lib/AST/Mangle.cpp
===
--- clang/lib/AST/Mangle.cpp
+++ clang/lib/AST/Mangle.cpp
@@ -50,7 +50,8 @@
   CCM_Fast,
   CCM_RegCall,
   CCM_Vector,
-  CCM_Std
+  CCM_Std,
+  CCM_WasmMainArgcArgv
 };
 
 

[PATCH] D70700: [WebAssembly] Mangle the argc/argv `main` as `__wasm_argc_argv`

[Dan Gohman via Phabricator via cfe-commits]

sunfish updated this revision to Diff 234426.
sunfish added reviewers: sbc100, dschuff, aheejin.
sunfish added a comment.

This updates the `main` vs `__main_argc_argv` patch so that it doesn't apply to 
Emscripten targets, following the discussion in 
https://github.com/WebAssembly/tool-conventions/pull/134.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70700

Files:
  clang/lib/AST/Mangle.cpp
  clang/lib/Frontend/InitHeaderSearch.cpp
  clang/test/CodeGen/wasm-call-main.c
  clang/test/CodeGen/wasm-main.c
  clang/test/CodeGen/wasm-main_argc_argv.c
  llvm/include/llvm/ADT/Triple.h

Index: llvm/include/llvm/ADT/Triple.h
===
--- llvm/include/llvm/ADT/Triple.h
+++ llvm/include/llvm/ADT/Triple.h
@@ -730,6 +730,11 @@
 return getArch() == Triple::riscv32 || getArch() == Triple::riscv64;
   }
 
+  /// Tests whether the target is wasm (32- and 64-bit).
+  bool isWasm() const {
+return getArch() == Triple::wasm32 || getArch() == Triple::wasm64;
+  }
+
   /// Tests whether the target supports comdat
   bool supportsCOMDAT() const {
 return !isOSBinFormatMachO();
Index: clang/test/CodeGen/wasm-main_argc_argv.c
===
--- /dev/null
+++ clang/test/CodeGen/wasm-main_argc_argv.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple wasm32 -o - -emit-llvm %s | FileCheck %s
+
+// Mangle the argc/argv form of main.
+
+int main(int argc, char **argv) {
+  return 0;
+}
+
+// CHECK-LABEL: define i32 @__main_argc_argv(i32 %argc, i8** %argv)
Index: clang/test/CodeGen/wasm-main.c
===
--- /dev/null
+++ clang/test/CodeGen/wasm-main.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple wasm32 -o - -emit-llvm %s | FileCheck %s
+
+// Don't mangle the no-arg form of main.
+
+int main(void) {
+  return 0;
+}
+
+// CHECK-LABEL: define i32 @main()
Index: clang/test/CodeGen/wasm-call-main.c
===
--- /dev/null
+++ clang/test/CodeGen/wasm-call-main.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -triple wasm32 -o - -emit-llvm %s | FileCheck %s
+
+// Mangle argc/argv main even when it's not defined in this TU.
+
+#include 
+
+int main(int argc, char *argv[]);
+
+int foo(void) {
+return main(0, NULL);
+}
+
+// CHECK: call i32 @__main_argc_argv(
Index: clang/lib/Frontend/InitHeaderSearch.cpp
===
--- clang/lib/Frontend/InitHeaderSearch.cpp
+++ clang/lib/Frontend/InitHeaderSearch.cpp
@@ -435,8 +435,7 @@
 break;
 
   case llvm::Triple::UnknownOS:
-if (triple.getArch() == llvm::Triple::wasm32 ||
-triple.getArch() == llvm::Triple::wasm64)
+if (triple.isWasm())
   return;
 break;
   }
Index: clang/lib/AST/Mangle.cpp
===
--- clang/lib/AST/Mangle.cpp
+++ clang/lib/AST/Mangle.cpp
@@ -50,7 +50,8 @@
   CCM_Fast,
   CCM_RegCall,
   CCM_Vector,
-  CCM_Std
+  CCM_Std,
+  CCM_WasmMainArgcArgv
 };
 
 static bool isExternC(const NamedDecl *ND) {
@@ -63,6 +64,16 @@
  const NamedDecl *ND) {
   const TargetInfo  = Context.getTargetInfo();
   const llvm::Triple  = TI.getTriple();
+
+  // On wasm, the argc/argv form of "main" is renamed so that the startup code
+  // can call it with the correct function signature.
+  // On Emscripten, users may be exporting "main" and expecting to call it
+  // themselves, so we can't mangle it.
+  if (Triple.isWasm() && !Triple.isOSEmscripten())
+if (const FunctionDecl *FD = dyn_cast(ND))
+  if (FD->isMain() && FD->hasPrototype() && FD->param_size() == 2)
+return CCM_WasmMainArgcArgv;
+
   if (!Triple.isOSWindows() ||
   !(Triple.getArch() == llvm::Triple::x86 ||
 Triple.getArch() == llvm::Triple::x86_64))
@@ -145,6 +156,12 @@
 
   const ASTContext  = getASTContext();
   CCMangling CC = getCallingConvMangling(ASTContext, D);
+
+  if (CC == CCM_WasmMainArgcArgv) {
+Out << "__main_argc_argv";
+return;
+  }
+
   bool MCXX = shouldMangleCXXName(D);
   const TargetInfo  = Context.getTargetInfo();
   if (CC == CCM_Other || (MCXX && TI.getCXXABI() == TargetCXXABI::Microsoft)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71493: [WebAssembly] Setting export_name implies no_dead_strip

[Dan Gohman via Phabricator via cfe-commits]

sunfish added inline comments.



Comment at: llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp:725
   TOut.emitExportName(WasmSym, ExportName);
+  Out.EmitSymbolAttribute(WasmSym, MCSA_NoDeadStrip);
 }

It feels like this is a little inconsistent. If `export_name` doesn't 
automatically imply `used` in LLVM IR or .o files, why does it do so in .s 
files?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71493



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

In D70500#1759831 , @ilya-biryukov 
wrote:

> I'm not an expert in driver code and how it should behave, but being 
> consistent with how other tools are found definitely looks much better than 
> special-casing a single tool.
>  Unless there are reasons why `wasm-opt` should be special, why not use the 
> mechanism used by all other tools?


@dschuff also suggested this approach offline, so it sounds like the way to go. 
I've now submitted https://reviews.llvm.org/D70780 which implements this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70780: [WebAssembly] Find wasm-opt with GetProgramPath

[Dan Gohman via Phabricator via cfe-commits]

sunfish created this revision.
sunfish added reviewers: dschuff, ilya-biryukov.
Herald added subscribers: cfe-commits, aheejin, jgravelle-google, sbc100.
Herald added a project: clang.

Instead of just searching for wasm-opt in PATH, use `GetProgramPath`, which 
checks the `COMPILER_PATH` environment variable, `-B` paths, and `PATH.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70780

Files:
  clang/lib/Driver/ToolChains/WebAssembly.cpp


Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -92,10 +92,10 @@
 
   C.addCommand(std::make_unique(JA, *this, Linker, CmdArgs, Inputs));
 
-  // When optimizing, if wasm-opt is in the PATH, run wasm-opt.
+  // When optimizing, if wasm-opt is available, run it.
   if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
-if (llvm::ErrorOr WasmOptPath =
-   llvm::sys::findProgramByName("wasm-opt")) {
+auto WasmOptPath = getToolChain().GetProgramPath("wasm-opt");
+if (WasmOptPath != "wasm-opt") {
   StringRef OOpt = "s";
   if (A->getOption().matches(options::OPT_O4) ||
   A->getOption().matches(options::OPT_Ofast))
@@ -106,7 +106,7 @@
 OOpt = A->getValue();
 
   if (OOpt != "0") {
-const char *WasmOpt = Args.MakeArgString(*WasmOptPath);
+const char *WasmOpt = Args.MakeArgString(WasmOptPath);
 ArgStringList CmdArgs;
 CmdArgs.push_back(Output.getFilename());
 CmdArgs.push_back(Args.MakeArgString(llvm::Twine("-O") + OOpt));


Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -92,10 +92,10 @@
 
   C.addCommand(std::make_unique(JA, *this, Linker, CmdArgs, Inputs));
 
-  // When optimizing, if wasm-opt is in the PATH, run wasm-opt.
+  // When optimizing, if wasm-opt is available, run it.
   if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
-if (llvm::ErrorOr WasmOptPath =
-   llvm::sys::findProgramByName("wasm-opt")) {
+auto WasmOptPath = getToolChain().GetProgramPath("wasm-opt");
+if (WasmOptPath != "wasm-opt") {
   StringRef OOpt = "s";
   if (A->getOption().matches(options::OPT_O4) ||
   A->getOption().matches(options::OPT_Ofast))
@@ -106,7 +106,7 @@
 OOpt = A->getValue();
 
   if (OOpt != "0") {
-const char *WasmOpt = Args.MakeArgString(*WasmOptPath);
+const char *WasmOpt = Args.MakeArgString(WasmOptPath);
 ArgStringList CmdArgs;
 CmdArgs.push_back(Output.getFilename());
 CmdArgs.push_back(Args.MakeArgString(llvm::Twine("-O") + OOpt));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70687: [WebAssembly] Add an llvm-lto path for compiler-rt.

[Dan Gohman via Phabricator via cfe-commits]

sunfish abandoned this revision.
sunfish added a comment.

I've done some more experimenting with this, and it turns out not to work very 
well, because calls to these functions get generated after LTO runs, so I'll 
abandon this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70687



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70700: [WebAssembly] Mangle the argc/argv `main` as `__wasm_argc_argv`

[Dan Gohman via Phabricator via cfe-commits]

sunfish created this revision.
Herald added subscribers: llvm-commits, dexonsmith, aheejin, jgravelle-google, 
sbc100, mehdi_amini, dschuff.
Herald added projects: clang, LLVM.

WebAssembly enforces a rule that caller and callee signatures must
match. This means that the traditional technique of passing `main`
`argc` and `argv` even when it doesn't need them doesn't work.

Currently the backend renames `main` to `__original_main`, however this
doesn't interact well with LTO'ing libc, and the name isn't intuitive.
This patch allows us to transition to `__main_argc_argv` instead.

This is currently being discussed in 
https://github.com/WebAssembly/tool-conventions/pull/134.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70700

Files:
  clang/lib/AST/Mangle.cpp
  clang/lib/Frontend/InitHeaderSearch.cpp
  clang/test/CodeGen/wasm-call-main.c
  clang/test/CodeGen/wasm-main.c
  clang/test/CodeGen/wasm-main_argc_argv.c
  llvm/include/llvm/ADT/Triple.h

Index: llvm/include/llvm/ADT/Triple.h
===
--- llvm/include/llvm/ADT/Triple.h
+++ llvm/include/llvm/ADT/Triple.h
@@ -730,6 +730,11 @@
 return getArch() == Triple::riscv32 || getArch() == Triple::riscv64;
   }
 
+  /// Tests whether the target is wasm (32- and 64-bit).
+  bool isWasm() const {
+return getArch() == Triple::wasm32 || getArch() == Triple::wasm64;
+  }
+
   /// Tests whether the target supports comdat
   bool supportsCOMDAT() const {
 return !isOSBinFormatMachO();
Index: clang/test/CodeGen/wasm-main_argc_argv.c
===
--- /dev/null
+++ clang/test/CodeGen/wasm-main_argc_argv.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple wasm32 -o - -emit-llvm %s | FileCheck %s
+
+// Mangle the argc/argv form of main.
+
+int main(int argc, char **argv) {
+  return 0;
+}
+
+// CHECK-LABEL: define i32 @__main_argc_argv(i32 %argc, i8** %argv)
Index: clang/test/CodeGen/wasm-main.c
===
--- /dev/null
+++ clang/test/CodeGen/wasm-main.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple wasm32 -o - -emit-llvm %s | FileCheck %s
+
+// Don't mangle the no-arg form of main.
+
+int main(void) {
+  return 0;
+}
+
+// CHECK-LABEL: define i32 @main()
Index: clang/test/CodeGen/wasm-call-main.c
===
--- /dev/null
+++ clang/test/CodeGen/wasm-call-main.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -triple wasm32 -o - -emit-llvm %s | FileCheck %s
+
+// Mangle argc/argv main even when it's not defined in this TU.
+
+#include 
+
+int main(int argc, char *argv[]);
+
+int foo(void) {
+return main(0, NULL);
+}
+
+// CHECK: call i32 @__main_argc_argv(
Index: clang/lib/Frontend/InitHeaderSearch.cpp
===
--- clang/lib/Frontend/InitHeaderSearch.cpp
+++ clang/lib/Frontend/InitHeaderSearch.cpp
@@ -435,8 +435,7 @@
 break;
 
   case llvm::Triple::UnknownOS:
-if (triple.getArch() == llvm::Triple::wasm32 ||
-triple.getArch() == llvm::Triple::wasm64)
+if (triple.isWasm())
   return;
 break;
   }
Index: clang/lib/AST/Mangle.cpp
===
--- clang/lib/AST/Mangle.cpp
+++ clang/lib/AST/Mangle.cpp
@@ -50,7 +50,8 @@
   CCM_Fast,
   CCM_RegCall,
   CCM_Vector,
-  CCM_Std
+  CCM_Std,
+  CCM_WasmMainArgcArgv
 };
 
 static bool isExternC(const NamedDecl *ND) {
@@ -63,6 +64,14 @@
  const NamedDecl *ND) {
   const TargetInfo  = Context.getTargetInfo();
   const llvm::Triple  = TI.getTriple();
+
+  // On wasm, the argc/argv form of "main" is renamed so that the startup code
+  // can call it with the correct function signature.
+  if (Triple.isWasm())
+if (const FunctionDecl *FD = dyn_cast(ND))
+  if (FD->isMain() && FD->hasPrototype() && FD->param_size() == 2)
+return CCM_WasmMainArgcArgv;
+
   if (!Triple.isOSWindows() ||
   !(Triple.getArch() == llvm::Triple::x86 ||
 Triple.getArch() == llvm::Triple::x86_64))
@@ -145,6 +154,12 @@
 
   const ASTContext  = getASTContext();
   CCMangling CC = getCallingConvMangling(ASTContext, D);
+
+  if (CC == CCM_WasmMainArgcArgv) {
+Out << "__main_argc_argv";
+return;
+  }
+
   bool MCXX = shouldMangleCXXName(D);
   const TargetInfo  = Context.getTargetInfo();
   if (CC == CCM_Other || (MCXX && TI.getCXXABI() == TargetCXXABI::Microsoft)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70687: [WebAssembly] Add an llvm-lto path for compiler-rt.

[Dan Gohman via Phabricator via cfe-commits]

sunfish created this revision.
Herald added subscribers: cfe-commits, dexonsmith, aheejin, jgravelle-google, 
inglorion, sbc100, mehdi_amini, dberris, dschuff.
Herald added a project: clang.

This allows LTO-enabled builds of compiler-rt to be used.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70687

Files:
  clang/lib/Driver/ToolChains/WebAssembly.cpp
  clang/lib/Driver/ToolChains/WebAssembly.h


Index: clang/lib/Driver/ToolChains/WebAssembly.h
===
--- clang/lib/Driver/ToolChains/WebAssembly.h
+++ clang/lib/Driver/ToolChains/WebAssembly.h
@@ -71,6 +71,7 @@
   const char *getDefaultLinker() const override { return "wasm-ld"; }
 
   Tool *buildLinker() const override;
+  std::string getCompilerRTPath() const;
 };
 
 } // end namespace toolchains
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -155,6 +155,19 @@
   }
 }
 
+std::string WebAssembly::getCompilerRTPath() const {
+  // If we're doing LTO and we have an LTO library available, use it.
+  const auto  = getDriver();
+  if (D.isUsingLTO()) {
+auto Dir = AppendLTOLibDir(ToolChain::getCompilerRTPath());
+if (llvm::sys::fs::exists(Dir))
+  return Dir;
+  }
+
+  // Otherwise just use the default.
+  return ToolChain::getCompilerRTPath();
+}
+
 bool WebAssembly::IsMathErrnoDefault() const { return false; }
 
 bool WebAssembly::IsObjCNonFragileABIDefault() const { return true; }


Index: clang/lib/Driver/ToolChains/WebAssembly.h
===
--- clang/lib/Driver/ToolChains/WebAssembly.h
+++ clang/lib/Driver/ToolChains/WebAssembly.h
@@ -71,6 +71,7 @@
   const char *getDefaultLinker() const override { return "wasm-ld"; }
 
   Tool *buildLinker() const override;
+  std::string getCompilerRTPath() const;
 };
 
 } // end namespace toolchains
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -155,6 +155,19 @@
   }
 }
 
+std::string WebAssembly::getCompilerRTPath() const {
+  // If we're doing LTO and we have an LTO library available, use it.
+  const auto  = getDriver();
+  if (D.isUsingLTO()) {
+auto Dir = AppendLTOLibDir(ToolChain::getCompilerRTPath());
+if (llvm::sys::fs::exists(Dir))
+  return Dir;
+  }
+
+  // Otherwise just use the default.
+  return ToolChain::getCompilerRTPath();
+}
+
 bool WebAssembly::IsMathErrnoDefault() const { return false; }
 
 bool WebAssembly::IsObjCNonFragileABIDefault() const { return true; }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

I've now posted https://reviews.llvm.org/D70677 which should fix the test 
failure when `LLVM_APPEND_VC_REV=NO` is set.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70677: [WebAssembly] Change the llvm-lto dir to use the LLVM Version

[Dan Gohman via Phabricator via cfe-commits]

sunfish created this revision.
sunfish added reviewers: sbc100, dschuff.
Herald added subscribers: cfe-commits, dexonsmith, aheejin, jgravelle-google, 
inglorion.
Herald added a project: clang.

Using the version instead of the VCS revision, which isn't available when 
LLVM_APPEND_VC_REV is set. The bitcode format should be backwards-compatible at 
least within a minor version, so the version string should be sufficient.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70677

Files:
  clang/lib/Driver/ToolChains/WebAssembly.cpp


Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -119,14 +119,11 @@
 }
 
 /// Given a base library directory, append path components to form the
-/// LTO directory. The LLVMRevision allows the path to be keyed to the
-/// specific version of LLVM in used, as the bitcode format is not stable.
-static std::string AppendLTOLibDir(const std::string ,
-   const std::string ) {
-if (LLVMRevision.empty()) {
-return Dir;
-}
-return Dir + "/llvm-lto/" + LLVMRevision;
+/// LTO directory.
+static std::string AppendLTOLibDir(const std::string ) {
+// The version allows the path to be keyed to the specific version of
+// LLVM in used, as the bitcode format is not stable.
+return Dir + "/llvm-lto/" LLVM_VERSION_STRING;
 }
 
 WebAssembly::WebAssembly(const Driver , const llvm::Triple ,
@@ -148,15 +145,11 @@
 getMultiarchTriple(getDriver(), Triple, getDriver().SysRoot);
 auto Sysroot = getDriver().SysRoot;
 if (D.isUsingLTO()) {
-  auto LLVMRevision = getLLVMRevision();
-  if (!LLVMRevision.empty()) {
-// For LTO, enable use of lto-enabled sysroot libraries too, if 
available.
-// Note that the directory is keyed to the LLVM revision, as LLVM's
-// bitcode format is not stable.
-auto Dir = AppendLTOLibDir(Sysroot + "/lib/" + MultiarchTriple,
-   LLVMRevision);
-getFilePaths().push_back(Dir);
-  }
+  // For LTO, enable use of lto-enabled sysroot libraries too, if 
available.
+  // Note that the directory is keyed to the LLVM revision, as LLVM's
+  // bitcode format is not stable.
+  auto Dir = AppendLTOLibDir(Sysroot + "/lib/" + MultiarchTriple);
+  getFilePaths().push_back(Dir);
 }
 getFilePaths().push_back(Sysroot + "/lib/" + MultiarchTriple);
   }
@@ -166,12 +159,9 @@
   // If we're doing LTO and we have an LTO library available, use it.
   const auto  = getDriver();
   if (D.isUsingLTO()) {
-auto LLVMRevision = getLLVMRevision();
-if (!LLVMRevision.empty()) {
-  auto Dir = AppendLTOLibDir(ToolChain::getCompilerRTPath(), LLVMRevision);
-  if (llvm::sys::fs::exists(Dir))
-return Dir;
-}
+auto Dir = AppendLTOLibDir(ToolChain::getCompilerRTPath());
+if (llvm::sys::fs::exists(Dir))
+  return Dir;
   }
 
   // Otherwise just use the default.


Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -119,14 +119,11 @@
 }
 
 /// Given a base library directory, append path components to form the
-/// LTO directory. The LLVMRevision allows the path to be keyed to the
-/// specific version of LLVM in used, as the bitcode format is not stable.
-static std::string AppendLTOLibDir(const std::string ,
-   const std::string ) {
-if (LLVMRevision.empty()) {
-return Dir;
-}
-return Dir + "/llvm-lto/" + LLVMRevision;
+/// LTO directory.
+static std::string AppendLTOLibDir(const std::string ) {
+// The version allows the path to be keyed to the specific version of
+// LLVM in used, as the bitcode format is not stable.
+return Dir + "/llvm-lto/" LLVM_VERSION_STRING;
 }
 
 WebAssembly::WebAssembly(const Driver , const llvm::Triple ,
@@ -148,15 +145,11 @@
 getMultiarchTriple(getDriver(), Triple, getDriver().SysRoot);
 auto Sysroot = getDriver().SysRoot;
 if (D.isUsingLTO()) {
-  auto LLVMRevision = getLLVMRevision();
-  if (!LLVMRevision.empty()) {
-// For LTO, enable use of lto-enabled sysroot libraries too, if available.
-// Note that the directory is keyed to the LLVM revision, as LLVM's
-// bitcode format is not stable.
-auto Dir = AppendLTOLibDir(Sysroot + "/lib/" + MultiarchTriple,
-   LLVMRevision);
-getFilePaths().push_back(Dir);
-  }
+  // For LTO, enable use of lto-enabled sysroot libraries too, if available.
+  // Note that the directory is keyed to the LLVM revision, as LLVM's
+  // bitcode format is not stable.
+  auto Dir = AppendLTOLibDir(Sysroot + 

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Dan Gohman via Phabricator via cfe-commits]

sunfish marked an inline comment as done.
sunfish added a comment.

In D70500#1757735 , @thakis wrote:

> Please don't add code to the driver that runs programs off PATH. Nothing else 
> does this.


Clang's normal `GetProgramPath` does do this: 
https://github.com/llvm/llvm-project/blob/master/clang/lib/Driver/Driver.cpp#L4719

> If you need to run an external program, look for it next to the compiler, 
> like we do for gas with -fno-integrated-as, linker, etc. People can use the 
> existing -B flag to make clang look elsewhere if they need that.

Unfortunately, wasm-opt isn't typically installed alongside the compiler. It's 
also not a typical system tool like an assembler, which is why it didn't seem 
right to search the -B paths.

The first version of my patch used a dedicated environment variable, WASM_OPT, 
rather than PATH, but I changed it to PATH after review feedback. If people 
feel using -B paths, and COMPILER_PATH, are appropriate, we can use 
`GetProgramPath` itself, though note that that still does have a PATH fallback.




Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:137
+  getFilePaths().push_back(getDriver().SysRoot + "/lib/" + MultiarchTriple 
+
+   "/llvm-lto");
+}

thakis wrote:
> sunfish wrote:
> > sbc100 wrote:
> > > Is there any kind of president here?   i.e. do any other platforms have 
> > > support for this kind thing?  Seems like good idea I'd just like to be 
> > > sure we follow existing practices.
> > I looked around, but didn't see anything similar.
> One immediate problem of this approach is that if HAVE_VCS_VERSION_INC is not 
> set, then the test fails, but if it is set, `clang --version` reports a 
> current git hash, which is either out of date or requires a pretty big 
> rebuild on every single git commit (i.e. not just after `git pull` but also 
> after local commits). Neither's great.
> 
> Do you expected that sysroots will ship 3-4 LTO folders, to support ~2 years 
> of clang releases? Or do you expect that a sysroot will always support a 
> single clang/llvm revision? If so, maybe the LLVM version is enough?
Yes, I think we can switch from the VCS string to the LLVM_VERSION string. The 
documentation says LLVM guarantees bitcode backwards compatibility between 
minor versions, so this should be sufficient. I'll post a new patch for that, 
which should also fix the test when HAVE_VCS_VERSION_INC is false.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

In D70500#1756011 , @dschuff wrote:

> WRT the LTO directory name, there's theoretically the danger that someone 
> (e.g. emscripten) could be doing a rolling release of the compiler and get 
> invalidated within a major revision.


The LLVM revision here is the git hash, not the major version, so we should be 
good even if this happens.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70520: [WebAssembly] Add new `export_name` clang attribute for controlling wasm export names

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

> Do you think that setting this attribute should also prevent GC?

That sounds right to me.

> I could split this up into clang, llvm, and lld parts if it makes reviewing 
> simpler.

For me, it's easier to have it all together in one.

In this patch, `export_name` has a required string operand, which makes sense 
to me. I wonder if it also makes sense to change wasm-ld's `--export` 
command-line option to take a pair of strings, a symbol and a name to export it 
as.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70520



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70520: [WebAssembly] Add new `export_name` clang attribute for controlling wasm export names

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

It appears this doesn't handle exporting an imported function yet, which is 
fine for now, but it would be good to issue a warning, because wasm itself is 
capable of representing this:

  void aaa(void) __attribute__((import_module("imp"), import_name("foo"), 
export_name("bar")));




Comment at: lld/wasm/Writer.cpp:523
+  StringRef exportName = f->function->getExportName();
+  if (!exportName.empty()) {
+name = exportName;

For wasm exports, it's valid to have empty strings. In fact, I may even have a 
usecase which wants an empty-string export. It'd be good to use `Optional<>` 
for export names, rather than special-casing the empty string.

(wasm-ld does often special-case the empty string in symbol names, but wasm 
export strings aren't ordinary symbol names, so it'd be good to follow wasm's 
rules for them.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70520



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Dan Gohman via Phabricator via cfe-commits]

sunfish marked 2 inline comments as done.
sunfish added inline comments.



Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:105
+OOpt = "0";
+  else if (A->getOption().matches(options::OPT_O))
+OOpt = A->getValue();

dschuff wrote:
> This chain is slightly confusing. I assume this `getValue()` captures the 
> number in `-O3`, `-O2`, etc? But why then do we need special-casing for 0 and 
> 4 above?
> 
> For that matter, we should probably not run wasm-opt at all if the opt-level 
> is 0, right?
This isn't a wasm thing; O0 and O4 [are 
special](https://github.com/llvm/llvm-project/blob/master/clang/include/clang/Driver/Options.td#L393).
 See also 
[here](https://github.com/llvm/llvm-project/blob/master/clang/lib/Driver/ToolChains/Cuda.cpp#L368)
 and 
[here](https://github.com/llvm/llvm-project/blob/master/clang/lib/Driver/ToolChains/HIP.cpp#L97)
 for other code which does similar things. The wasm-opt version here is 
actually simpler because we don't need to special-case Os or Oz.

The `if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {` guards against the 
case where no -O option is given, but you're right, we shouldn't run wasm-opt 
if -O0 is given. I'll update the patch.



Comment at: clang/test/Driver/wasm-toolchain-lto.c:6
+// LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi/llvm-lto/

sbc100 wrote:
> Include the final path segment in the expectation?
It's a git revision, so it'd be constantly changing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Dan Gohman via Phabricator via cfe-commits]

sunfish updated this revision to Diff 230364.
sunfish added a comment.

Don't run wasm-opt with -O0.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500

Files:
  clang/lib/Driver/ToolChains/WebAssembly.cpp
  clang/test/Driver/wasm-toolchain-lto.c


Index: clang/test/Driver/wasm-toolchain-lto.c
===
--- /dev/null
+++ clang/test/Driver/wasm-toolchain-lto.c
@@ -0,0 +1,6 @@
+// A basic C link command-line with optimization with known OS and LTO enabled.
+
+// RUN: %clang -### -O2 -flto -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_OPT_KNOWN %s
+// LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi/llvm-lto/
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -8,6 +8,7 @@
 
 #include "WebAssembly.h"
 #include "CommonArgs.h"
+#include "clang/Basic/Version.h"
 #include "clang/Config/config.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
@@ -90,6 +91,31 @@
   CmdArgs.push_back(Output.getFilename());
 
   C.addCommand(std::make_unique(JA, *this, Linker, CmdArgs, Inputs));
+
+  // When optimizing, if wasm-opt is in the PATH, run wasm-opt.
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (llvm::ErrorOr WasmOptPath =
+   llvm::sys::findProgramByName("wasm-opt")) {
+  StringRef OOpt = "s";
+  if (A->getOption().matches(options::OPT_O4) ||
+  A->getOption().matches(options::OPT_Ofast))
+OOpt = "4";
+  else if (A->getOption().matches(options::OPT_O0))
+OOpt = "0";
+  else if (A->getOption().matches(options::OPT_O))
+OOpt = A->getValue();
+
+  if (OOpt != "0") {
+const char *WasmOpt = Args.MakeArgString(*WasmOptPath);
+ArgStringList CmdArgs;
+CmdArgs.push_back(Output.getFilename());
+CmdArgs.push_back(Args.MakeArgString(llvm::Twine("-O") + OOpt));
+CmdArgs.push_back("-o");
+CmdArgs.push_back(Output.getFilename());
+C.addCommand(std::make_unique(JA, *this, WasmOpt, CmdArgs, 
Inputs));
+  }
+}
+  }
 }
 
 WebAssembly::WebAssembly(const Driver , const llvm::Triple ,
@@ -109,6 +135,16 @@
   } else {
 const std::string MultiarchTriple =
 getMultiarchTriple(getDriver(), Triple, getDriver().SysRoot);
+if (D.isUsingLTO()) {
+  auto LLVMRevision = getLLVMRevision();
+  if (!LLVMRevision.empty()) {
+// For LTO, enable use of lto-enabled sysroot libraries too, if 
available.
+// Note that the directory is keyed to the LLVM revision, as LLVM's
+// bitcode format is not stable.
+getFilePaths().push_back(getDriver().SysRoot + "/lib/" + 
MultiarchTriple +
+ "/llvm-lto/" + LLVMRevision);
+  }
+}
 getFilePaths().push_back(getDriver().SysRoot + "/lib/" + MultiarchTriple);
   }
 }


Index: clang/test/Driver/wasm-toolchain-lto.c
===
--- /dev/null
+++ clang/test/Driver/wasm-toolchain-lto.c
@@ -0,0 +1,6 @@
+// A basic C link command-line with optimization with known OS and LTO enabled.
+
+// RUN: %clang -### -O2 -flto -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_OPT_KNOWN %s
+// LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi/llvm-lto/
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -8,6 +8,7 @@
 
 #include "WebAssembly.h"
 #include "CommonArgs.h"
+#include "clang/Basic/Version.h"
 #include "clang/Config/config.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
@@ -90,6 +91,31 @@
   CmdArgs.push_back(Output.getFilename());
 
   C.addCommand(std::make_unique(JA, *this, Linker, CmdArgs, Inputs));
+
+  // When optimizing, if wasm-opt is in the PATH, run wasm-opt.
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (llvm::ErrorOr WasmOptPath =
+   llvm::sys::findProgramByName("wasm-opt")) {
+  StringRef OOpt = "s";
+  if (A->getOption().matches(options::OPT_O4) ||
+  A->getOption().matches(options::OPT_Ofast))
+OOpt = "4";
+  else if (A->getOption().matches(options::OPT_O0))
+OOpt = "0";
+  else if (A->getOption().matches(options::OPT_O))
+OOpt = A->getValue();
+
+  if (OOpt != "0") {
+const char *WasmOpt = Args.MakeArgString(*WasmOptPath);
+ArgStringList 

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Dan Gohman via Phabricator via cfe-commits]

sunfish marked an inline comment as done.
sunfish added inline comments.



Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:96
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (const char *WasmOptPath = getenv("WASM_OPT")) {
+  StringRef OOpt = "s";

dschuff wrote:
> sunfish wrote:
> > dschuff wrote:
> > > sunfish wrote:
> > > > dschuff wrote:
> > > > > sunfish wrote:
> > > > > > dschuff wrote:
> > > > > > > What would you think about adding a way to pass arguments through 
> > > > > > > to wasm-opt on the command line, like we have for the linker, 
> > > > > > > assembler, etc? Something like `-Wo,-O2` (or `-Ww` or whatever; 
> > > > > > > analogous to `-Wl` and `-Wa`). That seems nicer than an env var, 
> > > > > > > although it doesn't solve the problem of controlling whether to 
> > > > > > > run the optimizer in the first place.
> > > > > > My guess here is that we don't need clang to have an option for 
> > > > > > passing additional flags -- if people want to do extra special 
> > > > > > things with wasm-opt on clang's output they can just run wasm-opt 
> > > > > > directly themselves. Does that sound reasonable?
> > > > > Maybe. But I still don't like the use of an env var for this kind of 
> > > > > behavior-effecting (i.e. non-debugging) use case.  It's hard enough 
> > > > > to get reproducible and hermetic build behavior as it is, I 
> > > > > definitely wouldn't want to worry about the environment affecting the 
> > > > > output in addition to all the random flags, included files, etc.
> > > > If we did -Wo,-O2 or so, we'd still need to be able to find the 
> > > > wasm-opt program to be able to run it. We could just search for it in 
> > > > PATH, but that's also a little dicey from a hermetic build perspective.
> > > > 
> > > > I borrowed "WASM_OPT" from 
> > > > [cargo-wasi](https://github.com/bytecodealliance/cargo-wasi). I'm also 
> > > > not a fan of environment variables in general, but this way does have 
> > > > the advantage that people can set it once, and not have to modify their 
> > > > Makefiles to add new flags. Users can think of it as just being part of 
> > > > -O2 and friends.
> > > > 
> > > What's the usual way to locate things like external assemblers? 
> > > Presumably we could use the same mechanism for wasm-opt?
> > It checks the COMPILER_PATH environment variable and -B command-line flags, 
> > which I'm not sure we should use here, but it looks like it does fall back 
> > to checking PATH at the end.
> > 
> > So, what would you think of just checking PATH for wasm-opt?
> I suspect we'll end up with -B flags if/when people start building 
> interesting or nontrivial toolchains with clang (or we try to make emscripten 
> more standardish), but I'm fine with leaving that out for now. Checking PATH 
> for wasm-opt seems fine to me to locate the binary. Did you have that in mind 
> as also the way to determine whether or not to run wasm-opt (i.e. run if it's 
> there, don't if it's not)? That seems slightly error-prone in the sense that 
> there would be a silent behavior change if something went wrong (e.g. 
> wasm-opt goes missing) but in a world where most clang users are using 
> wasm-opt then using wasm-opt by default seems reasonable; so that seems fine 
> as a way to start out.
> 
> I do think we will eventually want some way to modify the behavior of 
> wasm-opt though. For that matter wasm-opt might at some point become able to 
> optimize object files (allowing faster links at the cost of less 
> LTO-optimized binaries; we'd run a reduced set of passes post-link or none at 
> all). In that case there'd also have to be further changes if we want builtin 
> support for that.
Yes, we'd just run wasm-opt if it's in the PATH. See the now updated patch. And 
yeah, this means if your wasm-opt disappears, you silently won't get as much 
optimization, which is a little unfortunate, but it is the most convenient for 
the common use case.

And yeah, we can always add more options in the future :-).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Dan Gohman via Phabricator via cfe-commits]

sunfish updated this revision to Diff 230350.
sunfish added a comment.

Use PATH instead of WASM_OPT to find wasm-opt.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500

Files:
  clang/lib/Driver/ToolChains/WebAssembly.cpp
  clang/test/Driver/wasm-toolchain-lto.c


Index: clang/test/Driver/wasm-toolchain-lto.c
===
--- /dev/null
+++ clang/test/Driver/wasm-toolchain-lto.c
@@ -0,0 +1,6 @@
+// A basic C link command-line with optimization with known OS and LTO enabled.
+
+// RUN: %clang -### -O2 -flto -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_OPT_KNOWN %s
+// LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi/llvm-lto/
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -8,6 +8,7 @@
 
 #include "WebAssembly.h"
 #include "CommonArgs.h"
+#include "clang/Basic/Version.h"
 #include "clang/Config/config.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
@@ -90,6 +91,29 @@
   CmdArgs.push_back(Output.getFilename());
 
   C.addCommand(std::make_unique(JA, *this, Linker, CmdArgs, Inputs));
+
+  // When optimizing, if wasm-opt is in the PATH, run wasm-opt.
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (llvm::ErrorOr WasmOptPath =
+   llvm::sys::findProgramByName("wasm-opt")) {
+  StringRef OOpt = "s";
+  if (A->getOption().matches(options::OPT_O4) ||
+  A->getOption().matches(options::OPT_Ofast))
+OOpt = "4";
+  else if (A->getOption().matches(options::OPT_O0))
+OOpt = "0";
+  else if (A->getOption().matches(options::OPT_O))
+OOpt = A->getValue();
+
+  const char *WasmOpt = Args.MakeArgString(*WasmOptPath);
+  ArgStringList CmdArgs;
+  CmdArgs.push_back(Output.getFilename());
+  CmdArgs.push_back(Args.MakeArgString(llvm::Twine("-O") + OOpt));
+  CmdArgs.push_back("-o");
+  CmdArgs.push_back(Output.getFilename());
+  C.addCommand(std::make_unique(JA, *this, WasmOpt, CmdArgs, 
Inputs));
+}
+  }
 }
 
 WebAssembly::WebAssembly(const Driver , const llvm::Triple ,
@@ -109,6 +133,16 @@
   } else {
 const std::string MultiarchTriple =
 getMultiarchTriple(getDriver(), Triple, getDriver().SysRoot);
+if (D.isUsingLTO()) {
+  auto LLVMRevision = getLLVMRevision();
+  if (!LLVMRevision.empty()) {
+// For LTO, enable use of lto-enabled sysroot libraries too, if 
available.
+// Note that the directory is keyed to the LLVM revision, as LLVM's
+// bitcode format is not stable.
+getFilePaths().push_back(getDriver().SysRoot + "/lib/" + 
MultiarchTriple +
+ "/llvm-lto/" + LLVMRevision);
+  }
+}
 getFilePaths().push_back(getDriver().SysRoot + "/lib/" + MultiarchTriple);
   }
 }


Index: clang/test/Driver/wasm-toolchain-lto.c
===
--- /dev/null
+++ clang/test/Driver/wasm-toolchain-lto.c
@@ -0,0 +1,6 @@
+// A basic C link command-line with optimization with known OS and LTO enabled.
+
+// RUN: %clang -### -O2 -flto -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_OPT_KNOWN %s
+// LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi/llvm-lto/
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -8,6 +8,7 @@
 
 #include "WebAssembly.h"
 #include "CommonArgs.h"
+#include "clang/Basic/Version.h"
 #include "clang/Config/config.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
@@ -90,6 +91,29 @@
   CmdArgs.push_back(Output.getFilename());
 
   C.addCommand(std::make_unique(JA, *this, Linker, CmdArgs, Inputs));
+
+  // When optimizing, if wasm-opt is in the PATH, run wasm-opt.
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (llvm::ErrorOr WasmOptPath =
+   llvm::sys::findProgramByName("wasm-opt")) {
+  StringRef OOpt = "s";
+  if (A->getOption().matches(options::OPT_O4) ||
+  A->getOption().matches(options::OPT_Ofast))
+OOpt = "4";
+  else if (A->getOption().matches(options::OPT_O0))
+OOpt = "0";
+  else if (A->getOption().matches(options::OPT_O))
+OOpt = A->getValue();
+
+  const char *WasmOpt = Args.MakeArgString(*WasmOptPath);
+  ArgStringList CmdArgs;
+  CmdArgs.push_back(Output.getFilename());
+  

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Dan Gohman via Phabricator via cfe-commits]

sunfish marked an inline comment as done.
sunfish added inline comments.



Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:96
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (const char *WasmOptPath = getenv("WASM_OPT")) {
+  StringRef OOpt = "s";

dschuff wrote:
> sunfish wrote:
> > dschuff wrote:
> > > sunfish wrote:
> > > > dschuff wrote:
> > > > > What would you think about adding a way to pass arguments through to 
> > > > > wasm-opt on the command line, like we have for the linker, assembler, 
> > > > > etc? Something like `-Wo,-O2` (or `-Ww` or whatever; analogous to 
> > > > > `-Wl` and `-Wa`). That seems nicer than an env var, although it 
> > > > > doesn't solve the problem of controlling whether to run the optimizer 
> > > > > in the first place.
> > > > My guess here is that we don't need clang to have an option for passing 
> > > > additional flags -- if people want to do extra special things with 
> > > > wasm-opt on clang's output they can just run wasm-opt directly 
> > > > themselves. Does that sound reasonable?
> > > Maybe. But I still don't like the use of an env var for this kind of 
> > > behavior-effecting (i.e. non-debugging) use case.  It's hard enough to 
> > > get reproducible and hermetic build behavior as it is, I definitely 
> > > wouldn't want to worry about the environment affecting the output in 
> > > addition to all the random flags, included files, etc.
> > If we did -Wo,-O2 or so, we'd still need to be able to find the wasm-opt 
> > program to be able to run it. We could just search for it in PATH, but 
> > that's also a little dicey from a hermetic build perspective.
> > 
> > I borrowed "WASM_OPT" from 
> > [cargo-wasi](https://github.com/bytecodealliance/cargo-wasi). I'm also not 
> > a fan of environment variables in general, but this way does have the 
> > advantage that people can set it once, and not have to modify their 
> > Makefiles to add new flags. Users can think of it as just being part of -O2 
> > and friends.
> > 
> What's the usual way to locate things like external assemblers? Presumably we 
> could use the same mechanism for wasm-opt?
It checks the COMPILER_PATH environment variable and -B command-line flags, 
which I'm not sure we should use here, but it looks like it does fall back to 
checking PATH at the end.

So, what would you think of just checking PATH for wasm-opt?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Dan Gohman via Phabricator via cfe-commits]

sunfish marked an inline comment as done.
sunfish added inline comments.



Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:96
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (const char *WasmOptPath = getenv("WASM_OPT")) {
+  StringRef OOpt = "s";

dschuff wrote:
> sunfish wrote:
> > dschuff wrote:
> > > What would you think about adding a way to pass arguments through to 
> > > wasm-opt on the command line, like we have for the linker, assembler, 
> > > etc? Something like `-Wo,-O2` (or `-Ww` or whatever; analogous to `-Wl` 
> > > and `-Wa`). That seems nicer than an env var, although it doesn't solve 
> > > the problem of controlling whether to run the optimizer in the first 
> > > place.
> > My guess here is that we don't need clang to have an option for passing 
> > additional flags -- if people want to do extra special things with wasm-opt 
> > on clang's output they can just run wasm-opt directly themselves. Does that 
> > sound reasonable?
> Maybe. But I still don't like the use of an env var for this kind of 
> behavior-effecting (i.e. non-debugging) use case.  It's hard enough to get 
> reproducible and hermetic build behavior as it is, I definitely wouldn't want 
> to worry about the environment affecting the output in addition to all the 
> random flags, included files, etc.
If we did -Wo,-O2 or so, we'd still need to be able to find the wasm-opt 
program to be able to run it. We could just search for it in PATH, but that's 
also a little dicey from a hermetic build perspective.

I borrowed "WASM_OPT" from 
[cargo-wasi](https://github.com/bytecodealliance/cargo-wasi). I'm also not a 
fan of environment variables in general, but this way does have the advantage 
that people can set it once, and not have to modify their Makefiles to add new 
flags. Users can think of it as just being part of -O2 and friends.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Dan Gohman via Phabricator via cfe-commits]

sunfish updated this revision to Diff 230322.
sunfish added a comment.

In D70500#1754012 , @sbc100 wrote:

> On I just remember why this is probably a bad idea.   llvm bitcode is not 
> designed to be stable,  unlike object files, so its probably not a good idea 
> to encourage the distributing of bitcode files in SDKs and such.


That's a good point. I think we can address this by including the LLVM revision 
in the path --I've now updated the patch to do this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500

Files:
  clang/lib/Driver/ToolChains/WebAssembly.cpp
  clang/test/Driver/wasm-toolchain-lto.c


Index: clang/test/Driver/wasm-toolchain-lto.c
===
--- /dev/null
+++ clang/test/Driver/wasm-toolchain-lto.c
@@ -0,0 +1,6 @@
+// A basic C link command-line with optimization with known OS and LTO enabled.
+
+// RUN: %clang -### -O2 -flto -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_OPT_KNOWN %s
+// LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi/llvm-lto/
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -8,6 +8,7 @@
 
 #include "WebAssembly.h"
 #include "CommonArgs.h"
+#include "clang/Basic/Version.h"
 #include "clang/Config/config.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
@@ -90,6 +91,28 @@
   CmdArgs.push_back(Output.getFilename());
 
   C.addCommand(llvm::make_unique(JA, *this, Linker, CmdArgs, Inputs));
+
+  // When optimizing, if WASM_OPT is set, run wasm-opt.
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (const char *WasmOptPath = getenv("WASM_OPT")) {
+  StringRef OOpt = "s";
+  if (A->getOption().matches(options::OPT_O4) ||
+  A->getOption().matches(options::OPT_Ofast))
+OOpt = "4";
+  else if (A->getOption().matches(options::OPT_O0))
+OOpt = "0";
+  else if (A->getOption().matches(options::OPT_O))
+OOpt = A->getValue();
+
+  const char *WasmOpt = Args.MakeArgString(WasmOptPath);
+  ArgStringList CmdArgs;
+  CmdArgs.push_back(Output.getFilename());
+  CmdArgs.push_back(Args.MakeArgString(llvm::Twine("-O") + OOpt));
+  CmdArgs.push_back("-o");
+  CmdArgs.push_back(Output.getFilename());
+  C.addCommand(llvm::make_unique(JA, *this, WasmOpt, CmdArgs, 
Inputs));
+}
+  }
 }
 
 WebAssembly::WebAssembly(const Driver , const llvm::Triple ,
@@ -109,6 +132,16 @@
   } else {
 const std::string MultiarchTriple =
 getMultiarchTriple(getDriver(), Triple, getDriver().SysRoot);
+if (D.isUsingLTO()) {
+  auto LLVMRevision = getLLVMRevision();
+  if (!LLVMRevision.empty()) {
+// For LTO, enable use of lto-enabled sysroot libraries too, if 
available.
+// Note that the directory is keyed to the LLVM revision, as LLVM's
+// bitcode format is not stable.
+getFilePaths().push_back(getDriver().SysRoot + "/lib/" + 
MultiarchTriple +
+ "/llvm-lto/" + LLVMRevision);
+  }
+}
 getFilePaths().push_back(getDriver().SysRoot + "/lib/" + MultiarchTriple);
   }
 }


Index: clang/test/Driver/wasm-toolchain-lto.c
===
--- /dev/null
+++ clang/test/Driver/wasm-toolchain-lto.c
@@ -0,0 +1,6 @@
+// A basic C link command-line with optimization with known OS and LTO enabled.
+
+// RUN: %clang -### -O2 -flto -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_OPT_KNOWN %s
+// LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi/llvm-lto/
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -8,6 +8,7 @@
 
 #include "WebAssembly.h"
 #include "CommonArgs.h"
+#include "clang/Basic/Version.h"
 #include "clang/Config/config.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
@@ -90,6 +91,28 @@
   CmdArgs.push_back(Output.getFilename());
 
   C.addCommand(llvm::make_unique(JA, *this, Linker, CmdArgs, Inputs));
+
+  // When optimizing, if WASM_OPT is set, run wasm-opt.
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (const char *WasmOptPath = getenv("WASM_OPT")) {
+  StringRef OOpt = "s";
+  if (A->getOption().matches(options::OPT_O4) ||
+  A->getOption().matches(options::OPT_Ofast))
+OOpt = "4";
+  else 

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Dan Gohman via Phabricator via cfe-commits]

sunfish marked an inline comment as done.
sunfish added inline comments.



Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:137
+  getFilePaths().push_back(getDriver().SysRoot + "/lib/" + MultiarchTriple 
+
+   "/llvm-lto");
+}

sbc100 wrote:
> Is there any kind of president here?   i.e. do any other platforms have 
> support for this kind thing?  Seems like good idea I'd just like to be sure 
> we follow existing practices.
I looked around, but didn't see anything similar.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Dan Gohman via Phabricator via cfe-commits]

sunfish marked an inline comment as done.
sunfish added inline comments.



Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:96
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (const char *WasmOptPath = getenv("WASM_OPT")) {
+  StringRef OOpt = "s";

dschuff wrote:
> What would you think about adding a way to pass arguments through to wasm-opt 
> on the command line, like we have for the linker, assembler, etc? Something 
> like `-Wo,-O2` (or `-Ww` or whatever; analogous to `-Wl` and `-Wa`). That 
> seems nicer than an env var, although it doesn't solve the problem of 
> controlling whether to run the optimizer in the first place.
My guess here is that we don't need clang to have an option for passing 
additional flags -- if people want to do extra special things with wasm-opt on 
clang's output they can just run wasm-opt directly themselves. Does that sound 
reasonable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Dan Gohman via Phabricator via cfe-commits]

sunfish created this revision.
sunfish added reviewers: sbc100, dschuff, aheejin.
Herald added subscribers: dexonsmith, steven_wu, hiraditya, jgravelle-google, 
inglorion, aprantl, mehdi_amini.
Herald added a project: clang.

When the WASM_OPT environment variable is set, run the wasm-opt tool to 
optimize LLVM's output. Note that using wasm-opt currently means invalidating 
all debug info, however we can just tell users this when we tell them about 
WASM_OPT. This fixes PR43796.

Also, add an "llvm-lto" directory to the sysroot library search paths, so that 
sysroots can optionally provide LTO-enabled system libraries.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70500

Files:
  clang/lib/Driver/ToolChains/WebAssembly.cpp
  clang/test/Driver/wasm-toolchain-lto.c


Index: clang/test/Driver/wasm-toolchain-lto.c
===
--- /dev/null
+++ clang/test/Driver/wasm-toolchain-lto.c
@@ -0,0 +1,6 @@
+// A basic C link command-line with optimization with known OS and LTO enabled.
+
+// RUN: %clang -### -O2 -flto -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_OPT_KNOWN %s
+// LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi/llvm-lto" 
"-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" "-lc" 
"{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -90,6 +90,28 @@
   CmdArgs.push_back(Output.getFilename());
 
   C.addCommand(llvm::make_unique(JA, *this, Linker, CmdArgs, Inputs));
+
+  // When optimizing, if WASM_OPT is set, run wasm-opt.
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (const char *WasmOptPath = getenv("WASM_OPT")) {
+  StringRef OOpt = "s";
+  if (A->getOption().matches(options::OPT_O4) ||
+  A->getOption().matches(options::OPT_Ofast))
+OOpt = "4";
+  else if (A->getOption().matches(options::OPT_O0))
+OOpt = "0";
+  else if (A->getOption().matches(options::OPT_O))
+OOpt = A->getValue();
+
+  const char *WasmOpt = Args.MakeArgString(WasmOptPath);
+  ArgStringList CmdArgs;
+  CmdArgs.push_back(Output.getFilename());
+  CmdArgs.push_back(Args.MakeArgString(llvm::Twine("-O") + OOpt));
+  CmdArgs.push_back("-o");
+  CmdArgs.push_back(Output.getFilename());
+  C.addCommand(llvm::make_unique(JA, *this, WasmOpt, CmdArgs, 
Inputs));
+}
+  }
 }
 
 WebAssembly::WebAssembly(const Driver , const llvm::Triple ,
@@ -109,6 +131,11 @@
   } else {
 const std::string MultiarchTriple =
 getMultiarchTriple(getDriver(), Triple, getDriver().SysRoot);
+if (D.isUsingLTO()) {
+  // For LTO, enable use of lto-enabled sysroot libraries too, if 
available.
+  getFilePaths().push_back(getDriver().SysRoot + "/lib/" + MultiarchTriple 
+
+   "/llvm-lto");
+}
 getFilePaths().push_back(getDriver().SysRoot + "/lib/" + MultiarchTriple);
   }
 }


Index: clang/test/Driver/wasm-toolchain-lto.c
===
--- /dev/null
+++ clang/test/Driver/wasm-toolchain-lto.c
@@ -0,0 +1,6 @@
+// A basic C link command-line with optimization with known OS and LTO enabled.
+
+// RUN: %clang -### -O2 -flto -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_OPT_KNOWN %s
+// LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi/llvm-lto" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -90,6 +90,28 @@
   CmdArgs.push_back(Output.getFilename());
 
   C.addCommand(llvm::make_unique(JA, *this, Linker, CmdArgs, Inputs));
+
+  // When optimizing, if WASM_OPT is set, run wasm-opt.
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (const char *WasmOptPath = getenv("WASM_OPT")) {
+  StringRef OOpt = "s";
+  if (A->getOption().matches(options::OPT_O4) ||
+  A->getOption().matches(options::OPT_Ofast))
+OOpt = "4";
+  else if (A->getOption().matches(options::OPT_O0))
+OOpt = "0";
+  else if (A->getOption().matches(options::OPT_O))
+OOpt = A->getValue();
+
+  const char *WasmOpt = Args.MakeArgString(WasmOptPath);
+  ArgStringList CmdArgs;
+  CmdArgs.push_back(Output.getFilename());
+  CmdArgs.push_back(Args.MakeArgString(llvm::Twine("-O") + OOpt));
+  

[PATCH] D67739: [WebAssembly] Let users know that wasm64 does not exist

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

Would it be better to do this check in LLVM, in the backend, with a 
`report_fatal_error`?

Clang for its part already supports what we expect wasm64 will need already, 
and we have a bunch of tests for it, so it'd be nice to keep that code around 
and tested. And, doing the check in the LLVM backend also means we'll catch 
users trying to use it from languages other than C/C++.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67739



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66983: [WebAssembly] Add wasm-specific vector shuffle builtin and intrinsic

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

In D66983#1651981 , @craig.topper 
wrote:

> DAG combine is supposed to check with TargetLowering::isShuffleMaskLegal.


In @tlively's example, it is DAGCombine, and it does check isShuffleMaskLegal. 
However for wasm, it appears that's not enough -- in wasm, all shuffle masks 
are legal, because you can do any shuffle in a single wasm instruction. This 
makes it tricky, because the user here is aiming for an x86-like cost model, 
but the LLVM wasm backend doesn't have any x86-specific knowledge, so it just 
tells DAGCombine to form any shuffle it sees fit.

I wonder if it would make sense to introduce a counterpart to 
isShuffleMaskLegal, which instead of returning a bool returned a cost value. 
And then, we could teach the wasm backend about certain shuffle patterns which 
are known to be fast across multiple architectures. Then we could teach 
DAGComine to check whether the new shuffle it wants to create is actually 
cheaper than the one it's replacing. Thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66983



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66983: [WebAssembly] Add wasm-specific vector shuffle builtin and intrinsic

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

In D66983#1651977 , @dschuff wrote:

> Oh, interesting I didn't notice that those are implementations of the 
> target-specific intrinsics. I wonder if they do that so they can implement 
> the intrinsics on hardware that doesn't support the corresponding 
> instruction? In a situation other than that I think I'd be surprised if I 
> used a builtin for a particular instruction and got something else.


One of the reasons is a belief that LLVM's optimizations are desirable, and 
that if there are cases where LLVM's optimizations make code worse, it's a bug 
in LLVM which should be fixed. Do you have any such cases?

I should say, I myself am sympathetic to the argument that if you write 
`_mm_shuffle_ps`, you might really want `SHUFPS`, and not 
`__builtin_shufflevector`, but I'm not aware of any target in LLVM that works 
this way, which is something to consider.

In fact, the argument that `__mm_shuffle_ps` is SHUFPS seems like it ought to 
be stronger for x86 than wasm, because x86 has about 100 different shuffle 
instructions each with their own opinion, while wasm just has one shuffle 
instruction that just does everything.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66983



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66983: [WebAssembly] Add wasm-specific vector shuffle builtin and intrinsic

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

Can you say what leads wasm users to maintain such an expectation when, for 
example, ARM users 

 and x86 users 

 don't?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66983



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66035: [WebAssembly] WIP: Add support for reference types

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

x86 uses address spaces starting at 256 and counting up for its 
architecture-specific address spaces. The docs 
 say 
"Address spaces 1-255 are currently reserved for user-defined code." so we 
should consider using 256 here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66035



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61452: [WebAssembly] Always include /lib in library path

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

> This allows for us to fall back from arch-specific to generic headers as 
> needed.  The same can be true of libraries.  Not all libraries contains 
> compiled code.  `.so` files can also be linker scripts that reference other 
> libraries in which case they can be arch-neutral.

If we add support for implicit linker scripts in wasm-ld, we can look into 
this. That said, it's not clear we'll want to. They're rare, and when they are 
used they're another moving part in the middle of a complex system. There are 
usually other ways to do what they do.

> Secondly, not everyone is going to want to use a multi-arch sysroot.  For 
> example I would argue that the wasi SDK would make more sense being single 
> arch since it has exactly one purpose.  Its only when installing wasi into an 
> existing system that one really needs to be multi-arch.  In any case we 
> should not dictate this.

We're anticipating wasm64. And it's possible WASI could have other triple 
variants in the future too.

I'm aware some users will sometimes know that they only care about wasm32-wasi. 
Some of those users may write code, and perhaps some of those that do will 
share it with others. If such code assumes it can install into $SYSROOT/lib, it 
may break things for others using a multiarch setup. Unless there are reasons 
otherwise, it seems valuable to minimize fragmentation between multiarch and 
non-multiarch users. That seems more valuable than enabling some users to have 
shorter library paths.

> We've already have one person trying to build a wasi-sdk without using 
> multi-arch paths.

They ended up deciding they misunderstood the project for other reasons and 
left, so it's unclear what conclusion to draw.

> I'm not sure what you mean by "single arch sysroots are possible either way". 
>  The point of a single arch sysroot would be avoid the unnecessary extra path 
> components and I thats exactly what this change is allowing for.

I just mean you can have a multi-arch-style directory tree with a single arch 
installed in it.

> Thirdly, its what the linux driver does, which is the example we are 
> following of how to build a multi-arch toolchain.

The Linux driver has to stay compatible with existing pre-multi-arch systems; 
we don't.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61452



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64900: [WebAssembly] Implement __builtin_wasm_tls_base intrinsic

[Dan Gohman via Phabricator via cfe-commits]

sunfish added inline comments.



Comment at: cfe/trunk/include/clang/Basic/BuiltinsWebAssembly.def:34
 TARGET_BUILTIN(__builtin_wasm_tls_size, "z", "nc", "bulk-memory")
+TARGET_BUILTIN(__builtin_wasm_tls_base, "v*", "n", "bulk-memory")
 

This can also be "p" for "pure", which despite the name, just means there are 
no mutating side effects.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64900



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64537: [WebAssembly] Implement thread-local storage for non-PIC cases

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

This looks nice!

> __wasm_init_tls(calloc(__builtin_wasm_tls_size(), 1));

Would it make sense to change the API contract for `__wasm_init_tls` to 
guarantee that initializes all bytes (with zeros as needed)? `__wasm_init_tls` 
knows what bytes it's initializing, so we could use plain `malloc` instead of 
`calloc` and avoid redundant zero initialization of those bytes.




Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:
+
+  return DAG.getNode(ISD::ADD, DL, VT, TLSBase, TLSOffset);
+}

It looks like this supports local-exec, but would need to be extended to handle 
initial-exec or the other TLS models. Assuming this is correct, could you add a 
check for the TLS model and `report_fatal_error` on unsupported models?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64537



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63030: [WebAssembly] Modernize include path handling

[Dan Gohman via Phabricator via cfe-commits]

sunfish accepted this revision.
sunfish added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63030



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62406: [WebAssembly] Use "linker" as linker shortname.

[Dan Gohman via Phabricator via cfe-commits]

sunfish accepted this revision.
sunfish added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62406



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61452: [WebAssembly] Always include /lib in library path

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

If "$sysroot/lib" ends up coming to mean "wasm32" because people come to depend 
on that, then wasm64 may end up needing to be different in a gratuitous way, 
which I'd like to avoid.

I'd like to keep our sysroots tidy when we can. If some libraries are installed 
in `lib/wasm32-wasi` and others `lib` for no reason other than build script 
inertia, that's untidy.

It's not an absolute for me, but I am inclined to see if we can understand the 
need better first. Single-arch sysroots are possible either way, for example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61452



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61452: [WebAssembly] Always include /lib in library path

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

The value of supporting single-arch sysroots is unclear to me. It's always 
possible to have a sysroot with libraries for just one architecture installed, 
even with multi-arch paths. Is this just about compatibility with build scripts 
and tools which are hard-coded to "$sysroot/lib"?

We'll eventually want the ability to have sysroots that support both wasm32 and 
wasm64. They can share headers, but they'll need their own library directories. 
If we have libraries installed in "$sysroot/lib", it'll complicate that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61452



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61452: [WebAssembly] Always include /lib in library path

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

If libraries don't correctly install into multi-arch directories, can we fix 
the libraries? We do this in the wasi-sdk repo to fix up the libc++ and 
libc++abi libraries, for example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61452



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61389: Bump DIAG_SIZE_SEMA up to 4000

[Dan Gohman via Phabricator via cfe-commits]

sunfish abandoned this revision.
sunfish added a comment.

Cool :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D61389



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61389: Bump DIAG_SIZE_SEMA up to 4000

[Dan Gohman via Phabricator via cfe-commits]

sunfish created this revision.
sunfish added a reviewer: aaron.ballman.
Herald added a subscriber: aheejin.
Herald added a project: clang.

https://reviews.llvm.org/D59520 adds a few new diagnostics, which happens to 
run into a limit with DIAG_SIZE_SEMA. As suggested here 
, here's a separate patch to 
bump the limit from 3500 to 4000.


Repository:
  rC Clang

https://reviews.llvm.org/D61389

Files:
  include/clang/Basic/DiagnosticIDs.h


Index: include/clang/Basic/DiagnosticIDs.h
===
--- include/clang/Basic/DiagnosticIDs.h
+++ include/clang/Basic/DiagnosticIDs.h
@@ -36,7 +36,7 @@
   DIAG_SIZE_AST   =  150,
   DIAG_SIZE_COMMENT   =  100,
   DIAG_SIZE_CROSSTU   =  100,
-  DIAG_SIZE_SEMA  = 3500,
+  DIAG_SIZE_SEMA  = 4000,
   DIAG_SIZE_ANALYSIS  =  100,
   DIAG_SIZE_REFACTORING   = 1000,
 };


Index: include/clang/Basic/DiagnosticIDs.h
===
--- include/clang/Basic/DiagnosticIDs.h
+++ include/clang/Basic/DiagnosticIDs.h
@@ -36,7 +36,7 @@
   DIAG_SIZE_AST   =  150,
   DIAG_SIZE_COMMENT   =  100,
   DIAG_SIZE_CROSSTU   =  100,
-  DIAG_SIZE_SEMA  = 3500,
+  DIAG_SIZE_SEMA  = 4000,
   DIAG_SIZE_ANALYSIS  =  100,
   DIAG_SIZE_REFACTORING   = 1000,
 };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61338: [WebAssembly] Use the "wasm32-wasi" triple in tests

[Dan Gohman via Phabricator via cfe-commits]

sunfish created this revision.
sunfish added reviewers: dschuff, sbc100, aheejin.
Herald added a subscriber: jgravelle-google.
Herald added a project: clang.

Similar to https://reviews.llvm.org/D61334, update clang tests to use the 
"wasm32-wasi" triple, removing the "-musl" environment and omitting the 
"-unknown" vendor.


Repository:
  rC Clang

https://reviews.llvm.org/D61338

Files:
  test/Driver/wasm-toolchain.c
  test/Driver/wasm-toolchain.cpp
  test/Preprocessor/init.c


Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -9577,10 +9577,10 @@
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=wasm64-unknown-unknown \
 // RUN:   < /dev/null \
 // RUN:   | FileCheck -match-full-lines 
-check-prefixes=WEBASSEMBLY,WEBASSEMBLY64 %s
-// RUN: %clang_cc1 -E -dM -ffreestanding -triple=wasm32-unknown-wasi \
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=wasm32-wasi \
 // RUN:   < /dev/null \
 // RUN:   | FileCheck -match-full-lines 
-check-prefixes=WEBASSEMBLY,WEBASSEMBLY32,WEBASSEMBLY-WASI %s
-// RUN: %clang_cc1 -E -dM -ffreestanding -triple=wasm64-unknown-wasi \
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=wasm64-wasi \
 // RUN:   < /dev/null \
 // RUN:   | FileCheck -match-full-lines 
-check-prefixes=WEBASSEMBLY,WEBASSEMBLY64,WEBASSEMBLY-WASI %s
 //
Index: test/Driver/wasm-toolchain.cpp
===
--- test/Driver/wasm-toolchain.cpp
+++ test/Driver/wasm-toolchain.cpp
@@ -24,17 +24,17 @@
 
 // A basic C++ link command-line with known OS.
 
-// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-unknown-wasi-musl 
--sysroot=/foo --stdlib=c++ %s 2>&1 | FileCheck -check-prefix=LINK_KNOWN %s
+// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo --stdlib=c++ %s 2>&1 | FileCheck -check-prefix=LINK_KNOWN %s
 // LINK_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
-// LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi-musl" "crt1.o" 
"[[temp]]" "-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" 
"-o" "a.out"
+// LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" 
"-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C++ link command-line with optimization with known OS.
 
-// RUN: %clangxx -### -O2 -no-canonical-prefixes -target 
wasm32-unknown-wasi-musl --sysroot=/foo %s --stdlib=c++ 2>&1 | FileCheck 
-check-prefix=LINK_OPT_KNOWN %s
+// RUN: %clangxx -### -O2 -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo %s --stdlib=c++ 2>&1 | FileCheck -check-prefix=LINK_OPT_KNOWN %s
 // LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
-// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi-musl" "crt1.o" 
"[[temp]]" "-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" 
"-o" "a.out"
+// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" 
"-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C++ compile command-line with known OS.
 
-// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-unknown-wasi-musl 
--sysroot=/foo --stdlib=c++ %s 2>&1 | FileCheck -check-prefix=COMPILE %s
-// COMPILE: clang{{.*}}" "-cc1" {{.*}} "-internal-isystem" 
"/foo/include/wasm32-wasi-musl/c++/v1" "-internal-isystem" 
"/foo/include/c++/v1" "-internal-isystem" "/foo/include/wasm32-wasi-musl" 
"-internal-isystem" "/foo/include"
+// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo --stdlib=c++ %s 2>&1 | FileCheck -check-prefix=COMPILE %s
+// COMPILE: clang{{.*}}" "-cc1" {{.*}} "-internal-isystem" 
"/foo/include/wasm32-wasi/c++/v1" "-internal-isystem" "/foo/include/c++/v1" 
"-internal-isystem" "/foo/include/wasm32-wasi" "-internal-isystem" 
"/foo/include"
Index: test/Driver/wasm-toolchain.c
===
--- test/Driver/wasm-toolchain.c
+++ test/Driver/wasm-toolchain.c
@@ -24,20 +24,20 @@
 
 // A basic C link command-line with known OS.
 
-// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-wasi-musl 
--sysroot=/foo %s 2>&1 | FileCheck -check-prefix=LINK_KNOWN %s
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo 
%s 2>&1 | FileCheck -check-prefix=LINK_KNOWN %s
 // LINK_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
-// LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi-musl" "crt1.o" 
"[[temp]]" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
+// LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" 
"-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C link command-line with optimization with known OS.
 
-// RUN: %clang -### -O2 -no-canonical-prefixes -target 
wasm32-unknown-wasi-musl --sysroot=/foo %s 2>&1 | FileCheck 
-check-prefix=LINK_OPT_KNOWN %s
+// RUN: 

[PATCH] D59520: [WebAssembly] Address review comments on r352930

[Dan Gohman via Phabricator via cfe-commits]

sunfish updated this revision to Diff 197215.
sunfish added a comment.

Implemented proper diagnostics for import_name/import_module on functions with 
definitions, and updated the test.

I'm unsure of the `DIAG_SIZE_SEMA` change, but without it, the build fails with 
this error:

  /llvm/tools/clang/lib/Basic/DiagnosticIDs.cpp:72:3: error: static assertion 
failed: DIAG_SIZE_SEMA is insufficient to contain all diagnostics, it may need 
to be made larger in DiagnosticIDs.h.
 static_assert( 
  \
 ^
  /llvm/tools/clang/lib/Basic/DiagnosticIDs.cpp:88:1: note: in expansion of 
macro ‘VALIDATE_DIAG_SIZE’
   VALIDATE_DIAG_SIZE(SEMA)
   ^~


Repository:
  rC Clang

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

https://reviews.llvm.org/D59520

Files:
  include/clang/Basic/DiagnosticIDs.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/Sema/attr-wasm.c

Index: test/Sema/attr-wasm.c
===
--- test/Sema/attr-wasm.c
+++ test/Sema/attr-wasm.c
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple wasm32-unknown-unknown -fsyntax-only -verify %s
+
+void name_a() __attribute__((import_name)); //expected-error {{'import_name' attribute takes one argument}}
+
+int name_b __attribute__((import_name("foo"))); //expected-error {{'import_name' attribute only applies to functions}}
+
+void name_c() __attribute__((import_name("foo", "bar"))); //expected-error {{'import_name' attribute takes one argument}}
+
+void name_d() __attribute__((import_name("foo", "bar", "qux"))); //expected-error {{'import_name' attribute takes one argument}}
+
+void name_z() __attribute__((import_name("foo")));
+
+void module_a() __attribute__((import_module)); //expected-error {{'import_module' attribute takes one argument}}
+
+int module_b __attribute__((import_module("foo"))); //expected-error {{'import_module' attribute only applies to functions}}
+
+void module_c() __attribute__((import_module("foo", "bar"))); //expected-error {{'import_module' attribute takes one argument}}
+
+void module_d() __attribute__((import_module("foo", "bar", "qux"))); //expected-error {{'import_module' attribute takes one argument}}
+
+void module_z() __attribute__((import_module("foo")));
+
+void both() __attribute__((import_name("foo"), import_module("bar")));
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -5755,50 +5755,80 @@
   handleSimpleAttribute(S, D, AL);
 }
 
-static void handleWebAssemblyImportModuleAttr(Sema , Decl *D, const ParsedAttr ) {
-  if (!isFunctionOrMethod(D)) {
-S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)
-<< "'import_module'" << ExpectedFunction;
-return;
+WebAssemblyImportModuleAttr *
+Sema::mergeImportModuleAttr(Decl *D, SourceRange Range,
+StringRef Name,
+unsigned AttrSpellingListIndex) {
+  auto *FD = cast(D);
+
+  if (WebAssemblyImportModuleAttr *ExistingAttr =
+  FD->getAttr()) {
+if (ExistingAttr->getImportModule() == Name)
+  return nullptr;
+Diag(ExistingAttr->getLocation(), diag::warn_mismatched_import_module);
+Diag(Range.getBegin(), diag::note_previous_attribute);
+return nullptr;
+  }
+  if (FD->hasBody()) {
+Diag(Range.getBegin(), diag::warn_import_module_on_definition);
+return nullptr;
   }
+  return ::new (Context) WebAssemblyImportModuleAttr(Range, Context, Name,
+ AttrSpellingListIndex);
+}
 
+WebAssemblyImportNameAttr *
+Sema::mergeImportNameAttr(Decl *D, SourceRange Range,
+  StringRef Name,
+  unsigned AttrSpellingListIndex) {
   auto *FD = cast(D);
-  if (FD->isThisDeclarationADefinition()) {
-S.Diag(D->getLocation(), diag::err_alias_is_definition) << FD << 0;
-return;
+
+  if (WebAssemblyImportNameAttr *ExistingAttr =
+  FD->getAttr()) {
+if (ExistingAttr->getImportName() == Name)
+  return nullptr;
+Diag(ExistingAttr->getLocation(), diag::warn_mismatched_import_name);
+Diag(Range.getBegin(), diag::note_previous_attribute);
+return nullptr;
+  }
+  if (FD->hasBody()) {
+Diag(Range.getBegin(), diag::warn_import_name_on_definition);
+return nullptr;
   }
+  return ::new (Context) WebAssemblyImportNameAttr(Range, Context, Name,
+   AttrSpellingListIndex);
+}
+
+static void
+handleWebAssemblyImportModuleAttr(Sema , Decl *D, const ParsedAttr ) {
+  auto *FD = cast(D);
 
   StringRef Str;
   SourceLocation ArgLoc;
   if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, ))
 return;
 
-  FD->addAttr(::new (S.Context) WebAssemblyImportModuleAttr(
-  

[PATCH] D59520: [WebAssembly] Address review comments on r352930

[Dan Gohman via Phabricator via cfe-commits]

sunfish marked an inline comment as done.
sunfish added inline comments.



Comment at: test/Sema/attr-wasm.c:3
+
+void name_a() {}
+

aaron.ballman wrote:
> Was this intended to be used somewhere? Probably can just be removed if not.
No, you're right that we don't need this (it's copypasta from another test). 
I'll remove it from the patch.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59520



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59520: [WebAssembly] Address review comments on r352930

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

In D59520#1434854 , @aaron.ballman 
wrote:

> > Removes errnoneous use of diag::err_alias_is_definition, which turned out 
> > to be ineffective anyway since functions can be defined later in the 
> > translation unit and avoid detection.
>
> If you need to keep the prohibition that these attributes not be applied to 
> functions with definitions, there are ways to accomplish that (we merge 
> declarations and can decide what to do at that point if we see a declaration 
> that is a definition). Given that, do you still want to lift this restriction?


These attributes don't make sense on definitions. So right now, they are 
silently ignored when applied to definitions. That's effectively the current 
behavior too, because the existing check doesn't work as intended, so the 
change here doesn't change anything in practice yet.

A diagnostic would be slightly tidier, but I'm not familiar enough with clang 
to do this quickly and didn't want to delay the rest of the patch while I 
investigated. Do you know the specific places we'd need to change to diagnose 
this?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59520



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59520: [WebAssembly] Address review comments on r352930

[Dan Gohman via Phabricator via cfe-commits]

sunfish created this revision.
sunfish added a reviewer: aaron.ballman.
Herald added subscribers: aheejin, jgravelle-google, sbc100, dschuff.
Herald added a project: clang.

This patch addresses the review comments on r352930:

- Removes redundant diagnostic checking code
- Removes errnoneous use of `diag::err_alias_is_definition`, which turned out 
to be ineffective anyway since functions can be defined later in the 
translation unit and avoid detection.
- Adds a test for various invalid cases for `import_name` and `import_module`.


Repository:
  rC Clang

https://reviews.llvm.org/D59520

Files:
  lib/Sema/SemaDeclAttr.cpp
  test/Sema/attr-wasm.c


Index: test/Sema/attr-wasm.c
===
--- test/Sema/attr-wasm.c
+++ test/Sema/attr-wasm.c
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -triple wasm32-unknown-unknown -fsyntax-only -verify %s
+
+void name_a() {}
+
+void name_b() __attribute__((import_name)); //expected-error {{'import_name' 
attribute takes one argument}}
+
+int name_c __attribute__((import_name("foo"))); //expected-error 
{{'import_name' attribute only applies to functions}}
+
+void name_d() __attribute__((import_name("foo", "bar"))); //expected-error 
{{'import_name' attribute takes one argument}}
+
+void name_e() __attribute__((import_name("foo", "bar", "qux"))); 
//expected-error {{'import_name' attribute takes one argument}}
+
+void name_z() __attribute__((import_name("foo")));
+
+void module_a() {}
+
+void module_b() __attribute__((import_module)); //expected-error 
{{'import_module' attribute takes one argument}}
+
+int module_c __attribute__((import_module("foo"))); //expected-error 
{{'import_module' attribute only applies to functions}}
+
+void module_d() __attribute__((import_module("foo", "bar"))); //expected-error 
{{'import_module' attribute takes one argument}}
+
+void module_e() __attribute__((import_module("foo", "bar", "qux"))); 
//expected-error {{'import_module' attribute takes one argument}}
+
+void module_z() __attribute__((import_module("foo")));
+
+void both() __attribute__((import_name("foo"), import_module("bar")));
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -5754,17 +5754,7 @@
 }
 
 static void handleWebAssemblyImportModuleAttr(Sema , Decl *D, const 
ParsedAttr ) {
-  if (!isFunctionOrMethod(D)) {
-S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)
-<< "'import_module'" << ExpectedFunction;
-return;
-  }
-
   auto *FD = cast(D);
-  if (FD->isThisDeclarationADefinition()) {
-S.Diag(D->getLocation(), diag::err_alias_is_definition) << FD << 0;
-return;
-  }
 
   StringRef Str;
   SourceLocation ArgLoc;
@@ -5777,17 +5767,7 @@
 }
 
 static void handleWebAssemblyImportNameAttr(Sema , Decl *D, const ParsedAttr 
) {
-  if (!isFunctionOrMethod(D)) {
-S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)
-<< "'import_name'" << ExpectedFunction;
-return;
-  }
-
   auto *FD = cast(D);
-  if (FD->isThisDeclarationADefinition()) {
-S.Diag(D->getLocation(), diag::err_alias_is_definition) << FD << 0;
-return;
-  }
 
   StringRef Str;
   SourceLocation ArgLoc;


Index: test/Sema/attr-wasm.c
===
--- test/Sema/attr-wasm.c
+++ test/Sema/attr-wasm.c
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -triple wasm32-unknown-unknown -fsyntax-only -verify %s
+
+void name_a() {}
+
+void name_b() __attribute__((import_name)); //expected-error {{'import_name' attribute takes one argument}}
+
+int name_c __attribute__((import_name("foo"))); //expected-error {{'import_name' attribute only applies to functions}}
+
+void name_d() __attribute__((import_name("foo", "bar"))); //expected-error {{'import_name' attribute takes one argument}}
+
+void name_e() __attribute__((import_name("foo", "bar", "qux"))); //expected-error {{'import_name' attribute takes one argument}}
+
+void name_z() __attribute__((import_name("foo")));
+
+void module_a() {}
+
+void module_b() __attribute__((import_module)); //expected-error {{'import_module' attribute takes one argument}}
+
+int module_c __attribute__((import_module("foo"))); //expected-error {{'import_module' attribute only applies to functions}}
+
+void module_d() __attribute__((import_module("foo", "bar"))); //expected-error {{'import_module' attribute takes one argument}}
+
+void module_e() __attribute__((import_module("foo", "bar", "qux"))); //expected-error {{'import_module' attribute takes one argument}}
+
+void module_z() __attribute__((import_module("foo")));
+
+void both() __attribute__((import_name("foo"), import_module("bar")));
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -5754,17 +5754,7 @@
 }
 
 static void handleWebAssemblyImportModuleAttr(Sema , 

[PATCH] D58742: [WebAssembly] Remove uses of ThreadModel

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

Wasm gives users reasons to want -mthread-model single that other architectures 
don't, even when -matomics is enabled by default.

When shared memory is used, wasm requires modules to declare a max memory size, 
which is a burden on applications that want to use dynamic amounts of memory. 
Wasm is more sensitive to code size than most other architectures. And, 
interoperating with single-threaded JS is easier from single-threaded wasm (JS 
has access to SharedArrayBuffer and atomics, but not all JS wants to use that).


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D58742



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58742: [WebAssembly] Remove uses of ThreadModel

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

This is still a little confusing to me. -matomic is supposed to be a subtarget 
flag, stating that the wasm implementation we will run on supports atomic 
instructions. -mthread-model posix is about the C++ interpretation -- what 
style implementation of memory model do we want? In the future, -matomic may 
become enabled by default, when enough wasm engines generally support it. 
However, -mthread-model single/posix may still be useful to control 
independently, because even with wasm engines supporting atomic, there are 
reasons users might still want to compile their apps single-threaded: access to 
linear memory with no declared max, lower overall code size, or other things.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58742



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57874: [WebAssembly] Make thread-related options consistent

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

> - `-matomics` means `-mthread-model posix`

The others sound reasonable, though this one seems a little surprising -- a 
user might have -matomics enabled because they're targeting a VM that has 
atomics, but still not want to use -mthread-model posix because their code 
doesn't actually using threads.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57874: [WebAssembly] Set '-matomics' when '-pthread' is set

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

That sounds reasonable to me too.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57577: Make predefined FLT16 macros conditional on support for the type

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

I think WebAssembly is in the same situation as most other architectures, as 
discussed here:

http://llvm.org/viewvc/llvm-project?view=revision=352221

and should not enable _Float16 yet. The test/Preprocessor/init.c tests were 
semi-automatically generated, so it wasn't specifically intended to include 
tests for FLT16 macros for WebAssembly.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57577



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57160: [WebAssembly] Add an import_module function attribute

[Dan Gohman via Phabricator via cfe-commits]

sunfish created this revision.
sunfish added reviewers: sbc100, dschuff, aheejin.
Herald added subscribers: llvm-commits, jgravelle-google.

This adds a C/C++ attribute which corresponds to the LLVM IR wasm-import-module 
attribute. It allows code to specify an explicit import module.


Repository:
  rL LLVM

https://reviews.llvm.org/D57160

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  lib/CodeGen/TargetInfo.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGen/wasm-import-module.c

Index: test/CodeGen/wasm-import-module.c
===
--- test/CodeGen/wasm-import-module.c
+++ test/CodeGen/wasm-import-module.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple wasm32-unknown-unknown-wasm -emit-llvm -o - %s | FileCheck %s
+
+void __attribute__((import_module("bar"))) foo(void);
+
+void call(void) {
+  foo();
+}
+
+// CHECK: declare void @foo() [[A:#[0-9]+]]
+
+// CHECK: attributes [[A]] = {{{.*}} "wasm-import-module"="bar" {{.*}}}
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -5734,7 +5734,29 @@
   handleSimpleAttribute(S, D, AL);
 }
 
+static void handleWebAssemblyImportModuleAttr(Sema , Decl *D, const ParsedAttr ) {
+  if (!isFunctionOrMethod(D)) {
+S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)
+<< "'import_module'" << ExpectedFunction;
+return;
+  }
 
+  auto *FD = cast(D);
+  if (FD->isThisDeclarationADefinition()) {
+S.Diag(D->getLocation(), diag::err_alias_is_definition) << FD << 0;
+return;
+  }
+
+  StringRef Str;
+  SourceLocation ArgLoc;
+  if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, ))
+return;
+
+  FD->addAttr(::new (S.Context) WebAssemblyImportModuleAttr(
+  AL.getRange(), S.Context, Str,
+  AL.getAttributeSpellingListIndex()));
+}
+
 static void handleRISCVInterruptAttr(Sema , Decl *D,
  const ParsedAttr ) {
   // Warn about repeated attributes.
@@ -6487,6 +6509,9 @@
   case ParsedAttr::AT_AVRSignal:
 handleAVRSignalAttr(S, D, AL);
 break;
+  case ParsedAttr::AT_WebAssemblyImportModule:
+handleWebAssemblyImportModuleAttr(S, D, AL);
+break;
   case ParsedAttr::AT_IBAction:
 handleSimpleAttribute(S, D, AL);
 break;
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -760,6 +760,16 @@
 
   void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
CodeGen::CodeGenModule ) const override {
+TargetCodeGenInfo::setTargetAttributes(D, GV, CGM);
+if (const auto *FD = dyn_cast_or_null(D)) {
+  if (const auto *Attr = FD->getAttr()) {
+llvm::Function *Fn = cast(GV);
+llvm::AttrBuilder B;
+B.addAttribute("wasm-import-module", Attr->getImportModuleName());
+Fn->addAttributes(llvm::AttributeList::FunctionIndex, B);
+  }
+}
+
 if (auto *FD = dyn_cast_or_null(D)) {
   llvm::Function *Fn = cast(GV);
   if (!FD->doesThisDeclarationHaveABody() && !FD->hasPrototype())
Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -3651,7 +3651,23 @@
 For more information see
 `gcc documentation `_
 or `msvc documentation `_.
-}];
+}]; }
+
+def WebAssemblyImportModuleDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+Clang supports the ``__attribute__((import_module()))`` 
+attribute for the WebAssembly target. This attribute may be attached to a
+function declaration, where it modifies how the symbol is to be imported
+within the WebAssembly linking environment.
+
+WebAssembly imports use a two-level namespace scheme, consisting of a module
+name, which typically identifies a module from which to import, and a field
+name, which typically identifies a field from that module to import. By
+default, module names for C/C++ symbols are assigned automatically by the
+linker. This attribute can be used to override the default behavior, and
+reuqest a specific module name be used instead.
+  }];
 }
 
 def ArtificialDocs : Documentation {
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -331,6 +331,7 @@
 def TargetRISCV : TargetArch<["riscv32", "riscv64"]>;
 def TargetX86 : TargetArch<["x86"]>;
 def TargetAnyX86 : TargetArch<["x86", "x86_64"]>;
+def TargetWebAssembly : TargetArch<["wasm32", "wasm64"]>;
 def TargetWindows : TargetArch<["x86", "x86_64", "arm", "thumb", "aarch64"]> {
   let OSes = ["Win32"];
 }
@@