[WebAssembly] Use 64-bit table when targeting wasm64

[sbc100]

See WebAssembly/memory64#51

[sbc100]

I'm planning on writing a table64 lowering pass in binaryen before landing this.

[llvmbot]

@llvm/pr-subscribers-backend-webassembly
@llvm/pr-subscribers-lld

@llvm/pr-subscribers-lld-wasm

Author: Sam Clegg (sbc100)

Changes

See WebAssembly/memory64#51

---

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

3 Files Affected:

• (modified) lld/test/wasm/shared64.s (+1)
• (modified) lld/wasm/Writer.cpp (+2)
• (modified) llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp (-12)

diff --git a/lld/test/wasm/shared64.s b/lld/test/wasm/shared64.s
index 3401faed8610c..2148d464f4158 100644
--- a/lld/test/wasm/shared64.s
+++ b/lld/test/wasm/shared64.s
@@ -154,6 +154,7 @@ get_local_func_address:
 # CHECK-NEXT:           Index:           0
 # CHECK-NEXT:           ElemType:        FUNCREF
 # CHECK-NEXT:           Limits:
+# CHECK-NEXT:             Flags:         [ IS_64 ]
 # CHECK-NEXT:             Minimum:         0x2
 # CHECK-NEXT:       - Module:          env
 # CHECK-NEXT:         Field:           __stack_pointer
diff --git a/lld/wasm/Writer.cpp b/lld/wasm/Writer.cpp
index 55eff995fb8a1..91d1113df6a3c 100644
--- a/lld/wasm/Writer.cpp
+++ b/lld/wasm/Writer.cpp
@@ -939,6 +939,8 @@ static void finalizeIndirectFunctionTable() {
     limits.Flags |= WASM_LIMITS_FLAG_HAS_MAX;
     limits.Maximum = limits.Minimum;
   }
+  if (config->is64.value_or(false))
+    limits.Flags |= WASM_LIMITS_FLAG_IS_64;
   WasmSym::indirectFunctionTable->setLimits(limits);
 }
 
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
index 1c62290704fe4..26e13948bc9a6 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
@@ -885,18 +885,6 @@ bool WebAssemblyFastISel::selectCall(const Instruction *I) {
       Table->setNoStrip();
       MIB.addImm(0);
     }
-    // See if we must truncate the function pointer.
-    // CALL_INDIRECT takes an i32, but in wasm64 we represent function pointers
-    // as 64-bit for uniformity with other pointer types.
-    // See also: WebAssemblyISelLowering.cpp: LowerCallResults
-    if (Subtarget->hasAddr64()) {
-      auto Wrap = BuildMI(*FuncInfo.MBB, std::prev(FuncInfo.InsertPt), MIMD,
-                          TII.get(WebAssembly::I32_WRAP_I64));
-      Register Reg32 = createResultReg(&WebAssembly::I32RegClass);
-      Wrap.addReg(Reg32, RegState::Define);
-      Wrap.addReg(CalleeReg);
-      CalleeReg = Reg32;
-    }
   }
 
   for (unsigned ArgReg : Args)

[aheejin]

If we don't do i32.wrap_i64 for table index anymore, can our call_indirect take an i64?

[sbc100]

If we don't do i32.wrap_i64 for table index anymore, can our call_indirect take an i64?

Yes with a 64-bit table all indexes must be 64-bit too, including those passed to call_indirect

[aheejin]

It still only takes a table32_op operand.. Am I missing something?

llvm-project/llvm/lib/Target/WebAssembly/WebAssemblyInstrCall.td

Lines 61 to 67 in 23f8fac

	defm CALL_INDIRECT :
	I<(outs),
	(ins TypeIndex:$type, table32_op:$table, variable_ops),
	(outs),
	(ins TypeIndex:$type, table32_op:$table),
	[],
	"call_indirect", "call_indirect\t$type, $table", 0x11>;

[sbc100]

It still only takes a table32_op operand.. Am I missing something?

llvm-project/llvm/lib/Target/WebAssembly/WebAssemblyInstrCall.td

Lines 61 to 67 in 23f8fac

	defm CALL_INDIRECT :
	I<(outs),
	(ins TypeIndex:$type, table32_op:$table, variable_ops),
	(outs),
	(ins TypeIndex:$type, table32_op:$table),
	[],
	"call_indirect", "call_indirect\t$type, $table", 0x11>;

I guess that code might need updating too.

[sbc100]

It still only takes a table32_op operand.. Am I missing something?

llvm-project/llvm/lib/Target/WebAssembly/WebAssemblyInstrCall.td

Lines 61 to 67 in 23f8fac

	defm CALL_INDIRECT :
	I<(outs),
	(ins TypeIndex:$type, table32_op:$table, variable_ops),
	(outs),
	(ins TypeIndex:$type, table32_op:$table),
	[],
	"call_indirect", "call_indirect\t$type, $table", 0x11>;

I guess that code might need updating too.

Actually isn't table32_op here referring to the table immediate? i.e. the index of the table itself, not the index into the table which is taken as an argument? That fact that its called $table (and that the tests pass) makes me think this.

[aheejin]

Ah you're right I'm confused. Then I think we should fix the non-isel path too:

llvm-project/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp

Lines 579 to 591 in f608ac2

	// See if we must truncate the function pointer.
	// CALL_INDIRECT takes an i32, but in wasm64 we represent function pointers
	// as 64-bit for uniformity with other pointer types.
	// See also: WebAssemblyFastISel::selectCall
	if (IsIndirect && MF.getSubtarget<WebAssemblySubtarget>().hasAddr64()) {
	Register Reg32 =
	MF.getRegInfo().createVirtualRegister(&WebAssembly::I32RegClass);
	auto &FnPtr = CallParams.getOperand(0);
	BuildMI(*BB, CallResults.getIterator(), DL,
	TII.get(WebAssembly::I32_WRAP_I64), Reg32)
	.addReg(FnPtr.getReg());
	FnPtr.setReg(Reg32);
	}

Also there will be tests that need fixing.. such as https://github.com/llvm/llvm-project/blob/main/llvm/test/CodeGen/WebAssembly/fast-isel-call-indirect64.ll

Also the change needs to be reflected in the type checker too (not necessarily in this PR):

llvm-project/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.cpp

Lines 378 to 380 in cf8b93d

	// Function value.
	if (popType(ErrorLoc, wasm::ValType::I32))
	return true;

[sbc100]

See WebAssembly/binaryen#6595

[sbc100]

Updated (and removed) tests.

Updated ISelLowering.

[sbc100]

Now waiting on emscripten-core/emscripten#21950 to land