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@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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
