[PATCH] D115749: [WebAssembly] Emit symbol labels for table global symbols

Tue Jan 11 08:44:41 PST 2022

sbc100 added inline comments.

================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp:202
+    // If the GlobalVariable refers to a table, we handle it here instead of
+    // in emitExternalDecls
+    if (Sym->isTable()) 
----------------
pmatos wrote:
> sbc100 wrote:
> > Are you saying that for undefined/external tables we call `emitTableType` elsewhere?   IIUC the table type needs to be declared ragardless of whether the table has a definition (hasInitializer()) or not, right?
> > 
> > This comment seems a little odd to me given that the code below doesn't just handle table symbols.
> Yes, so if the table is extern it's `tabletype` is emitted in `emitExternDecls`. If it's a global defined in the current compilation unit, then it needs to be emitted here.  That's what I was trying to say with my comment, but maybe I need to be more precise.
> 
> Regarding the mention of `hasInitializer()`, I am unsure how to actually test this. I haven't managed to create a table as a global that doesn't enter the condition, i.e. without an initializer. Do you have a suggestion?
I'm not sure... with globals and functions and data symbols you just skip the label, and that makes them externally defined (i.e. without an initializer).

It could be that for some reason we don't support that yet, but maybe thats a case for anther CL?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115749