From 12b2a8702fc0ba6acdef91ce2a91e7ba2ae684a7 Mon Sep 17 00:00:00 2001 From: Milad Fa Date: Wed, 7 Jul 2021 11:36:26 -0400 Subject: [PATCH] PPC: Fix UIM on disassembler and the simulator A few fixes are applied in this CL: 1- Instructions which use UIM in V8 only use bits 16 to 19 inclusive. 2- get_simd_register is set to return a reference and not a copy. 3- On vector extract and insert instructions, UIM could be used to select specific bytes as starting point which may not reflect a lane. Vector splat uses UIM as a lane selector which remains unchanged in this CL. Change-Id: Ieb43afb977dac11d3ea10a2f265c2823f64457e3 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3011166 Reviewed-by: Junliang Yan Commit-Queue: Milad Fa Cr-Commit-Position: refs/heads/master@{#75618} --- src/diagnostics/ppc/disasm-ppc.cc | 2 +- src/execution/ppc/simulator-ppc.cc | 26 +++++++++++++------------- src/execution/ppc/simulator-ppc.h | 20 +++++++++++++++++++- 3 files changed, 33 insertions(+), 15 deletions(-) diff --git a/src/diagnostics/ppc/disasm-ppc.cc b/src/diagnostics/ppc/disasm-ppc.cc index 2ca5a3cc42..a369704b2b 100644 --- a/src/diagnostics/ppc/disasm-ppc.cc +++ b/src/diagnostics/ppc/disasm-ppc.cc @@ -285,7 +285,7 @@ int Decoder::FormatOption(Instruction* instr, const char* format) { return 3; } case 'U': { // UIM - int32_t value = instr->Bits(20, 16); + uint8_t value = instr->Bits(19, 16); out_buffer_pos_ += base::SNPrintF(out_buffer_ + out_buffer_pos_, "%d", value); return 3; diff --git a/src/execution/ppc/simulator-ppc.cc b/src/execution/ppc/simulator-ppc.cc index cb7c26f013..b9ff88a603 100644 --- a/src/execution/ppc/simulator-ppc.cc +++ b/src/execution/ppc/simulator-ppc.cc @@ -4051,7 +4051,7 @@ void Simulator::ExecuteGeneric(Instruction* instr) { break; } #define VSPLT(type) \ - uint32_t uim = instr->Bits(20, 16); \ + uint8_t uim = instr->Bits(19, 16); \ int vrt = instr->RTValue(); \ int vrb = instr->RBValue(); \ type value = get_simd_register_by_lane(vrb, uim); \ @@ -4094,11 +4094,11 @@ void Simulator::ExecuteGeneric(Instruction* instr) { break; } #undef VSPLTI -#define VINSERT(type, element) \ - uint32_t uim = static_cast(instr->Bits(20, 16)) / sizeof(type); \ - int vrt = instr->RTValue(); \ - int vrb = instr->RBValue(); \ - set_simd_register_by_lane( \ +#define VINSERT(type, element) \ + uint8_t uim = instr->Bits(19, 16); \ + int vrt = instr->RTValue(); \ + int vrb = instr->RBValue(); \ + set_simd_register_bytes( \ vrt, uim, get_simd_register_by_lane(vrb, element)); case VINSERTD: { VINSERT(int64_t, 0) @@ -4117,13 +4117,13 @@ void Simulator::ExecuteGeneric(Instruction* instr) { break; } #undef VINSERT -#define VEXTRACT(type, element) \ - uint32_t uim = static_cast(instr->Bits(20, 16)) / sizeof(type); \ - int vrt = instr->RTValue(); \ - int vrb = instr->RBValue(); \ - type val = get_simd_register_by_lane(vrb, uim); \ - set_simd_register_by_lane(vrt, 0, 0); \ - set_simd_register_by_lane(vrt, 1, 0); \ +#define VEXTRACT(type, element) \ + uint8_t uim = instr->Bits(19, 16); \ + int vrt = instr->RTValue(); \ + int vrb = instr->RBValue(); \ + type val = get_simd_register_bytes(vrb, uim); \ + set_simd_register_by_lane(vrt, 0, 0); \ + set_simd_register_by_lane(vrt, 1, 0); \ set_simd_register_by_lane(vrt, element, val); case VEXTRACTD: { VEXTRACT(uint64_t, 0) diff --git a/src/execution/ppc/simulator-ppc.h b/src/execution/ppc/simulator-ppc.h index 2cf3603323..5a781c63a6 100644 --- a/src/execution/ppc/simulator-ppc.h +++ b/src/execution/ppc/simulator-ppc.h @@ -430,6 +430,16 @@ class Simulator : public SimulatorBase { return (reinterpret_cast(&simd_registers_[reg]))[lane]; } + template + T get_simd_register_bytes(int reg, int byte_from) { + // Byte location is reversed in memory. + int from = kSimd128Size - 1 - (byte_from + sizeof(T) - 1); + void* src = bit_cast(&simd_registers_[reg]) + from; + T dst; + memcpy(&dst, src, sizeof(T)); + return dst; + } + template void set_simd_register_by_lane(int reg, int lane, const T& value, bool force_ibm_lane_numbering = true) { @@ -443,7 +453,15 @@ class Simulator : public SimulatorBase { (reinterpret_cast(&simd_registers_[reg]))[lane] = value; } - simdr_t get_simd_register(int reg) { return simd_registers_[reg]; } + template + void set_simd_register_bytes(int reg, int byte_from, T value) { + // Byte location is reversed in memory. + int from = kSimd128Size - 1 - (byte_from + sizeof(T) - 1); + void* dst = bit_cast(&simd_registers_[reg]) + from; + memcpy(dst, &value, sizeof(T)); + } + + simdr_t& get_simd_register(int reg) { return simd_registers_[reg]; } void set_simd_register(int reg, const simdr_t& value) { simd_registers_[reg] = value;