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

brendandahl updated this revision to Diff 531008. brendandahl added a comment. After some feedback, a few people have indicated they'd like to do something similar to this, but for their own attributes. I've changed to a more generic attribute that allows arbitrary strings. Now a custom section

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

aaron.ballman added a comment. Aside from the suggested changes, the Clang bits LGTM Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7636-7639 +static void handleWebAssemblyAsyncAttr(Sema , Decl *D, const ParsedAttr ) { + D->addAttr(::new (S.Context)

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

sbc100 added a comment. In D150803#4401552 , @sunfish wrote: > In D150803#4401204 , @sbc100 wrote: > >> In D150803#4401061 , @sunfish >> wrote: >> >>> wasm_jspi works

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

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 >

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

sbc100 added a comment. 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

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

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

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

brendandahl marked an inline comment as done. brendandahl added a comment. In D150803#4389062 , @sunfish wrote: > In D150803#4389040 , @dschuff wrote: > >> Hm, this is interesting because in the long term we plan

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

brendandahl marked 2 inline comments as done. brendandahl added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7638 + D->addAttr(::new (S.Context) WebAssemblyAsyncAttr(S.Context, AL)); + D->addAttr(UsedAttr::CreateImplicit(S.Context)); +}

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

brendandahl updated this revision to Diff 529028. brendandahl added a comment. Review comments. Add tombstone for unused functions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150803/new/ https://reviews.llvm.org/D150803 Files:

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

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

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

dschuff added a comment. In D150803#4350554 , @sbc100 wrote: > This change looks really nice. I like the new relocation type, I think we > would have had to add that sooner or later anyway. > > My only major concern is making this attribute available

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

brendandahl added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:892 if (const auto *Attr = FD->getAttr()) { -llvm::Function *Fn = cast(GV); +auto *Fn = cast(GV); llvm::AttrBuilder B(GV->getContext()); Whoops, I

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

erichkeane added a comment. Couple of CFE comments, otherwise LGTM. Comment at: clang/docs/ReleaseNotes.rst:559 ^^^ +- Added ``attribute((wasm_async))`` to indicate that a function should be used with + JavaScript Promise Integration (JSPI).

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

brendandahl updated this revision to Diff 527218. brendandahl marked an inline comment as done. brendandahl added a comment. Address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150803/new/ https://reviews.llvm.org/D150803 Files:

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

brendandahl marked 12 inline comments as done. brendandahl added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7645 + if (FD->isThisDeclarationADefinition()) { +S.Diag(D->getLocation(), diag::err_alias_is_definition) << FD << 0; +return;

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

aaron.ballman added a reviewer: erichkeane. aaron.ballman added a comment. This is missing all the usual Sema tests that the attribute only applies to functions, takes no arguments, only works in web assembly targets, etc. Also, the changes should come with a release note so users know about

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

sbc100 added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1984 + TargetSpecificAttr { + let Spellings = [Clang<"wasm_async">]; + let Documentation = [WebAssemblyAsyncDocs]; brendandahl wrote: > sbc100 wrote: > > Should

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

brendandahl added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1984 + TargetSpecificAttr { + let Spellings = [Clang<"wasm_async">]; + let Documentation = [WebAssemblyAsyncDocs]; sbc100 wrote: > Should we call this

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

brendandahl updated this revision to Diff 525348. brendandahl marked 5 inline comments as done. brendandahl added a comment. Review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150803/new/ https://reviews.llvm.org/D150803 Files:

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

aheejin added a comment. Nice! Just some drive-by nitpicking, sorry  Comment at: lld/test/wasm/async.ll:11 + +define void @bar() #1 { +ret void Comment at: lld/test/wasm/async.ll:21-22 + +attributes #0 = { "wasm-async" } +attributes #1

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

sbc100 added a comment. This change looks really nice. I like the new relocation type, I think we would have had to add that sooner or later anyway. My only major concern is making this attribute available outside of emscripten (i.e. wasm32-unknown-emscripten). It seems like we should maybe

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

brendandahl created this revision. brendandahl added a reviewer: sbc100. Herald added subscribers: pmatos, asb, jdoerfert, ecnelises, sunfish, hiraditya, jgravelle-google, dschuff. Herald added a reviewer: aaron.ballman. Herald added a project: All. brendandahl requested review of this revision.