[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:555
+  const Module *M = MMI->getModule();
+  emitExternalDecls(*M);
   assert(MF->getConstantPool()->getConstants().empty() &&
----------------
sbc100 wrote:
> Just `emitExternalDecls(*MMI->getModule())` maybe?
Sure!


================
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
+
----------------
sbc100 wrote:
> 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?
Correct, `hidden` is not necessary, just part of the file that I extracted.


================
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
----------------
sbc100 wrote:
> 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).
Sure, I will do the change now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118995