[PATCH] D79137: [WebAssmebly] Add support for defined wasm globals in MC and lld

[Wouter van Oortmerssen via Phabricator via llvm-commits]

aardappel added inline comments.


================
Comment at: lld/test/wasm/globals.s:5
+
+.globl _start
+.globl read_global
----------------
I presume there's a reason or precedent why this is not `.global` ?


================
Comment at: lld/test/wasm/globals.s:13
+read_global:
+  .functype read_global () -> (i32)
+  global.get foo_global
----------------
You were saying earlier there's a lot of boilerplate to writing a function in asm.. this seems pretty reasonable?


================
Comment at: lld/test/wasm/globals.s:31
+
+foo_global:
+bar_global:
----------------
Are these labels required and if so, why?


================
Comment at: llvm/lib/MC/WasmObjectWriter.cpp:797
+
+  SectionBookkeeping Section;
+  startSection(Section, wasm::WASM_SEC_GLOBAL);
----------------
So why is this section entirely new? Where did `__stackpointer` etc go previously?


================
Comment at: llvm/test/MC/WebAssembly/globals.s:4
+
+# Tests creating an accessing actual wasm globals
+
----------------
I guess having almost the same test in LLD and MC is unavoidable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79137