Skip to content

Commit

Permalink
Bug 1821061 - wasm: Fix code generation of downcasts on x86/arm32. r=…
Browse files Browse the repository at this point in the history
…jseward

BaseCompiler::loadTypeDef assumes that it can use InstanceReg, but that
is only true on non x86/arm32.

This commit removes loadWasmGlobalPtr, as it's just a thin wrapper around
loadPtr that obscures more than helps. Callers of that now just specify
the register that contains the instance directly. This demystifies what's
going on. We can then fix baseline to use a register it has allocated.

Differential Revision: https://phabricator.services.mozilla.com/D172026
  • Loading branch information
eqrion committed Apr 20, 2023
1 parent eadab49 commit 3ecfbf7
Showing 8 changed files with 66 additions and 41 deletions.
4 changes: 2 additions & 2 deletions js/src/jit/Lowering.cpp
Original file line number Diff line number Diff line change
@@ -5449,7 +5449,7 @@ void LIRGenerator::visitWasmAlignmentCheck(MWasmAlignmentCheck* ins) {
}

void LIRGenerator::visitWasmLoadGlobalVar(MWasmLoadGlobalVar* ins) {
size_t offs = wasm::Instance::offsetOfGlobalArea() + ins->globalDataOffset();
size_t offs = wasm::Instance::offsetInGlobalArea(ins->globalDataOffset());
if (ins->type() == MIRType::Int64) {
#ifdef JS_PUNBOX64
LAllocation instance = useRegisterAtStart(ins->instance());
@@ -5497,7 +5497,7 @@ void LIRGenerator::visitWasmLoadTableElement(MWasmLoadTableElement* ins) {

void LIRGenerator::visitWasmStoreGlobalVar(MWasmStoreGlobalVar* ins) {
MDefinition* value = ins->value();
size_t offs = wasm::Instance::offsetOfGlobalArea() + ins->globalDataOffset();
size_t offs = wasm::Instance::offsetInGlobalArea(ins->globalDataOffset());
if (value->type() == MIRType::Int64) {
#ifdef JS_PUNBOX64
LAllocation instance = useRegisterAtStart(ins->instance());
43 changes: 24 additions & 19 deletions js/src/jit/MacroAssembler.cpp
Original file line number Diff line number Diff line change
@@ -4472,39 +4472,38 @@ std::pair<CodeOffset, uint32_t> MacroAssembler::wasmReserveStackChecked(
return std::pair<CodeOffset, uint32_t>(trapInsnOffset, amount);
}

void MacroAssembler::loadWasmGlobalPtr(uint32_t globalDataOffset,
Register dest) {
loadPtr(Address(InstanceReg,
wasm::Instance::offsetOfGlobalArea() + globalDataOffset),
dest);
}

CodeOffset MacroAssembler::wasmCallImport(const wasm::CallSiteDesc& desc,
const wasm::CalleeDesc& callee) {
storePtr(InstanceReg,
Address(getStackPointer(), WasmCallerInstanceOffsetBeforeCall));

// Load the callee, before the caller's registers are clobbered.
uint32_t globalDataOffset = callee.importGlobalDataOffset();
loadWasmGlobalPtr(
globalDataOffset + offsetof(wasm::FuncImportInstanceData, code),
loadPtr(
Address(InstanceReg, wasm::Instance::offsetInGlobalArea(
globalDataOffset +
offsetof(wasm::FuncImportInstanceData, code))),
ABINonArgReg0);

#if !defined(JS_CODEGEN_NONE) && !defined(JS_CODEGEN_WASM32)
static_assert(ABINonArgReg0 != InstanceReg, "by constraint");
#endif

// Switch to the callee's realm.
loadWasmGlobalPtr(
globalDataOffset + offsetof(wasm::FuncImportInstanceData, realm),
loadPtr(
Address(InstanceReg, wasm::Instance::offsetInGlobalArea(
globalDataOffset +
offsetof(wasm::FuncImportInstanceData, realm))),
ABINonArgReg1);
loadPtr(Address(InstanceReg, wasm::Instance::offsetOfCx()), ABINonArgReg2);
storePtr(ABINonArgReg1, Address(ABINonArgReg2, JSContext::offsetOfRealm()));

// Switch to the callee's instance and pinned registers and make the call.
loadWasmGlobalPtr(
globalDataOffset + offsetof(wasm::FuncImportInstanceData, instance),
InstanceReg);
loadPtr(Address(InstanceReg,
wasm::Instance::offsetInGlobalArea(
globalDataOffset +
offsetof(wasm::FuncImportInstanceData, instance))),
InstanceReg);

storePtr(InstanceReg,
Address(getStackPointer(), WasmCalleeInstanceOffsetBeforeCall));
@@ -4576,7 +4575,9 @@ CodeOffset MacroAssembler::asmCallIndirect(const wasm::CallSiteDesc& desc,

// asm.js tables require no signature check, and have had their index
// masked into range and thus need no bounds check.
loadWasmGlobalPtr(callee.tableFunctionBaseGlobalDataOffset(), scratch);
loadPtr(Address(InstanceReg, wasm::Instance::offsetInGlobalArea(
callee.tableFunctionBaseGlobalDataOffset())),
scratch);
if (sizeof(wasm::FunctionTableElem) == 8) {
computeEffectiveAddress(BaseIndex(scratch, index, TimesEight), scratch);
} else {
@@ -4631,8 +4632,8 @@ void MacroAssembler::wasmCallIndirect(const wasm::CallSiteDesc& desc,
boundsCheckFailedLabel);
} else {
branch32(Assembler::Condition::BelowOrEqual,
Address(InstanceReg, wasm::Instance::offsetOfGlobalArea() +
callee.tableLengthGlobalDataOffset()),
Address(InstanceReg, wasm::Instance::offsetInGlobalArea(
callee.tableLengthGlobalDataOffset())),
index, boundsCheckFailedLabel);
}
}
@@ -4642,7 +4643,9 @@ void MacroAssembler::wasmCallIndirect(const wasm::CallSiteDesc& desc,
const wasm::CallIndirectId callIndirectId = callee.wasmTableSigId();
switch (callIndirectId.kind()) {
case wasm::CallIndirectIdKind::Global:
loadWasmGlobalPtr(callIndirectId.globalDataOffset(), WasmTableCallSigReg);
loadPtr(Address(InstanceReg, wasm::Instance::offsetInGlobalArea(
callIndirectId.globalDataOffset())),
WasmTableCallSigReg);
break;
case wasm::CallIndirectIdKind::Immediate:
move32(Imm32(callIndirectId.immediate()), WasmTableCallSigReg);
@@ -4655,7 +4658,9 @@ void MacroAssembler::wasmCallIndirect(const wasm::CallSiteDesc& desc,
// Load the base pointer of the table and compute the address of the callee in
// the table.

loadWasmGlobalPtr(callee.tableFunctionBaseGlobalDataOffset(), calleeScratch);
loadPtr(Address(InstanceReg, wasm::Instance::offsetInGlobalArea(
callee.tableFunctionBaseGlobalDataOffset())),
calleeScratch);
shiftIndex32AndAdd(index, shift, calleeScratch);

// Load the callee instance and decide whether to take the fast path or the
2 changes: 0 additions & 2 deletions js/src/jit/MacroAssembler.h
Original file line number Diff line number Diff line change
@@ -3788,8 +3788,6 @@ class MacroAssembler : public MacroAssemblerSpecific {
Label* rejoin)
DEFINED_ON(arm, arm64, x86_shared, mips_shared, loong64, riscv64, wasm32);

void loadWasmGlobalPtr(uint32_t globalDataOffset, Register dest);

// This function takes care of loading the callee's instance and pinned regs
// but it is the caller's responsibility to save/restore instance or pinned
// regs.
34 changes: 24 additions & 10 deletions js/src/wasm/WasmBaselineCompile.cpp
Original file line number Diff line number Diff line change
@@ -1673,7 +1673,7 @@ bool BaseCompiler::throwFrom(RegRef exn) {

void BaseCompiler::loadTag(RegPtr instance, uint32_t tagIndex, RegRef tagDst) {
const TagDesc& tagDesc = moduleEnv_.tags[tagIndex];
size_t offset = Instance::offsetOfGlobalArea() + tagDesc.globalDataOffset;
size_t offset = Instance::offsetInGlobalArea(tagDesc.globalDataOffset);
masm.loadPtr(Address(instance, offset), tagDst);
}

@@ -2188,7 +2188,7 @@ void BaseCompiler::convertI64ToF64(RegI64 src, bool isUnsigned, RegF64 dest,

Address BaseCompiler::addressOfGlobalVar(const GlobalDesc& global, RegPtr tmp) {
uint32_t globalToInstanceOffset =
Instance::offsetOfGlobalArea() + global.offset();
Instance::offsetInGlobalArea(global.offset());
#ifdef RABALDR_PIN_INSTANCE
movePtr(RegPtr(InstanceReg), tmp);
#else
@@ -2208,8 +2208,8 @@ Address BaseCompiler::addressOfGlobalVar(const GlobalDesc& global, RegPtr tmp) {
Address BaseCompiler::addressOfTableField(const TableDesc& table,
uint32_t fieldOffset,
RegPtr instance) {
uint32_t tableToInstanceOffset = wasm::Instance::offsetOfGlobalArea() +
table.globalDataOffset + fieldOffset;
uint32_t tableToInstanceOffset =
wasm::Instance::offsetInGlobalArea(table.globalDataOffset + fieldOffset);
return Address(instance, tableToInstanceOffset);
}

@@ -6366,23 +6366,37 @@ void BaseCompiler::emitBarrieredClear(RegPtr valueAddr) {

RegPtr BaseCompiler::loadTypeDefInstanceData(uint32_t typeIndex) {
RegPtr rp = needPtr();
RegPtr instance;
# ifndef RABALDR_PIN_INSTANCE
fr.loadInstancePtr(InstanceReg);
instance = rp;
fr.loadInstancePtr(instance);
# else
// We can use the pinned instance register.
instance = RegPtr(InstanceReg);
# endif
masm.computeEffectiveAddress(
Address(InstanceReg,
Instance::offsetOfGlobalArea() +
moduleEnv_.offsetOfTypeDefInstanceData(typeIndex)),
Address(instance,
Instance::offsetInGlobalArea(
moduleEnv_.offsetOfTypeDefInstanceData(typeIndex))),
rp);
return rp;
}

RegPtr BaseCompiler::loadSuperTypeVector(uint32_t typeIndex) {
RegPtr rp = needPtr();
RegPtr instance;
# ifndef RABALDR_PIN_INSTANCE
fr.loadInstancePtr(InstanceReg);
// We need to load the instance register, but can use the destination
// register as a temporary.
instance = rp;
fr.loadInstancePtr(rp);
# else
// We can use the pinned instance register.
instance = RegPtr(InstanceReg);
# endif
masm.loadWasmGlobalPtr(moduleEnv_.offsetOfSuperTypeVector(typeIndex), rp);
masm.loadPtr(Address(instance, Instance::offsetInGlobalArea(
moduleEnv_.offsetOfSuperTypeVector(typeIndex))),
rp);
return rp;
}

5 changes: 4 additions & 1 deletion js/src/wasm/WasmFrameIter.cpp
Original file line number Diff line number Diff line change
@@ -699,7 +699,10 @@ void wasm::GenerateFunctionPrologue(MacroAssembler& masm,
switch (callIndirectId.kind()) {
case CallIndirectIdKind::Global: {
Register scratch = WasmTableCallScratchReg0;
masm.loadWasmGlobalPtr(callIndirectId.globalDataOffset(), scratch);
masm.loadPtr(
Address(InstanceReg, Instance::offsetInGlobalArea(
callIndirectId.globalDataOffset())),
scratch);
masm.branchPtr(Assembler::Condition::Equal, WasmTableCallSigReg,
scratch, &functionBody);
masm.wasmTrap(Trap::IndirectCallBadSig, BytecodeOffset(0));
3 changes: 3 additions & 0 deletions js/src/wasm/WasmInstance.h
Original file line number Diff line number Diff line change
@@ -293,6 +293,9 @@ class alignas(16) Instance {
static constexpr size_t offsetOfGlobalArea() {
return offsetof(Instance, globalArea_);
}
static constexpr size_t offsetInGlobalArea(size_t offset) {
return offsetOfGlobalArea() + offset;
}

JSContext* cx() const { return cx_; }
void* debugTrapHandler() const { return debugTrapHandler_; }
10 changes: 5 additions & 5 deletions js/src/wasm/WasmIonCompile.cpp
Original file line number Diff line number Diff line change
@@ -1697,7 +1697,7 @@ class FunctionCompiler {
// Compute the address of the ref-typed global
auto* valueAddr = MWasmDerivedPointer::New(
alloc(), instancePointer_,
wasm::Instance::offsetOfGlobalArea() + globalDataOffset);
wasm::Instance::offsetInGlobalArea(globalDataOffset));
curBlock_->add(valueAddr);

// Load the previous value for the post-write barrier
@@ -1724,8 +1724,8 @@ class FunctionCompiler {

MDefinition* loadTableField(const TableDesc& table, unsigned fieldOffset,
MIRType type) {
uint32_t globalDataOffset = wasm::Instance::offsetOfGlobalArea() +
table.globalDataOffset + fieldOffset;
uint32_t globalDataOffset = wasm::Instance::offsetInGlobalArea(
table.globalDataOffset + fieldOffset);
auto* load =
MWasmLoadInstance::New(alloc(), instancePointer_, globalDataOffset,
type, AliasSet::Load(AliasSet::WasmTableMeta));
@@ -3706,8 +3706,8 @@ class FunctionCompiler {
}

[[nodiscard]] MDefinition* loadTypeDefInstanceData(uint32_t typeIndex) {
size_t offset = Instance::offsetOfGlobalArea() +
moduleEnv_.offsetOfTypeDefInstanceData(typeIndex);
size_t offset = Instance::offsetInGlobalArea(
moduleEnv_.offsetOfTypeDefInstanceData(typeIndex));
auto* result = MWasmDerivedPointer::New(alloc(), instancePointer_, offset);
if (!result) {
return nullptr;
6 changes: 4 additions & 2 deletions js/src/wasm/WasmStubs.cpp
Original file line number Diff line number Diff line change
@@ -2286,8 +2286,10 @@ static bool GenerateImportJitExit(MacroAssembler& masm, const FuncImport& fi,

// 2.1. Get the callee. This must be a JSFunction if we're using this JIT
// exit.
masm.loadWasmGlobalPtr(
fi.instanceOffset() + offsetof(FuncImportInstanceData, callable), callee);
masm.loadPtr(Address(InstanceReg,
Instance::offsetInGlobalArea(
fi.instanceOffset() +
offsetof(FuncImportInstanceData, callable))), callee);

// 2.2. Save callee.
masm.storePtr(callee, Address(masm.getStackPointer(), calleeArgOffset));

0 comments on commit 3ecfbf7

Please sign in to comment.