[PATCH] D101913: [CodeGen][WebAssembly] Better lowering for WASM_SYMBOL_TYPE_GLOBAL symbols

Mon May 10 05:08:47 PDT 2021

wingo added inline comments.

================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp:204
+    OutStreamer->emitLabel(Sym);
+    OutStreamer->AddBlankLine();
+  }
----------------
sbc100 wrote:
> wingo wrote:
> > sbc100 wrote:
> > > There is no actual initializer here.. it will always be zero I guess?  Do we need a TODO?
> > Added the TODO.  This one is tricky because there's no MC support for global initializers yet, AFAIU.
> > 
> > Not sure how it should work, tbh; though the binary format specifies that this should be a sequence of constant instructions followed by `end`, I think I would want to restrict this one from the MC side to be some form of MCConstantExpr, serialized either as part of the `.globaltype` line or as a new `.globalvalue SYM, EXPR` form.  WDYT?  Would be a followup I guess.
> 
> > Not sure how it should work, tbh; though the binary format specifies that this should be a sequence of constant instructions followed by `end`, I think I would want to restrict this one from the MC side to be some form of MCConstantExpr, serialized either as part of the `.globaltype` line or as a new `.globalvalue SYM, EXPR` form.  WDYT?  Would be a followup I guess.
> 
> Yeah, I'm not sure about this either.   Using actual instructions seems overkill, but at the same time matches the spec.   I think it would be great if we could make it work... but also a more declarative mode might sense given the severe restrictions on instructions in cont expressions today.  
> 
> How close are you to needing initializer values here?  Can we punt for a little longer? :)
Thanks for review & sorry I forgot to reply :)  Yeah can wait on the initializers -- in most cases I am interested in, the default value will be fine.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101913