AntonLydike / riscemu

RISC-V emulator in python
MIT License
48 stars 14 forks source link

changed = to += #34

Closed KGrykiel closed 1 year ago

KGrykiel commented 1 year ago

I am pretty sure that this is everything necessary to make it work as the riscv spec expetcs. It was tested by running some of the example programs with "j 0/ jal 0" added to prove that now those instructions should be effectively ommitted.

KGrykiel commented 1 year ago

in relation to https://xdsl.zulipchat.com/#narrow/stream/388924-RISC-V/topic/riscemu.20bugs/near/372010154

AntonLydike commented 1 year ago

I don't think this will work, as currently, get_imm resolves labels, so you won't be able to tell the difference between j main and j 0x100, one of which gives you the address of (not offset to) the label main, and the other one gives you 0x100 as a constant...

KGrykiel commented 1 year ago

ohh, right, back to the drawing board then

AntonLydike commented 1 year ago

Maybe we should rethink what an immediate value really is, maybe we should add a new datatype Immediate that can do the following:

We have three different kinds of things we need to take care of here:

Integer immediate Symbolic label Numeric label 1b/1f
as_offset_from(x) value resolve(value) - x resolve(value, x) - x
as_absolute_val value resolve(value) impossible :boom:

See https://docs.oracle.com/cd/E19120-01/open.solaris/817-5477/esqaq/index.html for more info on numeric labels. They are a real PITA.

Or maybe I'm just overthinking this, and we have add get_abs_imm and get_rel_imm? Idk tho.

KGrykiel commented 1 year ago

we definetely need some sort of distinction between a label and an immediate that can be detected by the emulator. I'll have a go at those suggestions and see what seems most convenient. Do you reckon there's gonna be a lot of elements in the project directly affected by it as well?

superlopuh commented 1 year ago

FWIW I like Immediate a lot and I'm not a big fan of get_rel_imm, but I guess it's a matter of style, I think both should work. One other option that might be easier to implement is get_imm() -> tuple[int, ImmediateKind] where the kind is an enum of string and int.