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

2023-03-29 Thread Sam Clegg via Phabricator via cfe-commits
sbc100 abandoned this revision. sbc100 added a comment. In D76547#4231604 , @aaron.ballman wrote: > In D76547#4231501 , @sbc100 wrote: > >> In D76547#4231476 , @aaron.ballma

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

2023-03-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D76547#4231501 , @sbc100 wrote: > In D76547#4231476 , @aaron.ballman > wrote: > >> In D76547#4231422 , @sbc100 wrote: >> >>> The reason `__

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

2023-03-29 Thread Sam Clegg via Phabricator via cfe-commits
sbc100 added a comment. In D76547#4231476 , @aaron.ballman wrote: > In D76547#4231422 , @sbc100 wrote: > >> The reason `__attribute__((export_name("foo")))` doesn't work in all use >> cases is that we have a lot o

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

2023-03-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D76547#4231422 , @sbc100 wrote: > The reason `__attribute__((export_name("foo")))` doesn't work in all use > cases is that we have a lot of existing code that uses the > `EMSCRIPTEN_KEEPALIVE` macro. We also have run

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

2023-03-29 Thread Sam Clegg via Phabricator via cfe-commits
sbc100 added a comment. The reason `__attribute__((export_name("foo")))` doesn't work in all use cases is that we have a lot of existing code that uses the `EMSCRIPTEN_KEEPALIVE` macro. We also have run into other folks who want to include this is some kind of `FOO_API`, or `EXPORT_API` typ

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

2023-03-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a reviewer: erichkeane. aaron.ballman added a comment. The changes need a release note at some point, and this is missing all of the usual sema diagnostic tests (wrong subject, wrong number of args, wrong target, etc). That said, are we sure this attribute is sufficiently co

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

2023-03-23 Thread Sam Clegg via Phabricator via cfe-commits
sbc100 added a comment. I just figured out that this cannot replace the current use of `__attribute__((used))` in emscripten because function attributes only work for functions and we need this mechanism to work for global data addresses too. There is simply no way to do something like `Fn-

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

2023-03-23 Thread Sam Clegg via Phabricator via cfe-commits
sbc100 updated this revision to Diff 507826. sbc100 edited the summary of this revision. sbc100 added a comment. - limit to emscripten Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76547/new/ https://reviews.llvm.org/D76547 Files: clang/include/

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

2023-03-23 Thread Sam Clegg via Phabricator via cfe-commits
sbc100 added a comment. I've limited to new attribute to only the emcripten triple. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76547/new/ https://reviews.llvm.org/D76547 ___ cfe-commits mailing list c

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

2023-03-23 Thread Sam Clegg via Phabricator via cfe-commits
sbc100 updated this revision to Diff 507807. sbc100 added a comment. - update test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76547/new/ https://reviews.llvm.org/D76547 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/Attr

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

2023-03-23 Thread Sam Clegg via Phabricator via cfe-commits
sbc100 added a comment. In D76547#1945094 , @sbc100 wrote: > What about your idea of using the `default` keyword rather than adding a new > clang attr? I quite liked that approach. IIRC I tried this approach but wan't able to make it works since a singl

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

2023-03-23 Thread Sam Clegg via Phabricator via cfe-commits
sbc100 updated this revision to Diff 507802. sbc100 added a comment. Herald added a reviewer: aaron.ballman. Herald added subscribers: llvm-commits, pmatos, asb, wingo, ecnelises. Herald added projects: LLVM, All. - rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

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

2020-03-26 Thread Sam Clegg via Phabricator via cfe-commits
sbc100 added a comment. I was thinking of dropping the new clang attr in favor of the `default` keyword in the current attr, but actually keeping the llvm attr, since it corresponds quite nicely to the existing EXPORTED symbol attribute in the binary format and avoid duplication in the `.ll`, `

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

2020-03-26 Thread Sam Clegg via Phabricator via cfe-commits
sbc100 added a comment. What about your idea of using the `default` keyword rather than adding a new clang attr? I quite liked that approach. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76547/new/ https://reviews.llvm.org/D76547

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

2020-03-26 Thread 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

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

2020-03-23 Thread Alon Zakai via Phabricator via cfe-commits
kripken accepted this revision. kripken added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Basic/AttrDocs.td:4173 +attribute for the WebAssembly target. This attribute may be attached to a +function declaration, where it causes

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

2020-03-21 Thread Sam Clegg via Phabricator via cfe-commits
sbc100 created this revision. Herald added subscribers: cfe-commits, sunfish, aheejin, hiraditya, jgravelle-google, dschuff. Herald added a project: clang. sbc100 added reviewers: sunfish, kripken. This matches the existing export-name attribute but exports that symbol but its llvm symbol name.