[WebAssembly] Move toString helpers to BinaryFormat. NFC.

[sbc100]

-typo

[vsk]

Hi,

I'm seeing a few link failures due to this commit (I think):

Undefined symbols for architecture x86_64:
  "llvm::wasm::toString(llvm::wasm::WasmSymbolType)", referenced from:
      llvm::object::WasmSymbol::print(llvm::raw_ostream&) const in RandomAccessVisitorTest.cpp.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

This affects the llvm-rc binary and some unit tests. I'm not sure why others aren't seeing the same build break -- possibly this is due to my building with modules. Here's a candidate fix:

diff --git a/tools/llvm-rc/CMakeLists.txt b/tools/llvm-rc/CMakeLists.txt
index 4cadc176691..175de768bf7 100644
--- a/tools/llvm-rc/CMakeLists.txt
+++ b/tools/llvm-rc/CMakeLists.txt
@@ -1,6 +1,7 @@
 set(LLVM_LINK_COMPONENTS
   Option
   Support
+  BinaryFormat
   )
 
 set(LLVM_TARGET_DEFINITIONS Opts.td)
diff --git a/unittests/DebugInfo/CodeView/CMakeLists.txt b/unittests/DebugInfo/CodeView/CMakeLists.txt
index 70a7b8af144..ea3adc6f2ea 100644
--- a/unittests/DebugInfo/CodeView/CMakeLists.txt
+++ b/unittests/DebugInfo/CodeView/CMakeLists.txt
@@ -1,5 +1,6 @@
 set(LLVM_LINK_COMPONENTS
   DebugInfoCodeView
+  BinaryFormat
   )
 
 add_llvm_unittest(DebugInfoCodeViewTests

Could you please take a look?

[sbc100]

In D46847#1101688, @vsk wrote:

Hi,

I'm seeing a few link failures due to this commit (I think):

Undefined symbols for architecture x86_64:
  "llvm::wasm::toString(llvm::wasm::WasmSymbolType)", referenced from:
      llvm::object::WasmSymbol::print(llvm::raw_ostream&) const in RandomAccessVisitorTest.cpp.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

This affects the llvm-rc binary and some unit tests. I'm not sure why others aren't seeing the same build break -- possibly this is due to my building with modules. Here's a candidate fix:

diff --git a/tools/llvm-rc/CMakeLists.txt b/tools/llvm-rc/CMakeLists.txt
index 4cadc176691..175de768bf7 100644
--- a/tools/llvm-rc/CMakeLists.txt
+++ b/tools/llvm-rc/CMakeLists.txt
@@ -1,6 +1,7 @@
 set(LLVM_LINK_COMPONENTS
   Option
   Support
+  BinaryFormat
   )
 
 set(LLVM_TARGET_DEFINITIONS Opts.td)
diff --git a/unittests/DebugInfo/CodeView/CMakeLists.txt b/unittests/DebugInfo/CodeView/CMakeLists.txt
index 70a7b8af144..ea3adc6f2ea 100644
--- a/unittests/DebugInfo/CodeView/CMakeLists.txt
+++ b/unittests/DebugInfo/CodeView/CMakeLists.txt
@@ -1,5 +1,6 @@
 set(LLVM_LINK_COMPONENTS
   DebugInfoCodeView
+  BinaryFormat
   )
 
 add_llvm_unittest(DebugInfoCodeViewTests

Could you please take a look?

Yes, that looks like this change. I'll take a look. I wonder why I didn't see those errors.

[sbc100]

I can't reproduce those failures locally. I also build with BUILD_SHARED_LIBS=ON so I would expect to see the same errors as you.

Can you see how RandomAccessVisitorTest.cpp makes use of BinaryFormat/Wasm.h? Your fix looks find to me though. Is it just those two targets that fail?

[vsk]

In D46847#1101801, @sbc100 wrote:

I can't reproduce those failures locally. I also build with BUILD_SHARED_LIBS=ON so I would expect to see the same errors as you.

I don't use that option, but I do use LLVM_ENABLE_MODULES=On.

Can you see how RandomAccessVisitorTest.cpp makes use of BinaryFormat/Wasm.h?

In the .o, I see:

__ZNSt3__110unique_ptrIN12_GLOBAL__N_123RandomAccessVisitorTest15GlobalTestStateENS_14default_deleteIS3_EEED1Ev:
...
000000000000032a        movq    __ZNK4llvm6object10WasmSymbol4dumpEv(%rdi), %r14 ## llvm::object::WasmSymbol::dump() const

For some reason, the unit test's GlobalTestState deleter needs to store a function pointer to WasmSymbol::dump? If I remove "LLVM_DUMP_METHOD" from the definition, it no longer appears in the .o. I think something strange with attribute((used)) is happening, and I don't understand why. The only use of this symbol in the IR is in the magic @llvm.used global.

Your fix looks find to me though. Is it just those two targets that fail?

These are the only two targets which fail to build as a part of 'check-llvm'. Do you want to dig any deeper into this, or should I just land the build fix?

[sbc100]

In D46847#1101983, @vsk wrote:

In D46847#1101801, @sbc100 wrote:

I can't reproduce those failures locally. I also build with BUILD_SHARED_LIBS=ON so I would expect to see the same errors as you.

I don't use that option, but I do use LLVM_ENABLE_MODULES=On.

Can you see how RandomAccessVisitorTest.cpp makes use of BinaryFormat/Wasm.h?

In the .o, I see:

__ZNSt3__110unique_ptrIN12_GLOBAL__N_123RandomAccessVisitorTest15GlobalTestStateENS_14default_deleteIS3_EEED1Ev:
...
000000000000032a        movq    __ZNK4llvm6object10WasmSymbol4dumpEv(%rdi), %r14 ## llvm::object::WasmSymbol::dump() const

For some reason, the unit test's GlobalTestState deleter needs to store a function pointer to WasmSymbol::dump? If I remove "LLVM_DUMP_METHOD" from the definition, it no longer appears in the .o. I think something strange with attribute((used)) is happening, and I don't understand why. The only use of this symbol in the IR is in the magic @llvm.used global.

Your fix looks find to me though. Is it just those two targets that fail?

These are the only two targets which fail to build as a part of 'check-llvm'. Do you want to dig any deeper into this, or should I just land the build fix?

Yes, please land the fix. Its a logical extension to the fixes I already made. I'd try with LLVM_ENABLE_MODULES out of curiosity too.

[vsk]

After rebasing on r332530, the fix I suggested no longer works. I'll take another look.

llvm/trunk/include/llvm/BinaryFormat/Wasm.h

 inline bool operator==(const WasmGlobalType &LHS, const WasmGlobalType &RHS) {
   return LHS.Type == RHS.Type && LHS.Mutable == RHS.Mutable;
 }
 
 inline bool operator!=(const WasmGlobalType &LHS, const WasmGlobalType &RHS) {
   return !(LHS == RHS);
 }
 
+std::string toString(wasm::WasmSymbolType type);
+std::string relocTypetoString(uint32_t type);
+
 } // end namespace wasm
 } // end namespace llvm
 
 #endif

llvm/trunk/include/llvm/Object/Wasm.h

bool isHidden() const {
     return getVisibility() == wasm::WASM_SYMBOL_VISIBILITY_HIDDEN;
   }
 
   unsigned getVisibility() const {
     return Info.Flags & wasm::WASM_SYMBOL_VISIBILITY_MASK;
   }
 
   void print(raw_ostream &Out) const {
-    Out << "Name=" << Info.Name << ", Kind=" << int(Info.Kind)
+    Out << "Name=" << Info.Name
+        << ", Kind=" << toString(wasm::WasmSymbolType(Info.Kind))
         << ", Flags=" << Info.Flags;
     if (!isTypeData()) {
       Out << ", ElemIndex=" << Info.ElementIndex;
     } else if (isDefined()) {
       Out << ", Segment=" << Info.DataRef.Segment;
       Out << ", Offset=" << Info.DataRef.Offset;
       Out << ", Size=" << Info.DataRef.Size;
     }

llvm/trunk/lib/BinaryFormat/CMakeLists.txt

 add_llvm_library(LLVMBinaryFormat
   Dwarf.cpp
   Magic.cpp
+  Wasm.cpp
 
   ADDITIONAL_HEADER_DIRS
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/BinaryFormat
   )
+  
 
 

llvm/trunk/lib/BinaryFormat/Wasm.cpp

+//===-- llvm/BinaryFormat/Wasm.cpp -------------------------------*- C++-*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/BinaryFormat/Wasm.h"
+
+std::string llvm::wasm::toString(wasm::WasmSymbolType type) {
+  switch (type) {
+  case wasm::WASM_SYMBOL_TYPE_FUNCTION:
+    return "WASM_SYMBOL_TYPE_FUNCTION";
+  case wasm::WASM_SYMBOL_TYPE_GLOBAL:
+    return "WASM_SYMBOL_TYPE_GLOBAL";
+  case wasm::WASM_SYMBOL_TYPE_DATA:
+    return "WASM_SYMBOL_TYPE_DATA";
+  case wasm::WASM_SYMBOL_TYPE_SECTION:
+    return "WASM_SYMBOL_TYPE_SECTION";
+  }
+  llvm_unreachable("unknown symbol type");
+}
+
+std::string llvm::wasm::relocTypetoString(uint32_t type) {
+  switch (type) {
+#define WASM_RELOC(NAME, VALUE) case VALUE: return #NAME;
+#include "llvm/BinaryFormat/WasmRelocs.def"
+#undef WASM_RELOC
+  default:
+    llvm_unreachable("unknown reloc type");
+  }
+}

llvm/trunk/lib/MC/LLVMBuild.txt

 
 [common]
 subdirectories = MCDisassembler MCParser
 
 [component_0]
 type = Library
 name = MC
 parent = Libraries
-required_libraries = Support DebugInfoCodeView
+required_libraries = Support BinaryFormat DebugInfoCodeView

llvm/trunk/lib/MC/WasmObjectWriter.cpp

 #include "llvm/Support/LEB128.h"
 #include "llvm/Support/StringSaver.h"
 #include <vector>
 
 using namespace llvm;
 
 #define DEBUG_TYPE "mc"
 
-#if !defined(NDEBUG)
-static std::string toString(wasm::WasmSymbolType type) {
-  switch (type) {
-  case wasm::WASM_SYMBOL_TYPE_FUNCTION:
-    return "WASM_SYMBOL_TYPE_FUNCTION";
-  case wasm::WASM_SYMBOL_TYPE_GLOBAL:
-    return "WASM_SYMBOL_TYPE_GLOBAL";
-  case wasm::WASM_SYMBOL_TYPE_DATA:
-    return "WASM_SYMBOL_TYPE_DATA";
-  case wasm::WASM_SYMBOL_TYPE_SECTION:
-    return "WASM_SYMBOL_TYPE_SECTION";
-  }
-  llvm_unreachable("unknown symbol type");
-}
-#endif
-
-static std::string relocTypetoString(uint32_t type) {
-  switch (type) {
-#define WASM_RELOC(NAME, VALUE) case VALUE: return #NAME;
-#include "llvm/BinaryFormat/WasmRelocs.def"
-#undef WASM_RELOC
-  default:
-    llvm_unreachable("uknown reloc type");
-  }
-}
-
 namespace {
 
 // Went we ceate the indirect function table we start at 1, so that there is
 // and emtpy slot at 0 and therefore calling a null function pointer will trap.
 static const uint32_t kInitialTableOffset = 1;
 
 // For patching purposes, we need to remember where each section starts, both
 // for patching up the section size field, and for patching up references to
bool hasAddend() const {
     case wasm::R_WEBASSEMBLY_SECTION_OFFSET_I32:
       return true;
     default:
       return false;
     }
   }
 
   void print(raw_ostream &Out) const {
-    Out << relocTypetoString(Type)
+    Out << wasm::relocTypetoString(Type)
         << " Off=" << Offset << ", Sym=" << *Symbol << ", Addend=" << Addend
         << ", FixupSection=" << FixupSection->getSectionName();
   }
 
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
   LLVM_DUMP_METHOD void dump() const { print(dbgs()); }
 #endif
 };