emul-branch: move branch logic in its own category #512
Conversation
Well, that, and they mess with the 'pc' (program counter). |
| }; | ||
|
|
||
| let new_pc = if taken { | ||
| pc.wrapping_add(decoded.imm_b()) |
There was a problem hiding this comment.
You could change this to something like:
let new_pc = pc.wrapping_add(if taken { decoded.imm_b() } else { WORD_SIZE })
Btw, in my old VM we let the decoder change all the branches from relative addressing to absolute addressing, and forbid having executable code in the last four bytes, so we didn't have to worry about wrap-around for branches.
(If you are happy to forbid a few more kb of memory for execution, you can leave the relative addressing in, but still not worry about wrap-around.)
Wrap-around ain't a problem for the emulator, only for the constraints. See the discussion we had about memory.
There was a problem hiding this comment.
I just didn’t want to change any behavior in this PR.
Indeed, wrap-around by WORD_SIZE increment is not supported (unsatisfiable).
There was a problem hiding this comment.
Yes, preserving behaviour in this PR is a good idea. I was talking more generally, but unfortunately didn't make that clear. Sorry.
There was a problem hiding this comment.
Would you be able to make a follow up PR with the changes we discussed perhaps?
Branch instructions are different than Compute in that they don’t write rd.
Useful for #245.