Skip to content

[WebAssembly] Set IS_64 flag correctly on __indirect_function_table in object files #94487

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 6, 2024

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Jun 5, 2024

Follow up to #92042

@sbc100 sbc100 requested a review from dschuff June 5, 2024 15:30
@llvmbot llvmbot added backend:WebAssembly mc Machine (object) code labels Jun 5, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 5, 2024

@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-webassembly

Author: Sam Clegg (sbc100)

Changes

Follow up to #92042


Full diff: https://github.com/llvm/llvm-project/pull/94487.diff

5 Files Affected:

  • (modified) llvm/include/llvm/MC/MCSymbolWasm.h (+7-4)
  • (modified) llvm/lib/MC/WasmObjectWriter.cpp (+1-1)
  • (modified) llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp (+8-4)
  • (modified) llvm/lib/Target/WebAssembly/WebAssemblyUtilities.cpp (+2-1)
  • (modified) llvm/test/MC/WebAssembly/reloc-pic64.s (+1)
diff --git a/llvm/include/llvm/MC/MCSymbolWasm.h b/llvm/include/llvm/MC/MCSymbolWasm.h
index 0ce95c72a73f7..0c2b97a594231 100644
--- a/llvm/include/llvm/MC/MCSymbolWasm.h
+++ b/llvm/include/llvm/MC/MCSymbolWasm.h
@@ -114,9 +114,11 @@ class MCSymbolWasm : public MCSymbol {
     return isTable() && hasTableType() &&
            getTableType().ElemType == wasm::ValType::FUNCREF;
   }
