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

added pseudo op for jalr format used Hennesy&Patterson #81

Merged
merged 1 commit into from
Jun 22, 2020

Conversation

BenjaminBeichler
Copy link
Contributor

Unfortunately "Computer Organization and Design RISC-V Edition" use in their examples a non-standard format for jalr. To simply copy and paste their programs, add this pseudo op.

src/PseudoOps.txt Outdated Show resolved Hide resolved
@TheThirdOne
Copy link
Owner

TheThirdOne commented Jun 22, 2020

It does appear that gcc also supports this so I think it's reasonable to support it. With the one issue of 10 vs -100 resolved I would be happy to merge this.

Thanks for jumping in to add this.

@BenjaminBeichler
Copy link
Contributor Author

ahh thanks, I didn't fully understand the pseudo ops file format :-) I just updated my branch

@TheThirdOne
Copy link
Owner

It definitely is a little confusing and the comments are actually out of date. The only reason I noticed is because I work with the assembler code.

I actually don't see what you changed by the force push. I only see the commit and signature change.

Unfortunately "Computer Organization and Design RISC-V Edition" use in their examples a non-standard format for jalr. To simply copy and paste their programs, add this pseudo op.
@BenjaminBeichler
Copy link
Contributor Author

yeah, I forgot to add the real change ... somehow it resulted still in a new commit, what confused me. It should be really fixed now.

@TheThirdOne
Copy link
Owner

I suspect you made the first commit on Github, and then made the change on a local repo. That would result in the two commits being different because one was signed and the other was not.

Anyway it looks good now.

@TheThirdOne TheThirdOne merged commit 05e3309 into TheThirdOne:master Jun 22, 2020
@Jonas-Salcedo
Copy link

Wow! I think I commented on this a few weeks ago. I didn't expect you to actually add it. Thanks a lot!

@TheThirdOne
Copy link
Owner

@fudgenuggets12, You should be grateful to BenjaminBeichler. Without him this would not have gotten added; I didn't know if this was a common format. I did ask you at the time where you had seen it formatted like that.

Is there some book / tutorial that uses that syntax? It seems to definitely not be the standard, but the gnu assembler does allow it without warnings.

If you had told me that Patterson and Hennessy wrote it like that, I would have added it then. Because you additionally were concerned about why your program was infinitely looping, I explained why rather than probe you for information about the format.

If you notice something off in the future, a little bit of context can help get me to actually add something. I don't teach students RISC-V assembly and don't own a copy of the Patterson and Hennessy RISC-V edition so I have a substantial blind spot with regards to what a lot of users want.

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.

3 participants