Add pseudo-instructions for Zimop/Zcmop #194
Conversation
Codecov Report
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. @@ Coverage Diff @@
## master #194 +/- ##
==========================================
- Coverage 93.58% 92.83% -0.75%
==========================================
Files 3 3
Lines 717 642 -75
==========================================
- Hits 671 596 -75
Misses 46 46
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
unratified/rv_zimop
Outdated
|
|
||
| mop.r.N rd rs1 31=1 29..28=0 25..22=7 14..12=4 6..2=0x1C 1..0=3 |
There was a problem hiding this comment.
I'm surprised this works, given that the parser is supposed to issue an error when multiple instructions use the same opcode. Don't you need to use the $pseudo_op directive? If not, did you accidentally detect a bug in the parser?
There was a problem hiding this comment.
The parser did not complain. Likely it only looks for identical code points but not an overlapping code point. I could not use the pseudo_op directive as unlikely a true pseudo-op which requires a 1:1 map with an op, this a 1:N map.
There was a problem hiding this comment.
Maybe we can remove this change in opcodes. For spike for example, we can achieve the same by using MOP_R_0_MATCH as both match and mask. Or we just add 48 instructions into spike..
There was a problem hiding this comment.
I think we’ve found one bug and one deficiency in the parser. It should detect any form of overlap, not just exact matches, and we should be able to declare pseudos that overlap multiple instructions. That’s what the @ directive used to do in the old version of this script.
IMO we should use this version of the patch, and fix the bug later, but it would probably be good to get @neelgala’s input before merging.
There was a problem hiding this comment.
I was a bit worried about that people using this may get multi-hits but I now think thats unfounded as they would either use the _N version or the individual decode and not both. So agree that we should use this.
There was a problem hiding this comment.
Is this the right syntax to use:
c.mop.N c_mop_t 1..0=1 6..2=0 11=0 7=1 12=0 15..13=3
$pseudo_op rv_zcmop::c.mop.N c.mop.0 1..0=1 6..2=0 11..7=1 12=0 15..13=3
I added this to the arg_lut
arg_lut['c_mop_t'] = (10,8)
There was a problem hiding this comment.
Line 733 in e6b3022
And your syntax looks correct
There was a problem hiding this comment.
Thanks. I have updated this PR. It looks correct to me but I will test it with spike later today and let you know.
- include the list of pseudo instructions here so that they are emitted for spike.
May be a good addition to the readme.
There was a problem hiding this comment.
The tests are good.
There was a problem hiding this comment.
Updated with new mnemonics for c.mop
Add mop.r.N, mop.rr.N, and c.mop.N pseudo-instructions that match all instructions in that group.