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

Update RISC-V ISA documentation #492

Merged
merged 7 commits into from
Jan 3, 2024
Merged

Conversation

StarsDown64
Copy link
Contributor

@StarsDown64 StarsDown64 commented Dec 27, 2023

Simple edit to reflect the extra extensions supported in codewars/riscv#7. Also adds links to specifications since some extensions are not included in the RISC-V Spec.
Still not completely happy with line 13, I feel like it's not visually appealing but the ISA string the closest there is to a version number. Maybe listing the QEMU version? Not sure.
As I will comment in codewars/riscv#7 as well, Zbkx is not necessary as it's in both Zk and Zks shorthands.

@StarsDown64
Copy link
Contributor Author

StarsDown64 commented Dec 27, 2023

@dramforever requesting your thoughts.

@dramforever
Copy link

LGTM. I think for version string maybe QEMU as you said and also GCC and Binutils version?

@StarsDown64
Copy link
Contributor Author

Now that I think about it, none of the QEMU, GCC, or any runner tool version is specific to riscv language, so I wouldn't think it appropriate for the riscv specific docs.

@dramforever
Copy link

dramforever commented Dec 27, 2023

My understanding is that GCC, Binutils, QEMU are in fact unique to riscv language. Presently at least, no other language uses QEMU for emulation, and the GCC and Binutils here are cross compilation toolchains run on (emulated or real) RISC-V machines, which also no other language uses.

@StarsDown64
Copy link
Contributor Author

Huh. I see. If you know where to look, could you get me any relevant version info to include? I'm not familiar at all with the runner or toolchain.

@dramforever
Copy link

Just had an idea to "cheat" and do a system("gcc --version"); system("as --version"); in a kumite and got

gcc (Ubuntu 11.3.0-1ubuntu1~22.04) 11.3.0
GNU assembler (GNU Binutils for Ubuntu) 2.38

As for QEMU I think this means we're using 7.1 codewars/riscv#9

Copy link
Member

@DonaldKellett DonaldKellett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very detailed, LGTM!

@hobovsky
Copy link
Contributor

Looking at the preview here: https://deploy-preview-492--reverent-edison-2864ea.netlify.app/languages/riscv I am not sure I like the style of the asterisk/dagger/etc. annotations. Formatting is a bit off, and I htink they might look somewhat unusual to people more familiar with just numbered annotations like 1) . What do you think?

@DonaldKellett
Copy link
Member

The style seems in line with official RISC-V documentation, but I agree their appearance isn't that common in the Codewars docs and might not fit in well with the rest of the documentation.

I'm personally not bothered by it but let's collect some opinions from a few others before we decide whether to stick to the current style or revert to plain numbered annotations.

@StarsDown64
Copy link
Contributor Author

StarsDown64 commented Dec 27, 2023

Is there precedent elsewhere in the docs? I searched through the repo and found one other instance of the single dagger (in content\gamification\priveleges.md:27,32) and none of the double dagger or section symbol. I tried looking for instances of \d\) or \[\d\] in the repo but couldn't find any being used as footnote markers.

@hobovsky
Copy link
Contributor

Numbers vs daggers is one thing, and we can fix it later if needed. But another thing which puts me off a bit is that the footnote markers seem to be not superscripted, neither in the main text, nor in footnotes themselves?

@StarsDown64
Copy link
Contributor Author

I see what you mean. I like the look of superscripts as well. Adding.

@StarsDown64 StarsDown64 requested a review from error256 January 3, 2024 01:16
@DonaldKellett DonaldKellett merged commit b2d66d5 into codewars:master Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants