Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add RV64M instructions #4

Open
cyuria opened this issue May 21, 2024 · 26 comments
Open

Add RV64M instructions #4

cyuria opened this issue May 21, 2024 · 26 comments
Assignees
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Milestone

Comments

@cyuria
Copy link
Owner

cyuria commented May 21, 2024

See title

@cyuria cyuria added this to the rv64gc alpha milestone May 21, 2024
@cyuria cyuria added the good first issue Good for newcomers label May 21, 2024
@cyuria cyuria mentioned this issue May 21, 2024
7 tasks
@cyuria cyuria added the help wanted Extra attention is needed label May 22, 2024
@ThestralWarrior
Copy link

Hi, I want to contribute to this project. I have knowledge of x64 so RISC-V shouldn't be too difficult to pick up. How should I proceed with this? It's my first time trying open source and I could use some guidance!

@cyuria
Copy link
Owner Author

cyuria commented May 23, 2024

No worries, I really still need to fix up a lot of docs for contributing and stuff. Not to mention general usage docs 👀

The latest official RISC-V specs can be found here https://riscv.org/technical/specifications/, there's a chapter (ch13, page 65/84) on the RV64M extension on volume 1 (the unprivileged specification).

Have a look through https://github.com/cyuria/wrasm/blob/cddaa9f9248c5943e84f5359f95df250da5e016d/src/form/rv64i.c and the corresponding header file. Pretty much this issue is to create a new file (maybe called src/form/rv64m.c) and create the equivalent RISC-V bytecode generation functions and data structure.

Also make sure to add the instruction set to src/form/instructions.c (under the sets array). That'll make sure the assembly parser can actually find the instructions.

If you want to use any of the functions in src/form/rv64i.c, go ahead, but consider if they should be moved to src/form/generic.c instead.

One more thing is I'm currently working on overhauling the argument parsing, I'm happy to deal with any merge conflicts, but if you are on top of everything else I've just mentioned it would be fantastic if you could quickly check out this branch https://github.com/cyuria/wrasm/tree/11-implement-per-instruction-argument-parsing and make my life a bit easier 😃.

If you have any questions at all just let me know (including about the risc-v spec, it took me a while to wrap my head around a lot of the stuff in it).

@cyuria
Copy link
Owner Author

cyuria commented May 23, 2024

Oh also please write some unit tests under test/unit, for reference, here are my current unit tests for the rv64i instruction set https://github.com/cyuria/wrasm/blob/cddaa9f9248c5943e84f5359f95df250da5e016d/test/unit/form_rv64i.c

You might notice I'm not very good at writing tests but I have a few so that's there at least. To get the bytecode I created a tiny assembly file with the instructions I wanted and then used clang + llvm-objdump to assemble and then get the bytes and disassembly. The tests don't need to (and probably shouldn't) be exhaustive just make sure the right bytes are in the right place for the right instruction, it's mainly just to prevent regressions and make sure the code you write actually works.

If you also want you can add/change some tests under test/system. Basically there's a bunch of assembly (well a hello world file at least) and I just assemble and run it. You don't need to do this though, it requires qemu emulation binaries and lld (the llvm linker) to run (see the python script in said directory).

Sorry for writing so much, this should really be in CONTRIBUTING.md or something

@cyuria
Copy link
Owner Author

cyuria commented May 23, 2024

Oh also https://github.com/riscv-non-isa/riscv-asm-manual/blob/main/riscv-asm.md#user-content-fn-1-a43cd0e951dba5ca7a7ce7a0471ba5fc has some pseudo instructions and stuff, it might be worth having a look through it at some point.

Thanks for offering to help, I appreciate it.

@cyuria cyuria added help wanted Extra attention is needed and removed help wanted Extra attention is needed labels May 23, 2024
@cyuria
Copy link
Owner Author

cyuria commented May 23, 2024

I just opened up a draft PR #12 for the new assembly instruction argument parsing functionality I'm writing so you can click on that instead of trying to find the right branch

@ThestralWarrior
Copy link

Okay thank you! I will go through these.

@cyuria
Copy link
Owner Author

cyuria commented May 24, 2024

Update just to let you know I've merged #12 into master, also I had a quick look at the chapter on the multiplication and division instruction set and it seems like you won't need to actually implement anything, just maybe move the form_itype function to src/form/generic.c and put together the instruction definitions and add some preprocessor definitions etc. etc, just housekeeping stuff really and making sure everything works

