[WebAssembly] MC: Don't generate extra types for weak alias

[sbc100]

Summary

Previously we were generated a void(void) function type
for a weak alias. Update the weak-alias test case to
catch this.

Also, fix type if relocation index to be unsigned inline
with the spec (Linking.md)

[dschuff]

lib/MC/WasmObjectWriter.cpp

962

so a weak alias both isVariable() and isFunction()?

[sbc100]

lib/MC/WasmObjectWriter.cpp

962

Yup

[sbc100]

• [WebAssembly] Fix types for address taken functions

[sbc100]

• [WebAssembly] Fix types for address taken functions

[sbc100]

• Revert

[dschuff]

lib/MC/WasmObjectWriter.cpp

962

That's.... fun. Maybe should add a comment here about that.

[sbc100]

• add comment

include/llvm/BinaryFormat/Wasm.h

 struct WasmElemSegment {
   uint32_t TableIndex;
   WasmInitExpr Offset;
   std::vector<uint32_t> Functions;
 };
 
 struct WasmRelocation {
   uint32_t Type;   // The type of the relocation.
-  int32_t Index;   // Index into function to global index space.
+  uint32_t Index;  // Index into function to global index space.
   uint64_t Offset; // Offset from the start of the section.
   int64_t Addend;  // A value to add to the symbol.
 };
 
 struct WasmLinkingData {
   uint32_t DataSize;
   uint32_t DataAlignment;
 };

include/llvm/MC/MCSymbolWasm.h

 //===- MCSymbolWasm.h -  ----------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 #ifndef LLVM_MC_MCSYMBOLWASM_H
 #define LLVM_MC_MCSYMBOLWASM_H
 
 #include "llvm/BinaryFormat/Wasm.h"
 #include "llvm/MC/MCSymbol.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/Support/raw_ostream.h"
+
+#define DEBUG_TYPE "mc"
 
 namespace llvm {
 
 class MCSymbolWasm : public MCSymbol {
 private:
   bool IsFunction = false;
   bool IsWeak = false;
   std::string ModuleName;
   SmallVector<wasm::ValType, 1> Returns;
   SmallVector<wasm::ValType, 4> Params;
+  bool ParamsSet = false;
+  bool ReturnsSet = false;
 
   /// An expression describing how to calculate the size of a symbol. If a
   /// symbol has no size this field will be NULL.
   const MCExpr *SymbolSize = nullptr;
 
 public:
   // Use a module name of "env" for now, for compatibility with existing tools.
   // This is temporary, and may change, as the ABI is not yet stable.
   MCSymbolWasm(const StringMapEntry<bool> *Name, bool isTemporary)
       : MCSymbol(SymbolKindWasm, Name, isTemporary),
         ModuleName("env") {}
   static bool classof(const MCSymbol *S) { return S->isWasm(); }
 
   const MCExpr *getSize() const { return SymbolSize; }
   void setSize(const MCExpr *SS) { SymbolSize = SS; }
 
   bool isFunction() const { return IsFunction; }
   void setIsFunction(bool isFunc) { IsFunction = isFunc; }
 
   bool isWeak() const { return IsWeak; }
   void setWeak(bool isWeak) { IsWeak = isWeak; }
 
   const StringRef getModuleName() const { return ModuleName; }
 
-  const SmallVector<wasm::ValType, 1> &getReturns() const { return Returns; }
+  const SmallVector<wasm::ValType, 1> &getReturns() const {
+    assert(ReturnsSet);
+    return Returns;
+  }
 
   void setReturns(SmallVectorImpl<wasm::ValType> &&Rets) {
+    DEBUG(dbgs() << "setReturns: " << getName() << ": " << Rets.size() << "\n");
+    if (ReturnsSet) {
+      assert(Returns == Rets);
+      return;
+    }
+    ReturnsSet = true;
     Returns = std::move(Rets);
   }
 
-  const SmallVector<wasm::ValType, 4> &getParams() const { return Params; }
+  const SmallVector<wasm::ValType, 4> &getParams() const {
+    assert(ParamsSet);
+    return Params;
+  }
 
   void setParams(SmallVectorImpl<wasm::ValType> &&Pars) {
+    DEBUG(dbgs() << "setParams: " << getName() << ": " << Pars.size() << "\n");
+    if (ParamsSet) {
+      assert(Pars == Params);
+      return;
+    }
+    ParamsSet = true;
     Params = std::move(Pars);
   }
+
+  void print(raw_ostream &OS) const {
+    OS << getName() << ": isDefined=" << isDefined() << " isExternal="
+       << isExternal() << " isTemporary=" << isTemporary()
+       << " isFunction=" << isFunction()
+       << " isWeak=" << isWeak()
+       << " isVariable=" << isVariable();
+  }
+
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+  LLVM_DUMP_METHOD void dump() const {
+    dbgs() << *this;
+  }
+#endif
 };
 
+inline raw_ostream &operator<<(raw_ostream &OS, const MCSymbolWasm &Sym) {
+  Sym.print(OS);
+  return OS;
+}
+
 }  // end namespace llvm
 
