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

Merge upstream #2

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from
Draft

Merge upstream #2

wants to merge 13 commits into from

Conversation

PhilippvK
Copy link
Member

@PhilippvK PhilippvK commented May 19, 2023

Let's try to keep track with the latest upstream changes. This is a draft because I am not 100% sure about a few merges i did and this is currently untested.

TODO:

  • Discussion open questions
  • Check Syntax (M2-ISA-R Frontend)
  • Generate ETISS Archs (M2-ISA-R Backend)
  • Run RISC-V Instruction Test Suite

RV64I.core_desc Outdated Show resolved Hide resolved
RV64I.core_desc Outdated Show resolved Hide resolved
RVC.core_desc Outdated Show resolved Hide resolved
RVC.core_desc Outdated Show resolved Hide resolved
RVC.core_desc Outdated Show resolved Hide resolved
X[rd % RFS] = (unsigned<XLEN>)(signed<32>)((signed<32>)X[rs1 % RFS] * (signed<32>)X[rs2 % RFS]);
}
behavior: if(rd >=RFS || rs1>=RFS || rs2>=RFS) raise(0, 2); else {
if(rd!=0) X[rd] = (signed<64>)((signed<32>)X[rs1] * (signed<32>)X[rs2]);
Copy link
Member Author

Choose a reason for hiding this comment

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

Unsure about this change here...

Copy link
Collaborator

Choose a reason for hiding this comment

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

will also break on 128 bit cores; also signed / unsigned assignment

@PhilippvK
Copy link
Member Author

@wysiwyng I would appreciate it if you could have a look at the open threads as well. The RISC-V spec is not really explicit regarding the signedness here.

@PhilippvK
Copy link
Member Author

Maybe also @Samanti-Das because some of the "conflicting" changes have been added by you.

@wysiwyng
Copy link
Collaborator

Look at the comments, there's a few things I disagree with. This is also our development repository, so making pull requests back to upstream is also encouraged.

I don't really like the way the RVE handling is done, the old handling rd % RFS was hacky but the new handling feels even more hacky. There should be some way to specify this behavior once, just as X[0] === 0 should be specified once and not repeated every single instruction.

@PhilippvK
Copy link
Member Author

This is also our development repository, so making pull requests back to upstream is also encouraged.

I am currently looking into this.

I don't really like the way the RVE handling is done, the old handling rd % RFS was hacky but the new handling feels even more hacky.

In my option most of these illegal instruction exceptions should not be generated in the instructions behavior. There should be a way to specify constrains on fields/operands in the encoding. If you agree with me, I can open up an Issue to discuss that publicly.

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