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

[Sam Clegg via Phabricator via llvm-commits]

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 called it `em_async` or something like that?  And make it illegal on other targets?

@dschuff WDYT?



================
Comment at: clang/include/clang/Basic/Attr.td:1984
+                       TargetSpecificAttr<TargetWebAssembly> {
+  let Spellings = [Clang<"wasm_async">];
+  let Documentation = [WebAssemblyAsyncDocs];
----------------
Should we call this em_async or emscripten_async since this is an emscripten-specific attribute?


================
Comment at: llvm/lib/MC/MCExpr.cpp:364
+  case VK_WASM_FUNCINDEX:
+    return "FUNCINDEX";
   case VK_AMDGPU_GOTPCREL32_LO: return "gotpcrel32 at lo";
----------------
Maybe this on single line to match the local convention.


================
Comment at: llvm/lib/MC/MCExpr.cpp:530
+      .Case("tpoff_lo", VK_VE_TPOFF_LO32)
+      .Default(VK_Invalid);
 }
----------------
Was this whole block indented?  Maybe limit this to just a single line change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150803