Skip to content

Commit 96ef4f3

Browse files
sbc100sbc100
committed
Revert "[WebAssembly] call_indirect issues table number relocs"
This reverts commit 418df4a. This change broke emscripten tests, I believe because it started generating 5-byte a wide table index in the call_indirect instruction. Neither v8 nor wabt seem to be able to handle that. The spec currently says that this is single 0x0 byte and: "In future versions of WebAssembly, the zero byte occurring in the encoding of the call_indirectcall_indirect instruction may be used to index additional tables." So we need to revisit this change. For backwards compat I guess we need to guarantee that __indirect_function_table is always at address zero. We could also consider making this a single-byte relocation with and assert if have more than 127 tables (for now). Differential Revision: https://reviews.llvm.org/D95005
1 parent 0cd0eb6 commit 96ef4f3

19 files changed

+89
-212
lines changed

‎lld/test/wasm/call-indirect.ll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,16 +122,16 @@ define void @call_ptr(i64 (i64)* %arg) {
122122
; CHECK-NEXT: Body: 42010B
123123
; CHECK-NEXT: - Index: 1
124124
; CHECK-NEXT: Locals:
125-
; CHECK-NEXT: Body: 41002802808880800011808080800080808080001A41002802848880800011818080800080808080001A0B
125+
; CHECK-NEXT: Body: 410028028088808000118080808000001A410028028488808000118180808000001A0B
126126
; CHECK-NEXT: - Index: 2
127127
; CHECK-NEXT: Locals:
128128
; CHECK-NEXT: Body: 41020B
129129
; CHECK-NEXT: - Index: 3
130130
; CHECK-NEXT: Locals:
131-
; CHECK-NEXT: Body: 41002802888880800011818080800080808080001A0B
131+
; CHECK-NEXT: Body: 410028028888808000118180808000001A0B
132132
; CHECK-NEXT: - Index: 4
133133
; CHECK-NEXT: Locals:
134-
; CHECK-NEXT: Body: 4201200011828080800080808080001A0B
134+
; CHECK-NEXT: Body: 42012000118280808000001A0B
135135
; CHECK-NEXT: - Type: DATA
136136
; CHECK-NEXT: Segments:
137137
; CHECK-NEXT: - SectionOffset: 7

‎lld/test/wasm/compress-relocs.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,5 @@ entry:
2222

2323
; ERROR: wasm-ld: error: --compress-relocations is incompatible with output debug information. Please pass --strip-debug or --strip-all
2424

25-
; CHECK: Body: 41002802808880800011808080800080808080001A41002802848880800011818080800080808080001A0B
25+
; CHECK: Body: 410028028088808000118080808000001A410028028488808000118180808000001A0B
2626
; COMPRESS: Body: 4100280280081100001A4100280284081101001A0B

‎lld/test/wasm/shared.ll

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,17 +84,17 @@ declare void @func_external()
8484
; CHECK-NEXT: GlobalType: I32
8585
; CHECK-NEXT: GlobalMutable: false
8686
; CHECK-NEXT: - Module: env
87+
; CHECK-NEXT: Field: func_external
88+
; CHECK-NEXT: Kind: FUNCTION
89+
; CHECK-NEXT: SigIndex: 1
90+
; CHECK-NEXT: - Module: env
8791
; CHECK-NEXT: Field: __indirect_function_table
8892
; CHECK-NEXT: Kind: TABLE
8993
; CHECK-NEXT: Table:
9094
; CHECK-NEXT: Index: 0
9195
; CHECK-NEXT: ElemType: FUNCREF
9296
; CHECK-NEXT: Limits:
9397
; CHECK-NEXT: Initial: 0x2
94-
; CHECK-NEXT: - Module: env
95-
; CHECK-NEXT: Field: func_external
96-
; CHECK-NEXT: Kind: FUNCTION
97-
; CHECK-NEXT: SigIndex: 1
9898
; CHECK-NEXT: - Module: GOT.mem
9999
; CHECK-NEXT: Field: indirect_func
100100
; CHECK-NEXT: Kind: GLOBAL

‎llvm/lib/MC/WasmObjectWriter.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,13 @@ void WasmObjectWriter::writeHeader(const MCAssembler &Asm) {
405405

406406
void WasmObjectWriter::executePostLayoutBinding(MCAssembler &Asm,
407407
const MCAsmLayout &Layout) {
408+
// As a stopgap measure until call_indirect instructions start explicitly
409+
// referencing the indirect function table via TABLE_NUMBER relocs, ensure
410+
// that the indirect function table import makes it to the output if anything
411+
// in the compilation unit has caused it to be present.
412+
if (auto *Sym = Asm.getContext().lookupSymbol("__indirect_function_table"))
413+
Asm.registerSymbol(*Sym);
414+
408415
// Build a map of sections to the function that defines them, for use
409416
// in recordRelocation.
410417
for (const MCSymbol &S : Asm.symbols()) {

‎llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp

Lines changed: 11 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -472,15 +472,6 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
472472
WebAssemblyOperand::IntOp{static_cast<int64_t>(BT)}));
473473
}
474474

475-
void addFunctionTableOperand(OperandVector &Operands, StringRef TableName,
476-
SMLoc StartLoc, SMLoc EndLoc) {
477-
MCSymbolWasm *Sym = GetOrCreateFunctionTableSymbol(getContext(), TableName);
478-
auto *Val = MCSymbolRefExpr::create(Sym, getContext());
479-
Operands.push_back(std::make_unique<WebAssemblyOperand>(
480-
WebAssemblyOperand::Symbol, StartLoc, EndLoc,
481-
WebAssemblyOperand::SymOp{Val}));
482-
}
483-
484475
bool ParseInstruction(ParseInstructionInfo & /*Info*/, StringRef Name,
485476
SMLoc NameLoc, OperandVector &Operands) override {
486477
// Note: Name does NOT point into the sourcecode, but to a local, so
@@ -517,7 +508,6 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
517508
bool ExpectBlockType = false;
518509
bool ExpectFuncType = false;
519510
bool ExpectHeapType = false;
520-
bool ExpectFunctionTable = false;
521511
if (Name == "block") {
522512
push(Block);
523513
ExpectBlockType = true;
@@ -557,7 +547,15 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
557547
return true;
558548
} else if (Name == "call_indirect" || Name == "return_call_indirect") {
559549
ExpectFuncType = true;
560-
ExpectFunctionTable = true;
550+
// Ensure that the object file has a __indirect_function_table import, as
551+
// we call_indirect against it.
552+
auto &Ctx = getStreamer().getContext();
553+
MCSymbolWasm *Sym =
554+
GetOrCreateFunctionTableSymbol(Ctx, "__indirect_function_table");
555+
// Until call_indirect emits TABLE_NUMBER relocs against this symbol, mark
556+
// it as NO_STRIP so as to ensure that the indirect function table makes
557+
// it to linked output.
558+
Sym->setNoStrip();
561559
} else if (Name == "ref.null") {
562560
ExpectHeapType = true;
563561
}
@@ -573,7 +571,7 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
573571
return true;
574572
// Got signature as block type, don't need more
575573
ExpectBlockType = false;
576-
auto &Ctx = getContext();
574+
auto &Ctx = getStreamer().getContext();
577575
// The "true" here will cause this to be a nameless symbol.
578576
MCSymbol *Sym = Ctx.createTempSymbol("typeindex", true);
579577
auto *WasmSym = cast<MCSymbolWasm>(Sym);
@@ -585,16 +583,6 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
585583
Operands.push_back(std::make_unique<WebAssemblyOperand>(
586584
WebAssemblyOperand::Symbol, Loc.getLoc(), Loc.getEndLoc(),
587585
WebAssemblyOperand::SymOp{Expr}));
588-
589-
// Allow additional operands after the signature, notably for
590-
// call_indirect against a named table.
591-
if (Lexer.isNot(AsmToken::EndOfStatement)) {
592-
if (expect(AsmToken::Comma, ","))
593-
return true;
594-
if (Lexer.is(AsmToken::EndOfStatement)) {
595-
return error("Unexpected trailing comma");
596-
}
597-
}
598586
}
599587

600588
while (Lexer.isNot(AsmToken::EndOfStatement)) {
@@ -620,11 +608,8 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
620608
WebAssemblyOperand::Integer, Id.getLoc(), Id.getEndLoc(),
621609
WebAssemblyOperand::IntOp{static_cast<int64_t>(HeapType)}));
622610
Parser.Lex();
623-
} else if (ExpectFunctionTable) {
624-
addFunctionTableOperand(Operands, Id.getString(), Id.getLoc(),
625-
Id.getEndLoc());
626-
Parser.Lex();
627611
} else {
612+
// Assume this identifier is a label.
628613
const MCExpr *Val;
629614
SMLoc End;
630615
if (Parser.parseExpression(Val, End))
@@ -689,11 +674,6 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
689674
// Support blocks with no operands as default to void.
690675
addBlockTypeOperand(Operands, NameLoc, WebAssembly::BlockType::Void);
691676
}
692-
if (ExpectFunctionTable && Operands.size() == 2) {
693-
// If call_indirect doesn't specify a target table, supply one.
694-
addFunctionTableOperand(Operands, "__indirect_function_table", NameLoc,
695-
SMLoc::getFromPointer(Name.end()));
696-
}
697677
Parser.Lex();
698678
return false;
699679
}