-  void setFunctionTable() {
+  void setFunctionTable(bool is64) {
     setType(wasm::WASM_SYMBOL_TYPE_TABLE);
-    setTableType(wasm::ValType::FUNCREF);
+    uint8_t flags =
+        is64 ? wasm::WASM_LIMITS_FLAG_IS_64 : wasm::WASM_LIMITS_FLAG_NONE;
+    setTableType(wasm::ValType::FUNCREF, flags);
   }
 
   void setUsedInGOT() const { IsUsedInGOT = true; }
@@ -140,10 +142,11 @@ class MCSymbolWasm : public MCSymbol {
     return *TableType;
   }
   void setTableType(wasm::WasmTableType TT) { TableType = TT; }
-  void setTableType(wasm::ValType VT) {
+  void setTableType(wasm::ValType VT,
+                    uint8_t flags = wasm::WASM_LIMITS_FLAG_NONE) {
     // Declare a table with element type VT and no limits (min size 0, no max
     // size).
-    wasm::WasmLimits Limits = {wasm::WASM_LIMITS_FLAG_NONE, 0, 0};
+    wasm::WasmLimits Limits = {flags, 0, 0};
     setTableType({VT, Limits});
   }
 };
diff --git a/llvm/lib/MC/WasmObjectWriter.cpp b/llvm/lib/MC/WasmObjectWriter.cpp
index 985f9351f4a30..5207af8038234 100644
--- a/llvm/lib/MC/WasmObjectWriter.cpp
+++ b/llvm/lib/MC/WasmObjectWriter.cpp
@@ -877,7 +877,7 @@ void WasmObjectWriter::writeImportSection(ArrayRef<wasm::WasmImport> Imports,
       break;
     case wasm::WASM_EXTERNAL_TABLE:
       W->OS << char(Import.Table.ElemType);
-      encodeULEB128(0, W->OS);           // flags
+      encodeULEB128(Import.Table.Limits.Flags, W->OS);
       encodeULEB128(NumElements, W->OS); // initial
       break;
     case wasm::WASM_EXTERNAL_TAG:
diff --git a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
index 8e2063121e00b..f5bc584ac4e1a 100644
--- a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
+++ b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
@@ -178,14 +178,15 @@ static wasm::WasmLimits DefaultLimits() {
 }
 
 static MCSymbolWasm *GetOrCreateFunctionTableSymbol(MCContext &Ctx,
-                                                    const StringRef &Name) {
+                                                    const StringRef &Name,
+                                                    bool is64) {
   MCSymbolWasm *Sym = cast_or_null<MCSymbolWasm>(Ctx.lookupSymbol(Name));
   if (Sym) {
     if (!Sym->isFunctionTable())
       Ctx.reportError(SMLoc(), "symbol is not a wasm funcref table");
   } else {
     Sym = cast<MCSymbolWasm>(Ctx.getOrCreateSymbol(Name));
-    Sym->setFunctionTable();
+    Sym->setFunctionTable(is64);
     // The default function table is synthesized by the linker.
     Sym->setUndefined();
   }
@@ -258,7 +259,7 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
     MCAsmParserExtension::Initialize(Parser);
 
     DefaultFunctionTable = GetOrCreateFunctionTableSymbol(
-        getContext(), "__indirect_function_table");
+        getContext(), "__indirect_function_table", is64);
     if (!STI->checkFeatures("+reference-types"))
       DefaultFunctionTable->setOmitFromLinkingSection();
   }
@@ -508,7 +509,7 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
       auto &Tok = Lexer.getTok();
       if (Tok.is(AsmToken::Identifier)) {
         auto *Sym =
-            GetOrCreateFunctionTableSymbol(getContext(), Tok.getString());
+            GetOrCreateFunctionTableSymbol(getContext(), Tok.getString(), is64);
         const auto *Val = MCSymbolRefExpr::create(Sym, getContext());
         *Op = std::make_unique<WebAssemblyOperand>(
             WebAssemblyOperand::Symbol, Tok.getLoc(), Tok.getEndLoc(),
@@ -836,6 +837,9 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
       // symbol
       auto WasmSym = cast<MCSymbolWasm>(Ctx.getOrCreateSymbol(SymName));
       WasmSym->setType(wasm::WASM_SYMBOL_TYPE_TABLE);
+      if (is64) {
+        Limits.Flags |= wasm::WASM_LIMITS_FLAG_IS_64;
+      }
       wasm::WasmTableType Type = {*ElemType, Limits};
       WasmSym->setTableType(Type);
       TOut.emitTableType(WasmSym);
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyUtilities.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyUtilities.cpp
index 5e7279808cce6..c5a047ee47d73 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyUtilities.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyUtilities.cpp
@@ -108,8 +108,9 @@ MCSymbolWasm *WebAssembly::getOrCreateFunctionTableSymbol(
     if (!Sym->isFunctionTable())
       Ctx.reportError(SMLoc(), "symbol is not a wasm funcref table");
   } else {
+    bool is64 = Subtarget && Subtarget->getTargetTriple().isArch64Bit();
     Sym = cast<MCSymbolWasm>(Ctx.getOrCreateSymbol(Name));
-    Sym->setFunctionTable();
+    Sym->setFunctionTable(is64);
     // The default function table is synthesized by the linker.
     Sym->setUndefined();
   }
diff --git a/llvm/test/MC/WebAssembly/reloc-pic64.s b/llvm/test/MC/WebAssembly/reloc-pic64.s
index 0f2ebba2a2f33..981d8f0e094cd 100644
--- a/llvm/test/MC/WebAssembly/reloc-pic64.s
+++ b/llvm/test/MC/WebAssembly/reloc-pic64.s
@@ -93,6 +93,7 @@ hidden_func:
 # CHECK-NEXT:           Index:           0
 # CHECK-NEXT:           ElemType:        FUNCREF
 # CHECK-NEXT:           Limits:
+# CHECK-NEXT:             Flags:           [ IS_64 ]
 # CHECK-NEXT:             Minimum:         0x1
 # CHECK-NEXT:       - Module:          GOT.mem
 # CHECK-NEXT:         Field:           default_data

@sbc100
Copy link
Collaborator Author

sbc100 commented Jun 5, 2024

@matthias-blume

@sbc100 sbc100 merged commit c2244f8 into llvm:main Jun 6, 2024
10 checks passed
@sbc100 sbc100 deleted the table64_object_files branch June 6, 2024 03:28
sbc100 added a commit that referenced this pull request Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:WebAssembly mc Machine (object) code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants