misc: add riscemu as an optional dependency #1019
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1019 +/- ##
==========================================
+ Coverage 86.71% 86.79% +0.08%
==========================================
Files 132 134 +2
Lines 20686 20813 +127
Branches 3129 3130 +1
==========================================
+ Hits 17938 18065 +127
+ Misses 2234 2231 -3
- Partials 514 517 +3
☔ View full report in Codecov by Sentry. |
Finally an excuse to move riscemu to poetry! Lovely! |
|
I guess a way to capture stdout from riscemu is also wanted soon then? |
|
Yeah would be great. In the xdsl interpreter, I added an IO to the interpreter class, which all printers print to, would be nice to have a similar thing in riscemu |
superlopuh
force-pushed
the
sasha/riscemu
branch
3 times, most recently
from
a38275d
to
f017ad9
Compare
|
I think adding riscemu as an optional dependency is OK. It has no binary requirements and is reasonably lightweight. It will also allow us to have better notebooks and have things work 'out of the box'. |
|
Ok @AntonLydike I think the only thing that we'd need is a new riscemu version with all the changes in the commit, and we should be good to merge! |
| from ..emulator.emulator_iop import RV_Debug, run_riscv | ||
| from .utils import riscv_code |
There was a problem hiding this comment.
As usual, I am not very happy about relative imports. So I pose the classic question: Could we integrate this more deeply into xDSL? If we want, we could have a requirements file for only running with riscemu as the backend 🤔
There was a problem hiding this comment.
Why don't you like relative imports? I generally like them more
There was a problem hiding this comment.
But yeah this file would move to xdsl before I open the PR up for review :)
There was a problem hiding this comment.
It feels quite rigid to me, making things depend on the folder structure it is used in. Then again, that's always the case in python world 🤔
superlopuh
changed the title
superlopuh
requested review from
compor,
nazavode,
math-fehr and
georgebisbas
as code owners
There was a problem hiding this comment.
I wasn't sure where to put this xdsl/interpreters seemed most appropriate, happy to take suggestions
| from riscemu.config import RunConfig | ||
| from riscemu import RV32I, RV32M | ||
| from riscemu.CPU import UserModeCPU | ||
| from riscemu.parser import AssemblyFileLoader | ||
| from riscemu.instructions.instruction_set import InstructionSet | ||
| from riscemu.types.instruction import Instruction |
There was a problem hiding this comment.
This should probably be in a try .. catch ... block somewhere instead of the top level?
This PR adds riscemu as a dependency of xDSL, adds a target to xdsl-opt to emulate using riscemu, and adds some unit and filecheck tests.
This PR adds riscemu as a dependency of xDSL, adds a target to xdsl-opt to emulate using riscemu, and adds some unit and filecheck tests.