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

Wouter van Oortmerssen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 30 12:21:13 PDT 2020


aardappel accepted this revision.
aardappel added inline comments.
This revision is now accepted and ready to land.


================
Comment at: lld/test/wasm/globals.s:5
+
+.globl _start
+.globl read_global
----------------
sbc100 wrote:
> aardappel wrote:
> > I presume there's a reason or precedent why this is not `.global` ?
> I don't really know.  It seems that both ".global" and ".globl" are accepted in the input but when `.s` format is written it always seems to write the shorter form, so I assumed it was preferred.
> 
> Indeed, looking at the code it looks like GlobalDirective in  MCAsmInfo is always set to: `"\t.globl\t"`, even though to parser accepts the long form.
might be nice to standardize on `.global` then? up to you.


================
Comment at: lld/test/wasm/globals.s:13
+read_global:
+  .functype read_global () -> (i32)
+  global.get foo_global
----------------
sbc100 wrote:
> aardappel wrote:
> > You were saying earlier there's a lot of boilerplate to writing a function in asm.. this seems pretty reasonable?
> You are right, its not too bad.    I wonder if we could make `end_function` optional though?   Also, IIRC that location of the `.functype` directive is tied (needlessly?) to this position at the start I think. 
`end_function` is an actual instruction, not just a marker, so that's probably not a good idea.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79137





More information about the llvm-commits mailing list