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

Assert misa.B bit through --isa=...B... #1649

Merged
merged 1 commit into from
Apr 24, 2024

Conversation

YenHaoChen
Copy link
Contributor

@YenHaoChen YenHaoChen commented Apr 23, 2024

This PR provides misa.B=1 by --isa=...B...

Reference: #1559

Copy link
Collaborator

@aswaterman aswaterman left a comment

Choose a reason for hiding this comment

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

Seems like we are becoming inconsistent about how we handle these different cases, in multiple ways:

  • For P, we use the switch statement to populate the Zp extensions, rather than the if statements at the bottom. (This is only a stylistic concern; we don't need to fix it right now, but I thought it was worth mentioning.)
  • For A, we make it so that A implies Zaamo and Zalrsc, but we don't make Zaamo + Zalrsc imply A. This is inconsistent with what you propose we do for B. We should either delete the logic that makes Zba + Zbb + Zbs imply B, or add the logic so that Zaamo + Zalrsc imply A. I think people would prefer the latter option, so that misa will contain A if Zaamo+Zalrsc is specified.

@YenHaoChen
Copy link
Contributor Author

Thank you. I was not aware of the inconsistency. I targeted the missing character 'b' in the variable 'all_subsets,' which supports --isa=...B...

Let's delete the logic that makes Zba + Zbb + Zbs imply B in this PR. I will provide another PR that discusses the implications of A and B.

@YenHaoChen YenHaoChen changed the title Implement misa.B bit Implement misa.B bit through --isa=...B... Apr 23, 2024
@aswaterman
Copy link
Collaborator

Does this version still make B imply Zba+Zbb+Zbs? It isn’t clear to me how it works.

@YenHaoChen
Copy link
Contributor Author

YenHaoChen commented Apr 24, 2024

Does this version still make B imply Zba+Zbb+Zbs? It isn’t clear to me how it works.

Yes, we have B implying Zba+Zbb+Zbs (disasm/isa_parser.cc:333). The previous PR (#1559) provided the code for implication, which aims at the B extension. Unfortunately, it does not offer --isa=...B... functionality from the optional flag. This PR catches up with the missing.

@YenHaoChen YenHaoChen changed the title Implement misa.B bit through --isa=...B... Assert misa.B bit through --isa=...B... Apr 24, 2024
@aswaterman
Copy link
Collaborator

Ah, got it.

@aswaterman aswaterman merged commit 7aabaa7 into riscv-software-src:master Apr 24, 2024
3 checks passed
@YenHaoChen YenHaoChen deleted the pr-b branch April 24, 2024 01:10
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.

None yet

2 participants