Commit 52892c1a authored by Bill Budge's avatar Bill Budge Committed by Commit Bot

Revert "[wasm][memory64] Decode memory offset as 64-bit LEB"

This reverts commit 44efa00b.

Reason for revert: Breaks MSVC with warning as error:
https://ci.chromium.org/p/v8/builders/ci/V8%20Win64%20-%20msvc/15903

Original change's description:
> [wasm][memory64] Decode memory offset as 64-bit LEB
>
> After preparing Liftoff, TurboFan, and the interpreter for this change,
> we now store the memory offset as uint64_t. {LoadLane} and {StoreLane}
> were added after the TurboFan refactoring, so those two are adapted
> similar to the other memory operations.
>
> R=​manoskouk@chromium.org
>
> Bug: v8:10949
> Change-Id: Iba66ce448904e23b152fcb8612d171124e615473
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2555006
> Commit-Queue: Clemens Backes <clemensb@chromium.org>
> Reviewed-by: Manos Koukoutos <manoskouk@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#71382}

TBR=clemensb@chromium.org,manoskouk@chromium.org

Change-Id: Ia0f46a0b6fd2102a61c7664d7cdd86a2cf8ddb24
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:10949
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2558752Reviewed-by: 's avatarBill Budge <bbudge@chromium.org>
Commit-Queue: Bill Budge <bbudge@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71383}
parent 44efa00b
......@@ -4082,7 +4082,7 @@ Node* WasmGraphBuilder::LoadTransformBigEndian(
#endif
Node* WasmGraphBuilder::LoadLane(MachineType memtype, Node* value, Node* index,
uint64_t offset, uint8_t laneidx,
uint32_t offset, uint8_t laneidx,
wasm::WasmCodePosition position) {
has_simd_ = true;
Node* load;
......@@ -4093,12 +4093,9 @@ Node* WasmGraphBuilder::LoadLane(MachineType memtype, Node* value, Node* index,
MemoryAccessKind load_kind =
GetMemoryAccessKind(mcgraph(), memtype, use_trap_handler());
// {offset} is validated to be within uintptr_t range in {BoundsCheckMem}.
uintptr_t capped_offset = static_cast<uintptr_t>(offset);
load = SetEffect(graph()->NewNode(
mcgraph()->machine()->LoadLane(load_kind, memtype, laneidx),
MemBuffer(capped_offset), index, value, effect(), control()));
MemBuffer(offset), index, value, effect(), control()));
if (load_kind == MemoryAccessKind::kProtected) {
SetSourcePosition(load, position);
......@@ -4224,7 +4221,7 @@ Node* WasmGraphBuilder::LoadMem(wasm::ValueType type, MachineType memtype,
}
Node* WasmGraphBuilder::StoreLane(MachineRepresentation mem_rep, Node* index,
uint64_t offset, uint32_t alignment,
uint32_t offset, uint32_t alignment,
Node* val, uint8_t laneidx,
wasm::WasmCodePosition position,
wasm::ValueType type) {
......
......@@ -316,12 +316,12 @@ class WasmGraphBuilder {
wasm::LoadTransformationKind transform, Node* index,
uint64_t offset, uint32_t alignment,
wasm::WasmCodePosition position);
Node* LoadLane(MachineType memtype, Node* value, Node* index, uint64_t offset,
Node* LoadLane(MachineType memtype, Node* value, Node* index, uint32_t offset,
uint8_t laneidx, wasm::WasmCodePosition position);
Node* StoreMem(MachineRepresentation mem_rep, Node* index, uint64_t offset,
uint32_t alignment, Node* val, wasm::WasmCodePosition position,
wasm::ValueType type);
Node* StoreLane(MachineRepresentation mem_rep, Node* index, uint64_t offset,
Node* StoreLane(MachineRepresentation mem_rep, Node* index, uint32_t offset,
uint32_t alignment, Node* val, uint8_t laneidx,
wasm::WasmCodePosition position, wasm::ValueType type);
static void PrintDebugName(Node* node);
......
......@@ -689,16 +689,13 @@ class BranchTableIterator {
const uint32_t table_count_; // the count of entries, not including default.
};
template <Decoder::ValidateFlag validate>
class WasmDecoder;
template <Decoder::ValidateFlag validate>
struct MemoryAccessImmediate {
uint32_t alignment;
uint64_t offset;
uint32_t offset;
uint32_t length = 0;
inline MemoryAccessImmediate(Decoder* decoder, const byte* pc,
uint32_t max_alignment, bool is_memory64) {
uint32_t max_alignment) {
uint32_t alignment_length;
alignment =
decoder->read_u32v<validate>(pc, &alignment_length, "alignment");
......@@ -710,15 +707,10 @@ struct MemoryAccessImmediate {
max_alignment, alignment);
}
uint32_t offset_length;
offset = is_memory64 ? decoder->read_u64v<validate>(
pc + alignment_length, &offset_length, "offset")
: decoder->read_u32v<validate>(
pc + alignment_length, &offset_length, "offset");
offset = decoder->read_u32v<validate>(pc + alignment_length, &offset_length,
"offset");
length = alignment_length + offset_length;
}
// Defined below, after the definition of WasmDecoder.
inline MemoryAccessImmediate(WasmDecoder<validate>* decoder, const byte* pc,
uint32_t max_alignment);
};
// Immediate for SIMD lane operations.
......@@ -1593,11 +1585,6 @@ class WasmDecoder : public Decoder {
// Returns the length of the opcode under {pc}.
static uint32_t OpcodeLength(WasmDecoder* decoder, const byte* pc) {
WasmOpcode opcode = static_cast<WasmOpcode>(*pc);
// We don't have information about the module here, so we just assume that
// memory64 is enabled when parsing memory access immediates. This is
// backwards-compatible; decode errors will be detected at another time when
// actually decoding that opcode.
constexpr bool kConservativelyAssumeMemory64 = true;
switch (opcode) {
/********** Control opcodes **********/
case kExprUnreachable:
......@@ -1715,8 +1702,7 @@ class WasmDecoder : public Decoder {
return 1;
FOREACH_LOAD_MEM_OPCODE(DECLARE_OPCODE_CASE)
FOREACH_STORE_MEM_OPCODE(DECLARE_OPCODE_CASE) {
MemoryAccessImmediate<validate> imm(decoder, pc + 1, UINT32_MAX,
kConservativelyAssumeMemory64);
MemoryAccessImmediate<validate> imm(decoder, pc + 1, UINT32_MAX);
return 1 + imm.length;
}
// clang-format on
......@@ -1795,8 +1781,7 @@ class WasmDecoder : public Decoder {
case kExprPrefetchT:
case kExprPrefetchNT: {
MemoryAccessImmediate<validate> imm(decoder, pc + length,
UINT32_MAX,
kConservativelyAssumeMemory64);
UINT32_MAX);
return length + imm.length;
}
case kExprS128Load8Lane:
......@@ -1808,8 +1793,7 @@ class WasmDecoder : public Decoder {
case kExprS128Store32Lane:
case kExprS128Store64Lane: {
MemoryAccessImmediate<validate> imm(decoder, pc + length,
UINT32_MAX,
kConservativelyAssumeMemory64);
UINT32_MAX);
// 1 more byte for lane index immediate.
return length + imm.length + 1;
}
......@@ -1831,8 +1815,7 @@ class WasmDecoder : public Decoder {
switch (opcode) {
FOREACH_ATOMIC_OPCODE(DECLARE_OPCODE_CASE) {
MemoryAccessImmediate<validate> imm(decoder, pc + length,
UINT32_MAX,
kConservativelyAssumeMemory64);
UINT32_MAX);
return length + imm.length;
}
FOREACH_ATOMIC_0_OPERAND_OPCODE(DECLARE_OPCODE_CASE) {
......@@ -2090,12 +2073,6 @@ class WasmDecoder : public Decoder {
const FunctionSig* sig_;
};
template <Decoder::ValidateFlag validate>
MemoryAccessImmediate<validate>::MemoryAccessImmediate(
WasmDecoder<validate>* decoder, const byte* pc, uint32_t max_alignment)
: MemoryAccessImmediate(decoder, pc, max_alignment,
decoder->module_->is_memory64) {}
#define CALL_INTERFACE(name, ...) interface_.name(this, ##__VA_ARGS__)
#define CALL_INTERFACE_IF_REACHABLE(name, ...) \
do { \
......
......@@ -1534,8 +1534,7 @@ class WasmInterpreterInternals {
// the operation to keep trap reporting and tracing accurate, otherwise
// those will report at the middle of an opcode.
MemoryAccessImmediate<Decoder::kNoValidation> imm(
decoder, code->at(pc + prefix_len), sizeof(ctype),
module()->is_memory64);
decoder, code->at(pc + prefix_len), sizeof(ctype));
uint64_t index = ToMemType(Pop());
Address addr = BoundsCheckMem<mtype>(imm.offset, index);
if (!addr) {
......@@ -1567,8 +1566,7 @@ class WasmInterpreterInternals {
// the operation to keep trap reporting and tracing accurate, otherwise
// those will report at the middle of an opcode.
MemoryAccessImmediate<Decoder::kNoValidation> imm(
decoder, code->at(pc + prefix_len), sizeof(ctype),
module()->is_memory64);
decoder, code->at(pc + prefix_len), sizeof(ctype));
ctype val = Pop().to<ctype>();
uint64_t index = ToMemType(Pop());
......@@ -1595,7 +1593,7 @@ class WasmInterpreterInternals {
Address* address, pc_t pc, int* const len,
type* val = nullptr, type* val2 = nullptr) {
MemoryAccessImmediate<Decoder::kNoValidation> imm(
decoder, code->at(pc + *len), sizeof(type), module()->is_memory64);
decoder, code->at(pc + *len), sizeof(type));
if (val2) *val2 = static_cast<type>(Pop().to<op_type>());
if (val) *val = static_cast<type>(Pop().to<op_type>());
uint64_t index = ToMemType(Pop());
......@@ -1619,7 +1617,7 @@ class WasmInterpreterInternals {
int64_t* timeout = nullptr) {
// TODO(manoskouk): Introduce test which exposes wrong pc offset below.
MemoryAccessImmediate<Decoder::kFullValidation> imm(
decoder, code->at(pc + *len), sizeof(type), module()->is_memory64);
decoder, code->at(pc + *len), sizeof(type));
if (timeout) {
*timeout = Pop().to<int64_t>();
}
......@@ -2808,7 +2806,7 @@ class WasmInterpreterInternals {
case kExprPrefetchNT: {
// Max alignment doesn't matter, use an arbitrary value.
MemoryAccessImmediate<Decoder::kNoValidation> imm(
decoder, code->at(pc + *len), 4, module()->is_memory64);
decoder, code->at(pc + *len), 4);
// Pop address and do nothing.
Pop().to<uint32_t>();
*len += imm.length;
......@@ -2899,7 +2897,7 @@ class WasmInterpreterInternals {
s_type value = Pop().to_s128().to<s_type>();
MemoryAccessImmediate<Decoder::kNoValidation> imm(
decoder, code->at(pc + *len), sizeof(load_type), module()->is_memory64);
decoder, code->at(pc + *len), sizeof(load_type));
SimdLaneImmediate<Decoder::kNoValidation> lane_imm(
decoder, code->at(pc + *len + imm.length));
......
......@@ -67,23 +67,6 @@
static_cast<byte>((((x) >> 21) & MASK_7) | 0x80), \
static_cast<byte>((((x) >> 28) & MASK_7))
#define U64V_1(x) U32V_1(static_cast<uint32_t>(x))
#define U64V_2(x) U32V_2(static_cast<uint32_t>(x))
#define U64V_3(x) U32V_3(static_cast<uint32_t>(x))
#define U64V_4(x) U32V_4(static_cast<uint32_t>(x))
#define U64V_5(x) \
static_cast<uint8_t>((uint64_t{x} & MASK_7) | 0x80), \
static_cast<uint8_t>(((uint64_t{x} >> 7) & MASK_7) | 0x80), \
static_cast<uint8_t>(((uint64_t{x} >> 14) & MASK_7) | 0x80), \
static_cast<uint8_t>(((uint64_t{x} >> 21) & MASK_7) | 0x80), \
static_cast<uint8_t>(((uint64_t{x} >> 28) & MASK_7))
#define U64V_6(x) \
static_cast<uint8_t>((uint64_t{x} & MASK_7) | 0x80), \
static_cast<uint8_t>(((uint64_t{x} >> 7) & MASK_7) | 0x80), \
static_cast<uint8_t>(((uint64_t{x} >> 14) & MASK_7) | 0x80), \
static_cast<uint8_t>(((uint64_t{x} >> 21) & MASK_7) | 0x80), \
static_cast<uint8_t>(((uint64_t{x} >> 28) & MASK_7) | 0x80), \
static_cast<uint8_t>(((uint64_t{x} >> 35) & MASK_7))
#define U64V_10(x) \
static_cast<uint8_t>((uint64_t{x} & MASK_7) | 0x80), \
static_cast<uint8_t>(((uint64_t{x} >> 7) & MASK_7) | 0x80), \
......
......@@ -4945,15 +4945,8 @@ TEST_F(BytecodeIteratorTest, WithLocalDecls) {
* Memory64 tests
******************************************************************************/
class FunctionBodyDecoderTestOnBothMemoryTypes
: public FunctionBodyDecoderTestBase<::testing::TestWithParam<MemoryType>> {
public:
bool is_memory64() const { return GetParam() == kMemory64; }
WasmOpcode index_type_const() const {
return is_memory64() ? kExprI64Const : kExprI32Const;
}
};
using FunctionBodyDecoderTestOnBothMemoryTypes =
FunctionBodyDecoderTestBase<::testing::TestWithParam<MemoryType>>;
std::string PrintMemoryType(::testing::TestParamInfo<MemoryType> info) {
switch (info.param) {
......@@ -4971,38 +4964,17 @@ INSTANTIATE_TEST_SUITE_P(MemoryTypes, FunctionBodyDecoderTestOnBothMemoryTypes,
TEST_P(FunctionBodyDecoderTestOnBothMemoryTypes, IndexTypes) {
builder.InitializeMemory(GetParam());
Validate(!is_memory64(), sigs.i_v(),
const bool is_memory64 = GetParam() == kMemory64;
Validate(!is_memory64, sigs.i_v(),
{WASM_LOAD_MEM(MachineType::Int32(), WASM_ZERO)});
Validate(is_memory64(), sigs.i_v(),
Validate(is_memory64, sigs.i_v(),
{WASM_LOAD_MEM(MachineType::Int32(), WASM_ZERO64)});
Validate(!is_memory64(), sigs.v_v(),
Validate(!is_memory64, sigs.v_v(),
{WASM_STORE_MEM(MachineType::Int32(), WASM_ZERO, WASM_ZERO)});
Validate(is_memory64(), sigs.v_v(),
Validate(is_memory64, sigs.v_v(),
{WASM_STORE_MEM(MachineType::Int32(), WASM_ZERO64, WASM_ZERO)});
}
TEST_P(FunctionBodyDecoderTestOnBothMemoryTypes, 64BitOffset) {
builder.InitializeMemory(GetParam());
// Macro for defining a zero constant of the right type.
#define ZERO_FOR_TYPE WASM_SEQ(index_type_const(), 0)
// Offset is zero encoded in 5 bytes (works always).
Validate(
true, sigs.i_v(),
{WASM_LOAD_MEM_OFFSET(MachineType::Int32(), U64V_5(0), ZERO_FOR_TYPE)});
// Offset is zero encoded in 6 bytes (works only in memory64).
Validate(
is_memory64(), sigs.i_v(),
{WASM_LOAD_MEM_OFFSET(MachineType::Int32(), U64V_6(0), ZERO_FOR_TYPE)});
// Same with store.
Validate(true, sigs.v_v(),
{WASM_STORE_MEM_OFFSET(MachineType::Int32(), U64V_5(0),
ZERO_FOR_TYPE, WASM_ZERO)});
Validate(is_memory64(), sigs.v_v(),
{WASM_STORE_MEM_OFFSET(MachineType::Int32(), U64V_6(0),
ZERO_FOR_TYPE, WASM_ZERO)});
#undef ZERO_FOR_TYPE
}
#undef B1
#undef B2
#undef B3
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment