Fix RISC-V vector mask logical and unary operations to respect tail-agnostic policy limits This change fixes a bug introduced in an earlier change where vector mask logical operations (e.g., `vmnand`, `vmand`) and vector unary mask instructions (`vmsbf`, `vmsif`, `vmsof`) were extended to evaluate across the entire physical register block (`physical_bits`) rather than safely bounding execution at the dynamic vector length (`vlen`). According to Section 3.4.3 of the RISC-V V-Extension specification: - "Mask destination tail elements are always treated as tail-agnostic, regardless of the setting of vta." - "When a set is marked agnostic, the corresponding set of destination elements in any vector destination operand can either retain the value they previously held, or are overwritten with 1s." By running the active bitwise logical lambdas (such as `~` for NAND/NOR) over the tail zeroes, the ISS unconditionally modified tail elements. Worse, for operations lacking a bitwise NOT (such as `vmand`), this approach overwrote the tail elements with `0`s, which is an explicit violation of the specification (must be undisturbed or overwritten with `1`s). This commit reverts the `BitwiseMaskBinaryOp` and unary mask instructions to bound processing up to `vlen`, aligning the ISS with the valid "undisturbed" implementation of the tail-agnostic policy. This resolves mismatches against Spike trace outputs and ensures `vmnand.elf_test` and related architectural tests pass successfully. PiperOrigin-RevId: 978109338
diff --git a/riscv/riscv_vector_opm_instructions.cc b/riscv/riscv_vector_opm_instructions.cc index 234a01f..d286e5d 100644 --- a/riscv/riscv_vector_opm_instructions.cc +++ b/riscv/riscv_vector_opm_instructions.cc
@@ -190,6 +190,14 @@ RiscVVectorState* rv_vector, const Instruction* inst, std::function<uint8_t(uint8_t, uint8_t)> op) { if (rv_vector->vector_exception()) return; + int vstart = rv_vector->vstart(); + int vlen = rv_vector->vector_length(); + + if (vstart >= vlen) { + rv_vector->clear_vstart(); + return; + } + // Get spans for vector source and destination registers. auto* vs2_op = static_cast<RV32VectorSourceOperand*>(inst->Source(0)); auto vs2_span = vs2_op->GetRegister(0)->data_buffer()->Get<uint8_t>(); @@ -199,22 +207,35 @@ static_cast<RV32VectorDestinationOperand*>(inst->Destination(0)); auto* vd_db = vd_op->CopyDataBuffer(); auto vd_span = vd_db->Get<uint8_t>(); - // Compute start and end locations. - int vstart = rv_vector->vstart(); + // Compute start and end locations. int start_byte = vstart / 8; int start_offset = vstart % 8; + uint8_t start_mask = 0b1111'1111 << start_offset; - if (start_byte < vd_span.size()) { - uint8_t start_mask = 0b1111'1111 << start_offset; + int end_byte = (vlen - 1) / 8; + int end_offset = (vlen - 1) % 8; + uint8_t end_mask = 0b1111'1111 >> (7 - end_offset); + + if (start_byte == end_byte) { + uint8_t mask = start_mask & end_mask; + vd_span[start_byte] = + (op(vs2_span[start_byte], vs1_span[start_byte]) & mask) | + (vd_span[start_byte] & ~mask); + } else { vd_span[start_byte] = (op(vs2_span[start_byte], vs1_span[start_byte]) & start_mask) | (vd_span[start_byte] & ~start_mask); + + for (int i = start_byte + 1; i < end_byte; i++) { + vd_span[i] = op(vs2_span[i], vs1_span[i]); + } + + vd_span[end_byte] = + (op(vs2_span[end_byte], vs1_span[end_byte]) & end_mask) | + (vd_span[end_byte] & ~end_mask); } - for (int i = start_byte + 1; i < vd_span.size(); i++) { - vd_span[i] = op(vs2_span[i], vs1_span[i]); - } vd_db->Submit(); rv_vector->clear_vstart(); }
diff --git a/riscv/riscv_vector_unary_instructions.cc b/riscv/riscv_vector_unary_instructions.cc index 6769b71..7ab4687 100644 --- a/riscv/riscv_vector_unary_instructions.cc +++ b/riscv/riscv_vector_unary_instructions.cc
@@ -297,6 +297,7 @@ rv_vector->set_vector_exception(); return; } + int vlen = rv_vector->vector_length(); auto src_op = static_cast<RV32VectorSourceOperand*>(inst->Source(0)); auto src_span = src_op->GetRegister(0)->data_buffer()->Get<uint8_t>(); auto mask_op = static_cast<RV32VectorSourceOperand*>(inst->Source(1)); @@ -305,11 +306,10 @@ static_cast<RV32VectorDestinationOperand*>(inst->Destination(0)); auto* dest_db = dest_op->CopyDataBuffer(0); auto dest_span = dest_db->Get<uint8_t>(); - int physical_bits = dest_span.size() * 8; bool before_first = true; int last = 0; // Set the bits before the first active 1. - for (int i = 0; i < physical_bits; i++) { + for (int i = 0; i < vlen; i++) { last = i; int index = i >> 3; int offset = i & 0b111; @@ -322,13 +322,11 @@ dest_span[index] |= 1 << offset; } } - for (int i = last; !before_first && (i < physical_bits); i++) { + // Clear the remaining bits. + for (int i = last; !before_first && (i < vlen); i++) { int index = i >> 3; int offset = i & 0b111; - int mask_value = (mask_span[index] >> offset) & 0b1; - if (mask_value) { - dest_span[index] &= ~(1 << offset); - } + dest_span[index] &= ~(1 << offset); } dest_db->Submit(); rv_vector->clear_vstart(); @@ -341,6 +339,7 @@ rv_vector->set_vector_exception(); return; } + int vlen = rv_vector->vector_length(); auto src_op = static_cast<RV32VectorSourceOperand*>(inst->Source(0)); auto src_span = src_op->GetRegister(0)->data_buffer()->Get<uint8_t>(); auto mask_op = static_cast<RV32VectorSourceOperand*>(inst->Source(1)); @@ -349,9 +348,8 @@ static_cast<RV32VectorDestinationOperand*>(inst->Destination(0)); auto* dest_db = dest_op->CopyDataBuffer(0); auto dest_span = dest_db->Get<uint8_t>(); - int physical_bits = dest_span.size() * 8; uint8_t value = 1; - for (int i = 0; i < physical_bits; i++) { + for (int i = 0; i < vlen; i++) { int index = i >> 3; int offset = i & 0b111; int mask_value = (mask_span[index] >> offset) & 0b1; @@ -378,6 +376,7 @@ rv_vector->set_vector_exception(); return; } + int vlen = rv_vector->vector_length(); auto src_op = static_cast<RV32VectorSourceOperand*>(inst->Source(0)); auto src_span = src_op->GetRegister(0)->data_buffer()->Get<uint8_t>(); auto mask_op = static_cast<RV32VectorSourceOperand*>(inst->Source(1)); @@ -387,9 +386,8 @@ auto* dest_db = dest_op->CopyDataBuffer(0); auto dest_span = dest_db->Get<uint8_t>(); - int physical_bits = dest_span.size() * 8; bool first = true; - for (int i = 0; i < physical_bits; i++) { + for (int i = 0; i < vlen; i++) { int index = i >> 3; int offset = i & 0b111; int mask_value = (mask_span[index] >> offset) & 0b1;
diff --git a/riscv/test/riscv_vector_opm_instructions_test.cc b/riscv/test/riscv_vector_opm_instructions_test.cc index 8c8070d..6bcda8d 100644 --- a/riscv/test/riscv_vector_opm_instructions_test.cc +++ b/riscv/test/riscv_vector_opm_instructions_test.cc
@@ -102,10 +102,19 @@ FillArrayWithRandomValues<uint8_t>(vd_value); AppendVectorRegisterOperands({kVs2, kVs1}, {kVd}); for (int vstart : {0, 7, 32, 100, 250, 384}) { - for (int vlen_pct : {10, 20, 50, 100}) { - int vlen = - (kVectorLengthInBytes * 8 - vstart) * vlen_pct / 100 + vstart; - CHECK_LE(vlen, kVectorLengthInBytes * 8); + std::vector<int> test_vlens = { + 0, + vstart, + vstart + 1, + vstart + 3, + vstart + 7, + (kVectorLengthInBytes * 8 - vstart) * 10 / 100 + vstart, + (kVectorLengthInBytes * 8 - vstart) * 20 / 100 + vstart, + (kVectorLengthInBytes * 8 - vstart) * 50 / 100 + vstart, + (kVectorLengthInBytes * 8 - vstart) * 100 / 100 + vstart, + }; + for (int vlen : test_vlens) { + if (vlen > kVectorLengthInBytes * 8) continue; // Configure vector unit for different lmul settings. uint32_t vtype = (kSewSettingsByByteSize[1] << 3) | kLmulSettings[6]; ConfigureVectorUnit(vtype, vlen); @@ -120,7 +129,7 @@ int mask_index = i >> 3; int mask_offset = i & 0b111; bool result = (dst_span[mask_index] >> mask_offset) & 0b1; - if (i < vstart) { + if ((i < vstart) || (i >= vlen)) { bool vd = (vd_value[mask_index] >> mask_offset) & 0b1; EXPECT_EQ(result, vd) << "[" << i << "] " << std::hex << "vd: " << (int)vd_value[mask_index]