[WebAssembly] Emit .import_global assembler directives

[mgrang]

lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp

136

nit: You can leave out the braces here.

[sunfish]

This is ok, for now at least.

I was imagining that we'd eventually want to just take all undefined global symbols and automatically generate imports for them. Do you think this would work?

[jgravelle-google]

In D26875#600723, @sunfish wrote:

I was imagining that we'd eventually want to just take all undefined global symbols and automatically generate imports for them. Do you think this would work?

In my mind this makes it more explicit what the intent is, whereas auto-generating imports makes it unclear whether an undefined symbol is an error, or if it will be linked in, and we won't know until run-time. Even then we won't know whether the module is trying to import an invalid symbol, or whether the environment isn't providing that symbol. To me this makes it clearer what the module expects.

Although long-term we want to be outputting nearly-valid wasm as part of our .o format. I think at that point we'd still want to explicitly model imports/exports.

[dschuff]

I think this is fine for now too. For the longer term, I kind of think it depends on the use case.

For the case of something that looks like a traditional self-contained C program, I think we'd want to have undefined symbols behave like they do in traditional toolchains (which will be less surprising, and diagnose errors at link time rather than at load time).

For dylibs meant to be loaded by LLVM-built "main" modules, we probably just want something explicit (in the source) like dllimport or ELF visibility or whatever.

For a new/hybrid use case of a wasm library meant to be used from JS, that will be sort of like linking a dylib but not exactly (may have a different ABI, etc).

For the dylib/JS-module use case it might be useful to have the linker automatically convert undefined symbols to imports (as is the default in ELF for example). Although given the popularity and usefulness of -fvisibility=hidden, maybe it's not what we actually want. In any case I think we may want some explicit way to model it in the contract between the compiler and the linker, to at least allow for different ways to make it work? Anyway, we should probably discuss more in a github issue or something.

lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.h

public:
   /// .functype
   virtual void emitIndirectFunctionType(StringRef name,
                                         SmallVectorImpl<MVT> &Params,
                                         SmallVectorImpl<MVT> &Results) {
     llvm_unreachable("emitIndirectFunctionType not implemented");
   }
   /// .indidx
   virtual void emitIndIdx(const MCExpr *Value) = 0;
+  /// .import_global
+  virtual void emitGlobalImport(StringRef name) = 0;
 };
 
 /// This part is for ascii assembly output
 class WebAssemblyTargetAsmStreamer final : public WebAssemblyTargetStreamer {
   formatted_raw_ostream &OS;
 
 public:
   WebAssemblyTargetAsmStreamer(MCStreamer &S, formatted_raw_ostream &OS);
 
   void emitParam(ArrayRef<MVT> Types) override;
   void emitResult(ArrayRef<MVT> Types) override;
   void emitLocal(ArrayRef<MVT> Types) override;
   void emitEndFunc() override;
   void emitIndirectFunctionType(StringRef name,
                                 SmallVectorImpl<MVT> &Params,
                                 SmallVectorImpl<MVT> &Results) override;
   void emitIndIdx(const MCExpr *Value) override;
+  void emitGlobalImport(StringRef name) override;
 };
 
 /// This part is for ELF object output
 class WebAssemblyTargetELFStreamer final : public WebAssemblyTargetStreamer {
 public:
   explicit WebAssemblyTargetELFStreamer(MCStreamer &S);
 
   void emitParam(ArrayRef<MVT> Types) override;
   void emitResult(ArrayRef<MVT> Types) override;
   void emitLocal(ArrayRef<MVT> Types) override;
   void emitEndFunc() override;
   void emitIndirectFunctionType(StringRef name,
                                 SmallVectorImpl<MVT> &Params,
                                 SmallVectorImpl<MVT> &Results) override;
   void emitIndIdx(const MCExpr *Value) override;
+  void emitGlobalImport(StringRef name) override;
 };
 
 } // end namespace llvm
 
 #endif

lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.cpp

else {
     assert(Results.size() == 1);
     OS << ", " << WebAssembly::TypeToString(Results.front());
   }
   for (auto Ty : Params)
     OS << ", " << WebAssembly::TypeToString(Ty);
   OS << '\n';
 }
 
+void WebAssemblyTargetAsmStreamer::emitGlobalImport(StringRef name) {
+  OS << "\t.import_global\t" << name << '\n';
+}
+
 void WebAssemblyTargetAsmStreamer::emitIndIdx(const MCExpr *Value) {
   OS << "\t.indidx  \t" << *Value << '\n';
 }
 
 void WebAssemblyTargetELFStreamer::emitParam(ArrayRef<MVT> Types) {
   // Nothing to emit; params are declared as part of the function signature.
 }
 
void WebAssemblyTargetELFStreamer::emitIndIdx(const MCExpr *Value) {
   llvm_unreachable(".indidx encoding not yet implemented");
 }
 
 void WebAssemblyTargetELFStreamer::emitIndirectFunctionType(
     StringRef name, SmallVectorImpl<MVT> &Params, SmallVectorImpl<MVT> &Results) {
   // Nothing to emit here. TODO: Re-design how linking works and re-evaluate
   // whether it's necessary for .o files to declare indirect function types.
 }
+
+void WebAssemblyTargetELFStreamer::emitGlobalImport(StringRef name) {
+}
 No newline at end of file

lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp

for (const auto &F : M) {
     if (F.isDeclarationForLinker() && !F.isIntrinsic()) {
       SmallVector<MVT, 4> Results;
       SmallVector<MVT, 4> Params;
       ComputeSignatureVTs(F, TM, Params, Results);
       getTargetStreamer()->emitIndirectFunctionType(F.getName(), Params,
                                                     Results);
     }
   }
+  for (const auto &G : M.globals()) {
+    if (!G.hasInitializer() && G.hasExternalLinkage()) {
+      getTargetStreamer()->emitGlobalImport(G.getGlobalIdentifier());
+    }
+  }
 }
 
 void WebAssemblyAsmPrinter::EmitConstantPool() {
   assert(MF->getConstantPool()->getConstants().empty() &&
          "WebAssembly disables constant pools");
 }
 
 void WebAssemblyAsmPrinter::EmitJumpTableInfo() {

test/CodeGen/WebAssembly/globl.ll

 ; RUN: llc < %s -asm-verbose=false | FileCheck %s
 
 target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
 target triple = "wasm32-unknown-unknown"
 
 ; CHECK: .globl foo
 ; CHECK-LABEL: foo:
 define void @foo() {
   ret void
 }
+
+; Check import directives - must be at the end of the file
+; CHECK: .import_global bar{{$}}
+@bar = external global i32