unify put mnemonic; always value first#77
Conversation
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## master #77 +/- ##
======================================
Coverage 41.1% 41.1%
======================================
Files 19 19
Lines 4542 4547 +5
======================================
+ Hits 1865 1868 +3
- Misses 2677 2679 +2
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| write!(f, "\n{}S-REG:{}\t", sect, reset)?; | ||
| for i in 0..16 { | ||
| if let Some(ref v) = self.s16[i] { | ||
| write!(f, "{}s16{}[{}{:02}{}]={}{}{}\n\t", reg, eq, reset, i, eq, val, v, reset)?; | ||
| } | ||
| } |
There was a problem hiding this comment.
put 100,r8192[1];
put 2,r8192[2];
put 18,a16[1];
put 2,a16[2];
put "aaa",s16[1];
put "aaa",s16[2];
would be displayed as
CTRL: st0=true cy0=0 ca0=8 cl0=~ cp0=0
cs0=0 @ alien_vista_plaza_11111111111111111111111111111111
A-REG: a16[01]=0012h a16[02]=0002h
F-REG:
R-REG: r8192[01]=64h
S-REG: s16[01]="aaa"
s16[02]="aaa"
260a9f5 to
4caac4a
Compare
dr-orlovsky
left a comment
There was a problem hiding this comment.
I propose to make the PR not-API breaking (see my comment).
Also, what do you think, should we use left-to-right argument order (like now, with value going first), or do a X86-assembly way where the destination goes first?
src/isa/bytecode.rs
Outdated
| { | ||
| match self { | ||
| BytesOp::Put(reg, bytes, _) => { | ||
| BytesOp::Put(bytes, reg, _) => { |
There was a problem hiding this comment.
I am afraid changing in BytesOp::Put argument order is a breaking change which makes this PR impossible to release without a major version bump (while it will be nice to have it as a fix instead).
We have both styles for now (mov accepts source first while extr is dest first). So we can align to the "major" X86 style |
|
@6293 This alignment looks like a separate PR. But at least we can update this PR to match the future alignment. So if you are ok with x86 style (destination first, then source), can you pls update it here? |
|
It can be that Intel style BTW is not the best one so we might want to stick to more natural (for english) left-to-right order: https://stackoverflow.com/questions/67597388/correct-order-of-source-and-destination-in-assembly-language |
dr-orlovsky
left a comment
There was a problem hiding this comment.
LGTM except that we still need not to change BytesOp variants - otherwise we would break bytecode compatibility (and would also need a major version release).
So yes, the enum variants will not match the mnemonic representation. Sad, but otherwise too breaking downstream.
the change on BytesOp::Put has been reverted in the latest commit |
|
Thank you very much, great work! |
Closes #75