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

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added a comment.

lgtm % comments



================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp:555
+  const Module *M = MMI->getModule();
+  emitExternalDecls(*M);
   assert(MF->getConstantPool()->getConstants().empty() &&
----------------
Just `emitExternalDecls(*MMI->getModule())` maybe?


================
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
+
----------------
pmatos wrote:
> This test is the test that I extracted from the emscripten failure you referred to in D118122.
IIUC that "hidden" thing is no actually important there... isn't the important part that there are no functions declared here?


================
Comment at: llvm/test/CodeGen/WebAssembly/hidden-decl.ll:17
+; CHECK: .functype foo_write (i32, i32, i32) -> (i32)
+declare hidden i32 @foo_write(%struct.IO_FILE* noundef, i8* noundef, i32 noundef) #0
----------------
Can we make this a little simpler and more obvious what its testing? 

Maybe just a function `foo` and some data `foo_ptr` which stores a pointer to that function?   (I think you can remove all the function args, the 'hidden` keywords and the #0 at the end).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118995