‎llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyInstPrinter.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ void WebAssemblyInstPrinter::printInst(const MCInst *MI, uint64_t Address,
6868
for (auto I = Start, E = MI->getNumOperands(); I < E; ++I) {
6969
if (MI->getOpcode() == WebAssembly::CALL_INDIRECT &&
7070
I - Start == NumVariadicDefs) {
71-
// Skip type and table arguments when printing for tests.
71+
// Skip type and flags arguments when printing for tests
7272
++I;
7373
continue;
7474
}

‎llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -401,16 +401,6 @@ inline bool isCallIndirect(unsigned Opc) {
401401
}
402402
}
403403

404-
inline bool isRetCallIndirect(unsigned Opc) {
405-
switch (Opc) {
406-
case WebAssembly::RET_CALL_INDIRECT:
407-
case WebAssembly::RET_CALL_INDIRECT_S:
408-
return true;
409-
default:
410-
return false;
411-
}
412-
}
413-
414404
inline bool isBrTable(const MachineInstr &MI) {
415405
switch (MI.getOpcode()) {
416406
case WebAssembly::BR_TABLE_I32:

‎llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -869,11 +869,18 @@ bool WebAssemblyFastISel::selectCall(const Instruction *I) {
869869
if (IsDirect) {
870870
MIB.addGlobalAddress(Func);
871871
} else {
872-
// Placehoder for the type index.
872+
// Add placeholders for the type index and immediate flags
873873
MIB.addImm(0);
874-
// The table into which this call_indirect indexes.
875-
MIB.addSym(WebAssembly::getOrCreateFunctionTableSymbol(
876-
MF->getMMI().getContext(), "__indirect_function_table"));
874+
MIB.addImm(0);
875+
876+
// Ensure that the object file has a __indirect_function_table import, as we
877+
// call_indirect against it.
878+
MCSymbolWasm *Sym = WebAssembly::getOrCreateFunctionTableSymbol(
879+
MF->getMMI().getContext(), "__indirect_function_table");
880+
// Until call_indirect emits TABLE_NUMBER relocs against this symbol, mark
881+
// it as NO_STRIP so as to ensure that the indirect function table makes it
882+
// to linked output.
883+
Sym->setNoStrip();
877884
}
878885

879886
for (unsigned ArgReg : Args)

‎llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -475,12 +475,19 @@ static MachineBasicBlock *LowerCallResults(MachineInstr &CallResults,
475475
for (auto Def : CallResults.defs())
476476
MIB.add(Def);
477477

478+
// Add placeholders for the type index and immediate flags
478479
if (IsIndirect) {
479-
// Placehoder for the type index.
480480
MIB.addImm(0);
481-
// The table into which this call_indirect indexes.
482-
MIB.addSym(WebAssembly::getOrCreateFunctionTableSymbol(
483-
MF.getContext(), "__indirect_function_table"));
481+
MIB.addImm(0);
482+
483+
// Ensure that the object file has a __indirect_function_table import, as we
484+
// call_indirect against it.
485+
MCSymbolWasm *Sym = WebAssembly::getOrCreateFunctionTableSymbol(
486+
MF.getContext(), "__indirect_function_table");
487+
// Until call_indirect emits TABLE_NUMBER relocs against this symbol, mark
488+
// it as NO_STRIP so as to ensure that the indirect function table makes it
489+
// to linked output.
490+
Sym->setNoStrip();
484491
}
485492

486493
for (auto Use : CallParams.uses())

‎llvm/lib/Target/WebAssembly/WebAssemblyInstrCall.td

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,6 @@ defm RET_CALL_RESULTS :
4848
I<(outs), (ins variable_ops), (outs), (ins), [],
4949
"return_call_results", "return_call_results", -1>;
5050

51-
// Note that instructions with variable_ops have custom printers in
52-
// WebAssemblyInstPrinter.cpp.
53-
5451
let variadicOpsAreDefs = 1 in
5552
defm CALL :
5653
I<(outs), (ins function32_op:$callee, variable_ops),
@@ -59,12 +56,9 @@ defm CALL :
5956

6057
let variadicOpsAreDefs = 1 in
6158
defm CALL_INDIRECT :
62-
I<(outs),
63-
(ins TypeIndex:$type, table32_op:$table, variable_ops),
64-
(outs),
65-
(ins TypeIndex:$type, table32_op:$table),
66-
[],
67-
"call_indirect", "call_indirect\t$type, $table", 0x11>;
59+
I<(outs), (ins TypeIndex:$type, i32imm:$flags, variable_ops),
60+
(outs), (ins TypeIndex:$type, i32imm:$flags), [],
61+
"call_indirect", "call_indirect\t$type", 0x11>;
6862

6963
let isReturn = 1, isTerminator = 1, hasCtrlDep = 1, isBarrier = 1 in
7064
defm RET_CALL :
@@ -75,9 +69,9 @@ defm RET_CALL :
7569

7670
let isReturn = 1 in
7771
defm RET_CALL_INDIRECT :
78-
I<(outs), (ins TypeIndex:$type, table32_op:$table, variable_ops),
79-
(outs), (ins TypeIndex:$type, table32_op:$table), [],
80-
"return_call_indirect\t", "return_call_indirect\t$type, $table",
72+
I<(outs), (ins TypeIndex:$type, i32imm:$flags, variable_ops),
73+
(outs), (ins TypeIndex:$type, i32imm:$flags), [],
74+
"return_call_indirect\t", "return_call_indirect\t$type",
8175
0x13>,
8276
Requires<[HasTailCall]>;
8377

0 commit comments

Comments
 (0)