+#undef DEBUG_TYPE
+
 #endif // LLVM_MC_MCSYMBOLWASM_H

lib/MC/WasmObjectWriter.cpp

bool hasAddend() const {
     case wasm::R_WEBASSEMBLY_GLOBAL_ADDR_I32:
       return true;
     default:
       return false;
     }
   }
 
   void print(raw_ostream &Out) const {
-    Out << "Off=" << Offset << ", Sym=" << Symbol << ", Addend=" << Addend
+    Out << "Off=" << Offset << ", Sym=" << *Symbol << ", Addend=" << Addend
         << ", Type=" << Type << ", FixupSection=" << FixupSection;
   }
 
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
   LLVM_DUMP_METHOD void dump() const { print(dbgs()); }
 #endif
 };
 
for (const WasmRelocationEntry &RelEntry : DataRelocations) {
     case wasm::R_WEBASSEMBLY_GLOBAL_ADDR_I32:
       IsAddressTaken.insert(RelEntry.Symbol);
       break;
     default:
       break;
     }
   }
 
-  // Populate the Imports set.
+  // Populate Imports and FunctionTypes.
   for (const MCSymbol &S : Asm.symbols()) {
+    if (S.isVariable())
+      continue;
+
     const auto &WS = static_cast<const MCSymbolWasm &>(S);
     int32_t Type;
 
     if (WS.isFunction()) {
       // Prepare the function's type, if we haven't seen it yet.
       WasmFunctionType F;
       F.Returns = WS.getReturns();
       F.Params = WS.getParams();
for (const MCSymbol &S : Asm.symbols()) {
     if (S.isTemporary() && S.getName().empty())
       continue;
 
     // Variable references (weak references) are handled in a second pass
     if (S.isVariable())
       continue;
 
     const auto &WS = static_cast<const MCSymbolWasm &>(S);
-    DEBUG(dbgs() << "MCSymbol: '" << S << "'"
-                 << " isDefined=" << S.isDefined() << " isExternal="
-                 << S.isExternal() << " isTemporary=" << S.isTemporary()
-                 << " isFunction=" << WS.isFunction()
-                 << " isWeak=" << WS.isWeak()
-                 << " isVariable=" << WS.isVariable() << "\n");
+    DEBUG(dbgs() << "MCSymbol: " << WS << "\n");
 
     if (WS.isWeak())
       WeakSymbols.push_back(WS.getName());
 
     unsigned Index;
 
     if (WS.isFunction()) {
       // Prepare the function's type, if we haven't seen it yet.

lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.h

public:
   virtual void emitLocal(ArrayRef<MVT> Types) = 0;
   /// .globalvar
   virtual void emitGlobal(ArrayRef<wasm::Global> Globals) = 0;
   /// .stack_pointer
   virtual void emitStackPointer(uint32_t Index) = 0;
   /// .endfunc
   virtual void emitEndFunc() = 0;
   /// .functype
-  virtual void emitIndirectFunctionType(StringRef name,
+  virtual void emitIndirectFunctionType(MCSymbol *Symbol,
                                         SmallVectorImpl<MVT> &Params,
                                         SmallVectorImpl<MVT> &Results) = 0;
   /// .indidx
   virtual void emitIndIdx(const MCExpr *Value) = 0;
   /// .import_global
   virtual void emitGlobalImport(StringRef name) = 0;
 
 protected:
   void emitValueType(wasm::ValType Type);
 };
 
 /// 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(MCSymbol *Symbol, ArrayRef<MVT> Types) override;
   void emitResult(MCSymbol *Symbol, ArrayRef<MVT> Types) override;
   void emitLocal(ArrayRef<MVT> Types) override;
   void emitGlobal(ArrayRef<wasm::Global> Globals) override;
   void emitStackPointer(uint32_t Index) override;
   void emitEndFunc() override;
-  void emitIndirectFunctionType(StringRef name,
+  void emitIndirectFunctionType(MCSymbol *Symbol,
                                 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(MCSymbol *Symbol, ArrayRef<MVT> Types) override;
   void emitResult(MCSymbol *Symbol, ArrayRef<MVT> Types) override;
   void emitLocal(ArrayRef<MVT> Types) override;
   void emitGlobal(ArrayRef<wasm::Global> Globals) override;
   void emitStackPointer(uint32_t Index) override;
   void emitEndFunc() override;
-  void emitIndirectFunctionType(StringRef name,
+  void emitIndirectFunctionType(MCSymbol *Symbol,
                                 SmallVectorImpl<MVT> &Params,
                                 SmallVectorImpl<MVT> &Results) override;
   void emitIndIdx(const MCExpr *Value) override;
   void emitGlobalImport(StringRef name) override;
 };
 
 /// This part is for Wasm object output
 class WebAssemblyTargetWasmStreamer final : public WebAssemblyTargetStreamer {
 public:
   explicit WebAssemblyTargetWasmStreamer(MCStreamer &S);
 
   void emitParam(MCSymbol *Symbol, ArrayRef<MVT> Types) override;
   void emitResult(MCSymbol *Symbol, ArrayRef<MVT> Types) override;
   void emitLocal(ArrayRef<MVT> Types) override;
   void emitGlobal(ArrayRef<wasm::Global> Globals) override;
   void emitStackPointer(uint32_t Index) override;
   void emitEndFunc() override;
-  void emitIndirectFunctionType(StringRef name,
+  void emitIndirectFunctionType(MCSymbol *Symbol,
                                 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

 
 void WebAssemblyTargetAsmStreamer::emitStackPointer(uint32_t Index) {
   OS << "\t.stack_pointer\t" << Index << '\n';
 }
 
 void WebAssemblyTargetAsmStreamer::emitEndFunc() { OS << "\t.endfunc\n"; }
 
 void WebAssemblyTargetAsmStreamer::emitIndirectFunctionType(
-    StringRef name, SmallVectorImpl<MVT> &Params, SmallVectorImpl<MVT> &Results) {
-  OS << "\t.functype\t" << name;
+    MCSymbol *Symbol, SmallVectorImpl<MVT> &Params, SmallVectorImpl<MVT> &Results) {
+  OS << "\t.functype\t" << Symbol->getName();
   if (Results.empty())
     OS << ", void";
   else {
     assert(Results.size() == 1);
     OS << ", " << WebAssembly::TypeToString(Results.front());
   }
   for (auto Ty : Params)
     OS << ", " << WebAssembly::TypeToString(Ty);
void WebAssemblyTargetELFStreamer::emitEndFunc() {
   Streamer.EmitIntValue(WebAssembly::End, 1);
 }
 
 void WebAssemblyTargetELFStreamer::emitIndIdx(const MCExpr *Value) {
   llvm_unreachable(".indidx encoding not yet implemented");
 }
 
 void WebAssemblyTargetELFStreamer::emitIndirectFunctionType(
-    StringRef name, SmallVectorImpl<MVT> &Params, SmallVectorImpl<MVT> &Results) {
+    MCSymbol *Symbol, 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) {
 }
 
 void WebAssemblyTargetWasmStreamer::emitParam(MCSymbol *Symbol,
void WebAssemblyTargetWasmStreamer::emitEndFunc() {
   llvm_unreachable(".end_func is not needed for direct wasm output");
 }
 
 void WebAssemblyTargetWasmStreamer::emitIndIdx(const MCExpr *Value) {
   llvm_unreachable(".indidx encoding not yet implemented");
 }
 
 void WebAssemblyTargetWasmStreamer::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.
+    MCSymbol *Symbol, SmallVectorImpl<MVT> &Params,
+    SmallVectorImpl<MVT> &Results) {
+  MCSymbolWasm *WasmSym = cast<MCSymbolWasm>(Symbol);
+  if (WasmSym->isFunction()) {
+    // Symbol already has its arguments and result set.
+    return;
+  }
+
+  SmallVector<wasm::ValType, 4> ValParams;
+  for (MVT Ty : Params)
+    ValParams.push_back(WebAssembly::toValType(Ty));
+
+  SmallVector<wasm::ValType, 4> ValResults;
+  for (MVT Ty : Results)
+    ValResults.push_back(WebAssembly::toValType(Ty));
+
+  WasmSym->setParams(std::move(ValParams));
+  WasmSym->setReturns(std::move(ValResults));
+  WasmSym->setIsFunction(true);
 }
 
 void WebAssemblyTargetWasmStreamer::emitGlobalImport(StringRef name) {
 }

lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp

 
 void WebAssemblyAsmPrinter::EmitEndOfAsmFile(Module &M) {
   for (const auto &F : M) {
     // Emit function type info for all undefined functions
     if (F.isDeclarationForLinker() && !F.isIntrinsic()) {
       SmallVector<MVT, 4> Results;
       SmallVector<MVT, 4> Params;
       ComputeSignatureVTs(F, TM, Params, Results);
-      getTargetStreamer()->emitIndirectFunctionType(F.getName(), Params,
+      getTargetStreamer()->emitIndirectFunctionType(getSymbol(&F), Params,
                                                     Results);
     }
   }
   for (const auto &G : M.globals()) {
     if (!G.hasInitializer() && G.hasExternalLinkage()) {
       uint16_t Size = M.getDataLayout().getTypeAllocSize(G.getValueType());
       getTargetStreamer()->emitGlobalImport(G.getGlobalIdentifier());
       OutStreamer->emitELFSize(getSymbol(&G),
default: {
     break;
   }
   }
 }
 
 const MCExpr *WebAssemblyAsmPrinter::lowerConstant(const Constant *CV) {
   if (const GlobalValue *GV = dyn_cast<GlobalValue>(CV))
     if (GV->getValueType()->isFunctionTy()) {
-      MCSymbol* Sym = getSymbol(GV);
-      if (!isa<MCSymbolELF>(Sym))
-        cast<MCSymbolWasm>(Sym)->setIsFunction(true);
       return MCSymbolRefExpr::create(
-          Sym, MCSymbolRefExpr::VK_WebAssembly_FUNCTION, OutContext);
+          getSymbol(GV), MCSymbolRefExpr::VK_WebAssembly_FUNCTION, OutContext);
     }
   return AsmPrinter::lowerConstant(CV);
 }
 
 bool WebAssemblyAsmPrinter::PrintAsmOperand(const MachineInstr *MI,
                                             unsigned OpNo, unsigned AsmVariant,
                                             const char *ExtraCode,
                                             raw_ostream &OS) {

lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp

 }
 
 MCOperand WebAssemblyMCInstLower::LowerSymbolOperand(MCSymbol *Sym,
                                                      int64_t Offset,
                                                      bool IsFunc) const {
   MCSymbolRefExpr::VariantKind VK =
       IsFunc ? MCSymbolRefExpr::VK_WebAssembly_FUNCTION
              : MCSymbolRefExpr::VK_None;
-  if (!isa<MCSymbolELF>(Sym))
-    cast<MCSymbolWasm>(Sym)->setIsFunction(IsFunc);
 
   const MCExpr *Expr = MCSymbolRefExpr::create(Sym, VK, Ctx);
 
   if (Offset != 0) {
     if (IsFunc)
       report_fatal_error("Function addresses with offsets not supported");
     Expr =
         MCBinaryExpr::createAdd(Expr, MCConstantExpr::create(Offset, Ctx), Ctx);

test/MC/WebAssembly/external-func-address.ll

 ; RUN: llc -mtriple wasm32-unknown-unknown-wasm -filetype=obj %s -o - | obj2yaml | FileCheck %s
 ; Verify that addresses of external functions generate correctly typed
 ; imports and relocations or type R_TABLE_INDEX_I32.
 
-declare void @f1() #1
-@ptr_to_f1 = hidden global void ()* @f1, align 4
-
+declare void @f1(i32) #1
+@ptr_to_f1 = hidden global void (i32)* @f1, align 4
 
-; CHECK:   - Type:            IMPORT
-; CHECK:     Imports:
-; CHECK:       - Module:          env
-; CHECK:         Field:           f1
-; CHECK:         Kind:            FUNCTION
-; CHECK:         SigIndex:        0
-; CHECK:   - Type:            ELEM
-; CHECK:     Segments:
-; CHECK:       - Offset:
-; CHECK:           Opcode:          I32_CONST
-; CHECK:           Value:           0
-; CHECK:         Functions:       [ 0 ]
-; CHECK:   - Type:            DATA
-; CHECK:     Relocations:
-; CHECK:       - Type:            R_WEBASSEMBLY_TABLE_INDEX_I32
-; CHECK:         Index:           0
-; CHECK:         Offset:          0x00000006
+; CHECK:      --- !WASM
+; CHECK-NEXT: FileHeader:      
+; CHECK-NEXT:   Version:         0x00000001
+; CHECK-NEXT: Sections:        
+; CHECK-NEXT:   - Type:            TYPE
+; CHECK-NEXT:     Signatures:      
+; CHECK-NEXT:       - Index:           0
+; CHECK-NEXT:         ReturnType:      NORESULT
+; CHECK-NEXT:         ParamTypes:      
+; CHECK-NEXT:           - I32
+; CHECK:        - Type:            IMPORT
+; CHECK-NEXT:     Imports:
+; CHECK-NEXT:       - Module:          env
+; CHECK-NEXT:         Field:           f1
+; CHECK-NEXT:         Kind:            FUNCTION
+; CHECK-NEXT:         SigIndex:        0
+; CHECK:        - Type:            ELEM
+; CHECK-NEXT:     Segments:
+; CHECK-NEXT:       - Offset:
+; CHECK-NEXT:           Opcode:          I32_CONST
+; CHECK-NEXT:           Value:           0
+; CHECK-NEXT:         Functions:       [ 0 ]
+; CHECK:        - Type:            DATA
+; CHECK-NEXT:     Relocations:
+; CHECK-NEXT:       - Type:            R_WEBASSEMBLY_TABLE_INDEX_I32
+; CHECK-NEXT:         Index:           0
+; CHECK-NEXT:         Offset:          0x00000006

test/MC/WebAssembly/weak-alias.ll

 ; RUN: llc -mtriple wasm32-unknown-unknown-wasm -filetype=obj %s -o - | obj2yaml | FileCheck %s
 
 ; foo_alias() function is weak alias of function foo()
 ; Generates two exports of the same function, one of them weak
 
 @foo_alias = weak hidden alias i32 (...), bitcast (i32 ()* @foo to i32 (...)*)
 
 define hidden i32 @foo() #0 {
 entry:
   ret i32 0
 }
 
+; CHECK:        - Type:            TYPE
+; CHECK-NEXT:     Signatures:      
+; CHECK-NEXT:       - Index:           0
+; CHECK-NEXT:         ReturnType:      I32
+; CHECK-NEXT:         ParamTypes:      
+; CHECK-NEXT:   - Type:            FUNCTION
+; CHECK-NEXT:     FunctionTypes:   [ 0 ]
+
 ; CHECK:        - Type:            EXPORT
 ; CHECK-NEXT:     Exports:         
 ; CHECK-NEXT:       - Name:            foo
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           0
 ; CHECK-NEXT:       - Name:            foo_alias
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           0
 
-
 ; CHECK:        - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            name
 ; CHECK-NEXT:     FunctionNames:   
 ; CHECK-NEXT:       - Index:           0
 ; CHECK-NEXT:         Name:            foo
 ; CHECK-NEXT:   - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            linking
 ; CHECK-NEXT:     DataSize:        0
 ; CHECK-NEXT:     DataAlignment:   0
 ; CHECK-NEXT:     SymbolInfo:      
 ; CHECK-NEXT:       - Name:            foo_alias
 ; CHECK-NEXT:         Flags:           1
 ; CHECK-NEXT: ...