From a3a024f0e892437a5708f37868e8c19d9aca65c1 Mon Sep 17 00:00:00 2001 From: Clemens Hammacher Date: Tue, 12 Dec 2017 19:16:21 +0100 Subject: [PATCH] Reland^2 "[wasm] [cleanup] Only pass information really needed" This is a reland of fa18e78dc7bcc8fcb51e98f4d3aacb3d08ef5b4c. Mips compile error is fixed. Original change's description: > [wasm] [cleanup] Only pass information really needed > > Instead of always passing the MachineType, we can often just pass the > accessed memory size or the MachineRepresentation, which is less > information to pass and will simplify the upcoming refactoring for > memory operations in Liftoff. > > R=titzer@chromium.org > > Bug: v8:6600 > Change-Id: I8748f8e00dcfdbc4082893143fe88bdafde99053 > Reviewed-on: https://chromium-review.googlesource.com/822194 > Reviewed-by: Ben Titzer > Commit-Queue: Clemens Hammacher > Cr-Commit-Position: refs/heads/master@{#50041} TBR=titzer@chromium.org Bug: v8:6600 Change-Id: I3dff3072d6ceebd74873ace0c7dce7cccc3055d5 Reviewed-on: https://chromium-review.googlesource.com/822851 Reviewed-by: Clemens Hammacher Commit-Queue: Clemens Hammacher Cr-Commit-Position: refs/heads/master@{#50050} --- src/compiler/wasm-compiler.cc | 107 +++++++++++++------------- src/compiler/wasm-compiler.h | 6 +- src/wasm/baseline/liftoff-compiler.cc | 2 +- src/wasm/function-body-decoder-impl.h | 6 +- src/wasm/function-body-decoder.cc | 4 +- src/wasm/wasm-opcodes.h | 2 +- 6 files changed, 64 insertions(+), 63 deletions(-) diff --git a/src/compiler/wasm-compiler.cc b/src/compiler/wasm-compiler.cc index 2a78c420ca..8695790627 100644 --- a/src/compiler/wasm-compiler.cc +++ b/src/compiler/wasm-compiler.cc @@ -1010,9 +1010,8 @@ static bool ReverseBytesSupported(MachineOperatorBuilder* m, return false; } -Node* WasmGraphBuilder::BuildChangeEndiannessStore(Node* node, - MachineType memtype, - wasm::ValueType wasmtype) { +Node* WasmGraphBuilder::BuildChangeEndiannessStore( + Node* node, MachineRepresentation mem_rep, wasm::ValueType wasmtype) { Node* result; Node* value = node; MachineOperatorBuilder* m = jsgraph()->machine(); @@ -1041,23 +1040,22 @@ Node* WasmGraphBuilder::BuildChangeEndiannessStore(Node* node, break; } - if (memtype.representation() == MachineRepresentation::kWord8) { + if (mem_rep == MachineRepresentation::kWord8) { // No need to change endianness for byte size, return original node return node; } - if (wasmtype == wasm::kWasmI64 && - memtype.representation() < MachineRepresentation::kWord64) { + if (wasmtype == wasm::kWasmI64 && mem_rep < MachineRepresentation::kWord64) { // In case we store lower part of WasmI64 expression, we can truncate // upper 32bits value = graph()->NewNode(m->TruncateInt64ToInt32(), value); valueSizeInBytes = 1 << ElementSizeLog2Of(wasm::kWasmI32); valueSizeInBits = 8 * valueSizeInBytes; - if (memtype.representation() == MachineRepresentation::kWord16) { + if (mem_rep == MachineRepresentation::kWord16) { value = graph()->NewNode(m->Word32Shl(), value, jsgraph()->Int32Constant(16)); } } else if (wasmtype == wasm::kWasmI32 && - memtype.representation() == MachineRepresentation::kWord16) { + mem_rep == MachineRepresentation::kWord16) { value = graph()->NewNode(m->Word32Shl(), value, jsgraph()->Int32Constant(16)); } @@ -3485,7 +3483,7 @@ Node* WasmGraphBuilder::SetGlobal(uint32_t index, Node* val) { return node; } -void WasmGraphBuilder::BoundsCheckMem(MachineType memtype, Node* index, +void WasmGraphBuilder::BoundsCheckMem(uint8_t access_size, Node* index, uint32_t offset, wasm::WasmCodePosition position) { if (FLAG_wasm_no_bounds_checks) return; @@ -3499,8 +3497,6 @@ void WasmGraphBuilder::BoundsCheckMem(MachineType memtype, Node* index, : wasm::kV8MaxWasmMemoryPages) * wasm::WasmModule::kPageSize; - byte access_size = wasm::WasmOpcodes::MemSize(memtype); - if (access_size > max_size || offset > max_size - access_size) { // The access will be out of bounds, even for the largest memory. TrapIfEq32(wasm::kTrapMemOutOfBounds, jsgraph()->Int32Constant(0), 0, @@ -3612,7 +3608,8 @@ Node* WasmGraphBuilder::LoadMem(wasm::ValueType type, MachineType memtype, } // Wasm semantics throw on OOB. Introduce explicit bounds check. if (!use_trap_handler()) { - BoundsCheckMem(memtype, index, offset, position); + BoundsCheckMem(wasm::WasmOpcodes::MemSize(memtype), index, offset, + position); } if (memtype.representation() == MachineRepresentation::kWord8 || @@ -3659,7 +3656,7 @@ Node* WasmGraphBuilder::LoadMem(wasm::ValueType type, MachineType memtype, return load; } -Node* WasmGraphBuilder::StoreMem(MachineType memtype, Node* index, +Node* WasmGraphBuilder::StoreMem(MachineRepresentation mem_rep, Node* index, uint32_t offset, uint32_t alignment, Node* val, wasm::WasmCodePosition position, wasm::ValueType type) { @@ -3671,22 +3668,23 @@ Node* WasmGraphBuilder::StoreMem(MachineType memtype, Node* index, } // Wasm semantics throw on OOB. Introduce explicit bounds check. if (!use_trap_handler()) { - BoundsCheckMem(memtype, index, offset, position); + BoundsCheckMem(wasm::WasmOpcodes::MemSize(mem_rep), index, offset, + position); } #if defined(V8_TARGET_BIG_ENDIAN) - val = BuildChangeEndiannessStore(val, memtype, type); + val = BuildChangeEndiannessStore(val, mem_rep, type); #endif - if (memtype.representation() == MachineRepresentation::kWord8 || - jsgraph()->machine()->UnalignedStoreSupported(memtype.representation())) { + if (mem_rep == MachineRepresentation::kWord8 || + jsgraph()->machine()->UnalignedStoreSupported(mem_rep)) { if (use_trap_handler()) { - store = graph()->NewNode( - jsgraph()->machine()->ProtectedStore(memtype.representation()), - MemBuffer(offset), index, val, *effect_, *control_); + store = + graph()->NewNode(jsgraph()->machine()->ProtectedStore(mem_rep), + MemBuffer(offset), index, val, *effect_, *control_); SetSourcePosition(store, position); } else { - StoreRepresentation rep(memtype.representation(), kNoWriteBarrier); + StoreRepresentation rep(mem_rep, kNoWriteBarrier); store = graph()->NewNode(jsgraph()->machine()->Store(rep), MemBuffer(offset), index, val, *effect_, *control_); @@ -3694,7 +3692,7 @@ Node* WasmGraphBuilder::StoreMem(MachineType memtype, Node* index, } else { // TODO(eholk): Support unaligned stores with trap handlers. DCHECK(!use_trap_handler()); - UnalignedStoreRepresentation rep(memtype.representation()); + UnalignedStoreRepresentation rep(mem_rep); store = graph()->NewNode(jsgraph()->machine()->UnalignedStore(rep), MemBuffer(offset), index, val, *effect_, *control_); @@ -3703,8 +3701,7 @@ Node* WasmGraphBuilder::StoreMem(MachineType memtype, Node* index, *effect_ = store; if (FLAG_wasm_trace_memory) { - TraceMemoryOperation(true, memtype.representation(), index, offset, - position); + TraceMemoryOperation(true, mem_rep, index, offset, position); } return store; @@ -4236,47 +4233,51 @@ Node* WasmGraphBuilder::AtomicOp(wasm::WasmOpcode opcode, Node* const* inputs, // TODO(gdeepti): Add alignment validation, traps on misalignment Node* node; switch (opcode) { -#define BUILD_ATOMIC_BINOP(Name, Operation, Type) \ - case wasm::kExpr##Name: { \ - BoundsCheckMem(MachineType::Type(), inputs[0], offset, position); \ - node = graph()->NewNode( \ - jsgraph()->machine()->Atomic##Operation(MachineType::Type()), \ - MemBuffer(offset), inputs[0], inputs[1], *effect_, *control_); \ - break; \ +#define BUILD_ATOMIC_BINOP(Name, Operation, Type) \ + case wasm::kExpr##Name: { \ + BoundsCheckMem(wasm::WasmOpcodes::MemSize(MachineType::Type()), inputs[0], \ + offset, position); \ + node = graph()->NewNode( \ + jsgraph()->machine()->Atomic##Operation(MachineType::Type()), \ + MemBuffer(offset), inputs[0], inputs[1], *effect_, *control_); \ + break; \ } ATOMIC_BINOP_LIST(BUILD_ATOMIC_BINOP) #undef BUILD_ATOMIC_BINOP -#define BUILD_ATOMIC_TERNARY_OP(Name, Operation, Type) \ - case wasm::kExpr##Name: { \ - BoundsCheckMem(MachineType::Type(), inputs[0], offset, position); \ - node = graph()->NewNode( \ - jsgraph()->machine()->Atomic##Operation(MachineType::Type()), \ - MemBuffer(offset), inputs[0], inputs[1], inputs[2], *effect_, \ - *control_); \ - break; \ +#define BUILD_ATOMIC_TERNARY_OP(Name, Operation, Type) \ + case wasm::kExpr##Name: { \ + BoundsCheckMem(wasm::WasmOpcodes::MemSize(MachineType::Type()), inputs[0], \ + offset, position); \ + node = graph()->NewNode( \ + jsgraph()->machine()->Atomic##Operation(MachineType::Type()), \ + MemBuffer(offset), inputs[0], inputs[1], inputs[2], *effect_, \ + *control_); \ + break; \ } ATOMIC_TERNARY_LIST(BUILD_ATOMIC_TERNARY_OP) #undef BUILD_ATOMIC_TERNARY_OP -#define BUILD_ATOMIC_LOAD_OP(Name, Type) \ - case wasm::kExpr##Name: { \ - BoundsCheckMem(MachineType::Type(), inputs[0], offset, position); \ - node = graph()->NewNode( \ - jsgraph()->machine()->AtomicLoad(MachineType::Type()), \ - MemBuffer(offset), inputs[0], *effect_, *control_); \ - break; \ +#define BUILD_ATOMIC_LOAD_OP(Name, Type) \ + case wasm::kExpr##Name: { \ + BoundsCheckMem(wasm::WasmOpcodes::MemSize(MachineType::Type()), inputs[0], \ + offset, position); \ + node = graph()->NewNode( \ + jsgraph()->machine()->AtomicLoad(MachineType::Type()), \ + MemBuffer(offset), inputs[0], *effect_, *control_); \ + break; \ } ATOMIC_LOAD_LIST(BUILD_ATOMIC_LOAD_OP) #undef BUILD_ATOMIC_LOAD_OP -#define BUILD_ATOMIC_STORE_OP(Name, Type, Rep) \ - case wasm::kExpr##Name: { \ - BoundsCheckMem(MachineType::Type(), inputs[0], offset, position); \ - node = graph()->NewNode( \ - jsgraph()->machine()->AtomicStore(MachineRepresentation::Rep), \ - MemBuffer(offset), inputs[0], inputs[1], *effect_, *control_); \ - break; \ +#define BUILD_ATOMIC_STORE_OP(Name, Type, Rep) \ + case wasm::kExpr##Name: { \ + BoundsCheckMem(wasm::WasmOpcodes::MemSize(MachineType::Type()), inputs[0], \ + offset, position); \ + node = graph()->NewNode( \ + jsgraph()->machine()->AtomicStore(MachineRepresentation::Rep), \ + MemBuffer(offset), inputs[0], inputs[1], *effect_, *control_); \ + break; \ } ATOMIC_STORE_LIST(BUILD_ATOMIC_STORE_OP) #undef BUILD_ATOMIC_STORE_OP diff --git a/src/compiler/wasm-compiler.h b/src/compiler/wasm-compiler.h index 915dc31547..a7c6f8e29b 100644 --- a/src/compiler/wasm-compiler.h +++ b/src/compiler/wasm-compiler.h @@ -353,7 +353,7 @@ class WasmGraphBuilder { Node* LoadMem(wasm::ValueType type, MachineType memtype, Node* index, uint32_t offset, uint32_t alignment, wasm::WasmCodePosition position); - Node* StoreMem(MachineType memtype, Node* index, uint32_t offset, + Node* StoreMem(MachineRepresentation mem_rep, Node* index, uint32_t offset, uint32_t alignment, Node* val, wasm::WasmCodePosition position, wasm::ValueType type); static void PrintDebugName(Node* node); @@ -457,11 +457,11 @@ class WasmGraphBuilder { Node* String(const char* string); Node* MemBuffer(uint32_t offset); - void BoundsCheckMem(MachineType memtype, Node* index, uint32_t offset, + void BoundsCheckMem(uint8_t access_size, Node* index, uint32_t offset, wasm::WasmCodePosition position); const Operator* GetSafeLoadOperator(int offset, wasm::ValueType type); const Operator* GetSafeStoreOperator(int offset, wasm::ValueType type); - Node* BuildChangeEndiannessStore(Node* node, MachineType type, + Node* BuildChangeEndiannessStore(Node* node, MachineRepresentation rep, wasm::ValueType wasmtype = wasm::kWasmStmt); Node* BuildChangeEndiannessLoad(Node* node, MachineType type, wasm::ValueType wasmtype = wasm::kWasmStmt); diff --git a/src/wasm/baseline/liftoff-compiler.cc b/src/wasm/baseline/liftoff-compiler.cc index 4d36034d8f..b7012a249f 100644 --- a/src/wasm/baseline/liftoff-compiler.cc +++ b/src/wasm/baseline/liftoff-compiler.cc @@ -528,7 +528,7 @@ class LiftoffCompiler { Value* result) { unsupported(decoder, "memory load"); } - void StoreMem(Decoder* decoder, ValueType type, MachineType mem_type, + void StoreMem(Decoder* decoder, ValueType type, MachineRepresentation mem_rep, const MemoryAccessOperand& operand, const Value& index, const Value& value) { unsupported(decoder, "memory store"); diff --git a/src/wasm/function-body-decoder-impl.h b/src/wasm/function-body-decoder-impl.h index 71f87b00dc..d861cf739a 100644 --- a/src/wasm/function-body-decoder-impl.h +++ b/src/wasm/function-body-decoder-impl.h @@ -581,7 +581,7 @@ struct ControlWithNamedConstructors : public ControlBase { F(LoadMem, ValueType type, MachineType mem_type, \ const MemoryAccessOperand& operand, const Value& index, \ Value* result) \ - F(StoreMem, ValueType type, MachineType mem_type, \ + F(StoreMem, ValueType type, MachineRepresentation mem_rep, \ const MemoryAccessOperand& operand, const Value& index, \ const Value& value) \ F(CurrentMemoryPages, Value* result) \ @@ -1997,8 +1997,8 @@ class WasmFullDecoder : public WasmDecoder { ElementSizeLog2Of(mem_type.representation())); auto value = Pop(1, type); auto index = Pop(0, kWasmI32); - CALL_INTERFACE_IF_REACHABLE(StoreMem, type, mem_type, operand, index, - value); + CALL_INTERFACE_IF_REACHABLE(StoreMem, type, mem_type.representation(), + operand, index, value); return operand.length; } diff --git a/src/wasm/function-body-decoder.cc b/src/wasm/function-body-decoder.cc index 08f49b353f..1c6a09a787 100644 --- a/src/wasm/function-body-decoder.cc +++ b/src/wasm/function-body-decoder.cc @@ -347,10 +347,10 @@ class WasmGraphBuildingInterface { operand.alignment, decoder->position()); } - void StoreMem(Decoder* decoder, ValueType type, MachineType mem_type, + void StoreMem(Decoder* decoder, ValueType type, MachineRepresentation mem_rep, const MemoryAccessOperand& operand, const Value& index, const Value& value) { - BUILD(StoreMem, mem_type, index.node, operand.offset, operand.alignment, + BUILD(StoreMem, mem_rep, index.node, operand.offset, operand.alignment, value.node, decoder->position(), type); } diff --git a/src/wasm/wasm-opcodes.h b/src/wasm/wasm-opcodes.h index d8239d96b8..c0f56fd0df 100644 --- a/src/wasm/wasm-opcodes.h +++ b/src/wasm/wasm-opcodes.h @@ -551,7 +551,7 @@ class V8_EXPORT_PRIVATE WasmOpcodes { static const char* TrapReasonMessage(TrapReason reason); static byte MemSize(MachineType type) { - return 1 << ElementSizeLog2Of(type.representation()); + return MemSize(type.representation()); } static byte MemSize(ValueType type) { return 1 << ElementSizeLog2Of(type); }