@cyuria
Copy link
Owner Author

cyuria commented May 24, 2024

Sorry to give you so much stuff to read through, you only really need to have a look through the source code and the specific parts of the spec, but also chapter 34 of the RISC-V spec outlines the actual bits which need to be set so you'll probably want to have a look through there as well at some point (it's probably the most useful chapter in the whole spec for this particular project).

I should've maybe clarified I don't expect you to know the spec (I barely understand the basics myself and I've been working on this assembler for 4 months), you really only need to go over the two chapters (13 and 34), which should have all the info you need to get started with writing some code.

@ThestralWarrior
Copy link

All right then, I was beginning to worry that I had bit more than I could chew! Out of curiosity, how much risc v assembly do I actually need to know or do I simply refer the spec and go about implementing as it says?

@cyuria
Copy link
Owner Author

cyuria commented May 26, 2024

Out of curiosity, how much risc v assembly do I actually need to know or do I simply refer the spec and go about implementing as it says?

Pretty much just implement what the spec says. You might need to know how to write assembly in general and how the assembly parsing works (depending on what parts of the codebase you use/change). In my last PR (I think idk, one of them) I implemented the fence instruction and I still have no idea how to actually use it, so you should be fine. You really just need the spec to find out what opcodes correspond to which instruction and how they're generated.

Also I made some docs on adding extensions, it's pretty much just what I've already said here, but if you want to read through it anyway its under docs/contrib/adding_extensions.md

@cyuria
Copy link
Owner Author

cyuria commented Jun 5, 2024

Hey @ThestralWarrior, I noticed you haven't forked the repo yet. Have you made any progress towards this?

@ThestralWarrior
Copy link

Hi, I am currently going through this - https://riscv-programming.org/book/riscv-book.html. I am trying to atleast understand some basic RISC-V properly before I set out to contribute. But I will fork it rn, and I have also been studying the code and had some doubts, I will formulate them and ask you later.

@cyuria
Copy link
Owner Author

cyuria commented Jun 5, 2024

Thanks for getting back to me, if you have any doubts, the codebase is in a highly unstable state atm. That means everything is subject to change, so please let me know if you think there's anywhere to improve wrasm

@ThestralWarrior
Copy link

Let me know if I got this right - I need to create a separate file say muldiv.c under src/form which would contain the array of formation structs rv32m[] and rv64m[] and then fill it up with the correct values which would be of the r-types as multiplication and division instructions are of r-type. But form_rtype() and parse_rtype() are already defined so I can simply reuse them. So is this all I need to do for this issue? Besides addingrv32m and rv64m to the sets array in instructions.c. If there is something I am missing out or overlooking, please let me know.

@cyuria
Copy link
Owner Author

cyuria commented Jun 5, 2024

That sounds about right. You should be able to pretty easily test it, just write a little bit of assembly and see if it works. You'll want an emulator like qemu unless you have Linux running on an actual chip.

@ThestralWarrior
Copy link

Okay got it, I will get started on this

@ThestralWarrior
Copy link

ThestralWarrior commented Jun 6, 2024

Alright so I did as I said you can check it out under testing branch of my fork - https://github.com/ThestralWarrior/wrasm. But I am still kind of confused as to how to write and run the tests as I have never really had any experience dealing with tests. Also what is the qemu emulator being used for here?

@cyuria
Copy link
Owner Author

cyuria commented Jun 15, 2024

Sorry for not getting back to you earlier on this, that's entirely my bad.

For wrasm I have two types of tests, unit tests and system tests.

Both of them have a relevant subdirectory under test/.

The unit tests don't need an emulator. They basically just ensure that all (well not really because I need to write more tests) the individual functions work correctly. I'd appreciate it if you could write a few of tests, probably just one for each instruction unless you want to get fancy. Have a look at the code under test/unit/form_*.c to get an idea of what you should do. Again if you have any questions let me know and hopefully I don't take nine days to respond again (sorry about that).

The system tests are a series (currently two) RISC-V assembly files. There's a script under test/system/run tests.py which calls wrasm, lld and qemu to assemble and then run the assembly. It then checks that the stdout of the executed program is the same as expected.

There aren't too many files and I don't remember them off the top of my head but there's a JSON file which holds the file paths for the assembly and the files containing the expected output.

For qemu, you'd want to install the userspace riscv qemu binaries. There are pretty much two types of qemu binaries, system binaries and user(space) binaries. The system binaries emulate an entire machine, similar to a virtual machine. If you want to run any normal code on them, you'll first need to set up a working OS. The qemu userspace binaries still emulate the machine but translate and pass along any environment info and stuff like syscalls to the host OS instead of an emulated OS.

Basically if you have a RISC-V version of a program you can run it as if it were an x86 or ARM or whatever version of the program on your own host system.

Note that this does potentially pose a security risk, so I'd advise you to look through the code before you run it. The assembly isn't too long and should take you at most five minutes to figure out. Keep in mind there are security implications to running untrusted code on your machine and my python testing script is not an exception.

@cyuria
Copy link
Owner Author

cyuria commented Jun 15, 2024

Also if you have any questions or worries or anything and I don't get back to you in a day or two, try sending another message through just in case. I try to stay responsive, but I also tend to skim stuff so it's possible I've just missed your question

@cyuria
Copy link
Owner Author

cyuria commented Jun 15, 2024

Oh yeah here's a link to qemu's homepage https://www.qemu.org/
There are three main links from there, the second one is user mode emulation, which is the one I'm using for this project.

If you're running Linux, you should be able to get qemu from your distro's repositories (it'll be called something like qemu-user or maybe qemu-user-riscv).

If you're on windows then all bets of the assembly even working are off, I have no idea if the Linux syscall interface will actually work on windows or how qemu handles it.

If you're on Mac it'll be somewhere between the Linux and the windows experience.

Basically as long as you have an executable called qemu-user-riscv64 or qemu-user-riscv64-static on your path you did it correctly.

You can also choose to run your own tests by just straight up using the compiled wrasm executable. If you do, it'd be great if you could add the assembly to tests/system/ anyway, but I understand that might not be feasible and I don't expect you to add any tests there.

@ThestralWarrior
Copy link

Thank you for getting back to me. I will have to shift to Linux now haha, I was using Windows but it's obvious Linux syscalls won't work there. I will go through Qemu and the relevant files then. Also I hope it's not an issue that I am asking more questions than actually contributing code, I will catch on eventually 😅.

@cyuria
Copy link
Owner Author

cyuria commented Jun 16, 2024

Haha don't worry about it.

I don't actually know how qemu handles syscalls on windows, but it might still work, I'd give it a go so you can keep using windows. If you accidentally break something somehow as long as it's tested it should get automatically run on Ubuntu by GitHub, so you'll know if it's broken.

I'd say the easiest way to check if stuff works on windows is to just test it (the hello world program under test/system might be a good starting point).

WSL should also work fine (and maybe cygwin) if you want to try that instead. My aim is for wrasm to be supported on as many platforms as possible and windows is definitely part of that.

@cyuria
Copy link
Owner Author

cyuria commented Jun 25, 2024

Hey @ThestralWarrior just wanted to check in to see how this is going. If you don't want to add any non unit tests, you absolutely don't need to. In fact if you don't want to add unit tests, I'd be happy to add them for you, just let me know. Your branch is looking pretty good so far, I'm basically just waiting for a PR at this point. Of course that should wait until you're happy (although you could also open a draft PR). I think I said this earlier, but I'd be happy to deal with any merge conflicts (although idk exactly how github deals with them from your end).

Also it's very much understandable if you're busy doing linux things instead :)

@ThestralWarrior
Copy link

ThestralWarrior commented Jun 28, 2024

Hi, got caught up with assignments and upcoming end-semesters, my bad. I have added the test for the rv32m and rv64m instructions, added the bytecodes and reused the test_case() and main() from_rv32i. Does it look ready for a PR now? Also should I do a PR from my testing branch or the master branch? I tried merging but the changes weren't reflected in the master branch for some reason. I will try again if that's what's needed.

@cyuria
Copy link
Owner Author

cyuria commented Jul 1, 2024

Don't worry about it, everyone has other stuff. I don't mind which branch the PR comes from, so do whatever feels right to you (or whatever you can get working). I haven't had a look at the code, but you can open a PR whenever you feel ready, I'll definitely have a look then.

If you've still got some exams and/or assignments good luck with those, and don't worry about this project too much, I can always wait :)

@cyuria
Copy link
Owner Author

cyuria commented Aug 5, 2024

Hey @ThestralWarrior, it's been a month. I just wanted to check in with you again on how you would feel opening a PR for this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants