Add bseti instruction

[yanelox]

Single-Bit Set (Immediate) instruction

Mnemonic: bseti rd, rs1, shamt

Opcode: 1100100

This instruction returns rs1 with a single bit set at the index specified in shamt. The index is read from the lower log2(XLEN) bits of shamt. For RV32, the encodings corresponding to shamt[5]=1 are reserved.

Operation:

let index = shamt & (XLEN - 1) 
X(rd) = X(rs1) | (1 << index)

[codecov[bot]]

Codecov Report

Merging #1503 (cd74375) into main (0c116e3) will decrease coverage by 0.57%. The diff coverage is 100.00%.

:exclamation: Current head cd74375 differs from pull request most recent head 12e3e73. Consider uploading reports for the commit 12e3e73 to get more accurate results

@@            Coverage Diff             @@
##             main    #1503      +/-   ##
==========================================
- Coverage   99.81%   99.23%   -0.58%     
==========================================
  Files         139      139              
  Lines       11687    10324    -1363     
==========================================
- Hits        11665    10245    -1420     
- Misses         22       79      +57     

Impacted Files	Coverage Δ
simulator/func_sim/alu.h	93.90% <100.00%> (-6.10%)	:arrow_down:
simulator/risc_v/riscv_instr.cpp	100.00% <100.00%> (ø)
simulator/risc_v/t/unit_test.cpp	100.00% <100.00%> (ø)
simulator/modules/fetch/fetch.h	33.33% <0.00%> (-51.29%)	:arrow_down:
simulator/mips/mips.h	50.00% <0.00%> (-50.00%)	:arrow_down:
simulator/risc_v/risc_v.h	50.00% <0.00%> (-50.00%)	:arrow_down:
simulator/func_sim/instr_memory.h	38.46% <0.00%> (-47.26%)	:arrow_down:
simulator/modules/fetch/bpu/bp_interface.h	66.66% <0.00%> (-33.34%)	:arrow_down:
simulator/modules/branch/branch.h	66.66% <0.00%> (-28.79%)	:arrow_down:
simulator/infra/target.h	83.33% <0.00%> (-16.67%)	:arrow_down:
... and 117 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 0c116e3...12e3e73. Read the comment docs.

[yanelox]

What should i do, if your simulator parse commands with shamt[5] = 0 and with shamt[5] = 1 ? And result is absolutely same for both, [image: image.png] These tests both passed. Difference between them only in shamt[5] bit.

пн, 15 нояб. 2021 г. в 16:32, Pavel I. Kryukov @.***>:

@.**** requested changes on this pull request.

In simulator/risc_v/t/unit_test.cpp https://github.com/MIPT-ILab/mipt-mips/pull/1503#discussion_r749326950:

