[PATCH] D118995: [WebAssembly] Refactor and fix emission of external IR global decls

[Paulo Matos via Phabricator via llvm-commits]

pmatos added inline comments.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp:185
     Type *GlobalVT = GV->getValueType();
-    computeLegalValueVTs(TLI, GV->getParent()->getContext(),
-                         GV->getParent()->getDataLayout(), GlobalVT, VTs);
+    if (Subtarget) {
+      // Subtarget is only set when a function is defined, because
----------------
To be honest, we shouldn't be using Subtarget or TLI here at all since using these means we are inside a Function. But we aren't, what we do is set Subtarget from the last Function we saw. Which means that it will always be assigned to the Subtarget from the last seen Function. This might be ok in the Wasm backend but not in general and breaks when no function is defined. However, as far as I can tell from doing some checks whenever we call this _and_ Subtarget is `null`, we don't need to call `computeLegalValueVTs` therefore I added this check.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp:386
+  // emitExternalDecls() is not called until now.
   emitExternalDecls(M);
 
----------------
Added a comment here so we don't forget why this is necessary. Removing this call will cause `hidden-decl.ll` to fail.


================
Comment at: llvm/test/CodeGen/WebAssembly/hidden-decl.ll:1
+; RUN: llc < %s --mtriple=wasm32-unknown-unknown -asm-verbose=false -mattr=+reference-types | FileCheck %s
+
----------------
This test is the test that I extracted from the emscripten failure you referred to in D118122.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118995