[PATCH] D37497: [WebAssembly] Fix getSymbolValue() for globals that are not 32-bit const values.

[Don Hinton via Phabricator via llvm-commits]

hintonda added inline comments.


================
Comment at: lib/Object/WasmObjectFile.cpp:794
     const wasm::WasmGlobal& Global = Globals[GlobalIndex];
-    return Global.InitExpr.Value.Int32;
+    switch (Global.InitExpr.Opcode) {
+      case wasm::WASM_OPCODE_I32_CONST:
----------------
sbc100 wrote:
> hintonda wrote:
> > Is there any reason not to handle all 5 fields?
> We are running into a some dissonance here between wasm globals in general, and wasm globals as used by llvm as an object file format.
> 
> For clang-generated object files, all the global will be type Int32 and represent memory offsets.   However, wasm globals in general can contain globals of other types.  Specifically the 3 missing types here are Float32, Float64 and Global.  In those cases I'm not sure it makes sense to return anything here (or at least I'm not sure how 'nm' would display it). 
> 
> Now that I think about it, perhaps it would be better to say that only Int32 global make it into the symbol table at all, since other types of globals can't/don't represent symbols (C globals).
Sounds reasonable, but could you add a comment or message to your assert to that effect?  Otherwise it's sorta confusing.


https://reviews.llvm.org/D37497