@@ -358,6 +360,11 @@ TEST_RV64_RR_OP( 1, add_uw, 0x12344321b5a69788, 0xabababab82736455, 0x1234432133 TEST_RV64_RR_OP( 2, add_uw, 0x1111111233333332, 0x11111111ffffffff, 0x1111111133333333) // 32 bits overflow TEST_RV64_RR_OP( 3, add_uw, 0x01010102222221ce, 0xffffffffffffffac, 0x0101010122222222) // 64 bits overflow

+TEST_RV64_IMM_OP ( 1, bseti, 0xA95, 0xA91, 0x2) +TEST_RV64_IMM_OP ( 2, bseti, 0x159D26AF37BF48C0, 0x159D26AE37BF48C0, 0x20)

This is a very unobvious test. What value I expect to see if it fails after my changes? The tests should tell about the purpose of instruction w/o looking into spec. Check #1504 https://github.com/MIPT-ILab/mipt-mips/pull/1504, for instance.

In simulator/risc_v/t/unit_test.cpp https://github.com/MIPT-ILab/mipt-mips/pull/1503#discussion_r749329103:

@@ -118,6 +118,8 @@ TEST_CASE("RISCV disassembly") TEST_RV32_DISASM ( 0x0ae7d7b3, "minu $a5, $a5, $a4"); TEST_RV32_DISASM ( 0x48D747B3, "packu $a5, $a4, $a3"); TEST_RV64_DISASM ( 0x8D707BB, "add_uw $a5, $a4, $a3"); // 0000100 | 01101 ($a3) | 01110 ($a4) | 000 | 01111 ($a5) | 0111011

• TEST_RV64_DISASM ( 0x28179713, "bseti $a4, $a5, 1");
• TEST_RV32_DISASM ( 0x28269793, "bseti $a5, $a3, 2");

For RV32, the encodings corresponding to shamt[5]=1 are reserved.

Is it possible to test that encoding is not parsed as bseti by our simulator?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/MIPT-ILab/mipt-mips/pull/1503#pullrequestreview-806067647, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHWAJBI5POEHO5WPVFD7EGDUMEDXVANCNFSM5IANNPVA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[pavelkryukov]

I don't see the image. Please never share a screenshot, they make less use as you cannot copy from it.

if your simulator parse commands with shamt[5] = 0 and with shamt[5] = 1 ?

Our simulator. Generally we can check it while constructing the instruction and say it is 'illegal opcode' if it is. Another option is to split it in riscv-opcodes, so RV32 version will have 0 hardcoded and RV64 will not. See how rori and roriw are different. I would implement it in this PR so we won't face it suddenly in the future. If you don't feel strong enough to do it, open an Issue, add a commented test, and reference Issue number, so this problem will be known and possibly be solved later.

[yanelox]

I read specification again with more attention and I stop to understand this phrase " For RV32, the encodings corresponding to shamt[5]=1 are reserved.". How we can reserve codes with shamt[5] = 1, if in RV32 case shamt have only 5 bits (indexes from 0 to 4)?

пн, 15 нояб. 2021 г. в 18:58, Pavel I. Kryukov @.***>:

I don't see the image. Please never share a screenshot, they make less use as you cannot copy from it.

if your simulator parse commands with shamt[5] = 0 and with shamt[5] = 1 ?

Our simulator. Generally we can check it while constructing the instruction and say it is 'illegal opcode' if it is. Another option is to split it in riscv-opcodes, so RV32 version will have 0 hardcoded and RV64 will not. See how rori and roriw are different. I would implement it in this PR so we won't face it suddenly in the future. If you don't feel strong enough to do it, open an Issue, add a commented test, and reference Issue number, so this problem will be known and possibly be solved later.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/MIPT-ILab/mipt-mips/pull/1503#issuecomment-969053949, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHWAJBNKYED6FFTICU45NOLUMEU2NANCNFSM5IANNPVA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[yanelox]

I understand my last question. But i have a new question, why RV64 version of rori is in opcodes32 and RV32 version of rori (roriw) is in opcodes64?

пн, 15 нояб. 2021 г. в 18:58, Pavel I. Kryukov @.***>:

I don't see the image. Please never share a screenshot, they make less use as you cannot copy from it.

if your simulator parse commands with shamt[5] = 0 and with shamt[5] = 1 ?

Our simulator. Generally we can check it while constructing the instruction and say it is 'illegal opcode' if it is. Another option is to split it in riscv-opcodes, so RV32 version will have 0 hardcoded and RV64 will not. See how rori and roriw are different. I would implement it in this PR so we won't face it suddenly in the future. If you don't feel strong enough to do it, open an Issue, add a commented test, and reference Issue number, so this problem will be known and possibly be solved later.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/MIPT-ILab/mipt-mips/pull/1503#issuecomment-969053949, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHWAJBNKYED6FFTICU45NOLUMEU2NANCNFSM5IANNPVA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[yanelox]

I tried to create a new test for rori func. Test, where shamt[5] bit is 1, and it is passed, but opcode was incorrect. I modify upper test and add 1 to shamt[5], is it supposed to work like this? (both tests are passed) TEST_RV32_DISASM ( 0x61F7D793, "rori $a5, $a5, 31"); // 01100 | 0011111 (31) | 01111 ($a5) | 101 | 01111 ($a5) | 0010011 TEST_RV32_DISASM ( 0x65F7D793, "rori $a5, $a5, 31"); // 01100 | 1011111 (31) | 01111 ($a5) | 101 | 01111 ($a5) | 0010011

пн, 15 нояб. 2021 г. в 18:58, Pavel I. Kryukov @.***>:

I don't see the image. Please never share a screenshot, they make less use as you cannot copy from it.

if your simulator parse commands with shamt[5] = 0 and with shamt[5] = 1 ?

Our simulator. Generally we can check it while constructing the instruction and say it is 'illegal opcode' if it is. Another option is to split it in riscv-opcodes, so RV32 version will have 0 hardcoded and RV64 will not. See how rori and roriw are different. I would implement it in this PR so we won't face it suddenly in the future. If you don't feel strong enough to do it, open an Issue, add a commented test, and reference Issue number, so this problem will be known and possibly be solved later.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/MIPT-ILab/mipt-mips/pull/1503#issuecomment-969053949, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHWAJBNKYED6FFTICU45NOLUMEU2NANCNFSM5IANNPVA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[pavelkryukov]

Do I understand correctly rori has the same wording regarding shamt[5]?

[yanelox]

Yes, it is.

пн, 15 нояб. 2021 г. в 22:00, Pavel I. Kryukov @.***>:

Do I understand correctly rori has the same wording regarding shamt[5]?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/MIPT-ILab/mipt-mips/pull/1503#issuecomment-969219841, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHWAJBNMLMYNWMVPIPKSNUDUMFKEBANCNFSM5IANNPVA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[yanelox]

"This instruction performs a rotate right of rs1 by the amount in the least-significant log2(XLEN) bits of shamt. For RV32, the encodings corresponding to shamt[5]=1 are reserved."

It is description of rori.

пн, 15 нояб. 2021 г. в 22:00, Pavel I. Kryukov @.***>:

Do I understand correctly rori has the same wording regarding shamt[5]?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/MIPT-ILab/mipt-mips/pull/1503#issuecomment-969219841, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHWAJBNMLMYNWMVPIPKSNUDUMFKEBANCNFSM5IANNPVA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[pavelkryukov]

OK. So please

• open an Issue
• add a commented test (both for rori and bseti)
• reference here the Issue number, so this problem will be known and possibly be solved later.

[yanelox]

Done. Issue numbers is #1507 for rori instr and #1508 for bseti instr. Add tests for both commands and comment them

пн, 15 нояб. 2021 г. в 22:10, Pavel I. Kryukov @.***>:

OK. So please

• open an Issue
• add a commented test (both for rori and bseti)
• reference here the Issue number, so this problem will be known and possibly be solved later.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/MIPT-ILab/mipt-mips/pull/1503#issuecomment-969229095, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHWAJBKZHSIXM5KI47FCGRDUMFLIRANCNFSM5IANNPVA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[pavelkryukov]

Please reference the issues (# + number) in code

[yanelox]

In comments to tests?

пн, 15 нояб. 2021 г. в 22:39, Pavel I. Kryukov @.***>:

Please reference the issues (# + number) in code

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/MIPT-ILab/mipt-mips/pull/1503#issuecomment-969253825, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHWAJBJFGTI5UVT5NVKLZULUMFOXLANCNFSM5IANNPVA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[pavelkryukov]

Yep. I'll fix the submodule conflict.

[yanelox]

Done.

пн, 15 нояб. 2021 г. в 22:43, Pavel I. Kryukov @.***>:

Yep. I'll fix the submodule conflict.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/MIPT-ILab/mipt-mips/pull/1503#issuecomment-969257042, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHWAJBI25A6S2RT7BBK6UT3UMFPHDANCNFSM5IANNPVA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.