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 extension requirements/implications #118

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

a4lg
Copy link
Contributor

@a4lg a4lg commented Sep 17, 2023

Those implications are useful to capture simpler "an extension specifies a feature" principle.

For instance, if the Za64rs extension is implemented (at most 64-byte reservation set), the feature specified by the Za128rs extension is also implemented (at most 128-byte reservation set).
In that case, implying Za64rsZa128rs makes feature detection logic simpler.

Despite that requiring the H extension to some Sh* extensions in the specification seems redundant (because they are mandatory in the profile if H is implemented), it will be useful if those extensions are used separately from the RISC-V Profiles.

Note that one of them (SmstateenSsstateen) is implemented in GNU Binutils version 2.40 or later.

Those implications are useful to capture simpler "an extension specifies
a feature" principle.  For instance, if the 'Za64rs' extension is
implemented (at most 64-byte reservation set), the feature specified by the
'Za128rs' extension is also implemented (at most 128-byte reservation set).
Implying 'Za64rs' → 'Za128rs' makes feature detection logic simpler.

Despite that requiring the 'H' extension to some Sh* extensions in the
specification seems redundant, it will be useful if those extensions are
used separately from the RISC-V Profiles.

Note that one of them ('Smstateen' → 'Ssstateen') is implemented in
GNU Binutils version 2.40 or later.

Signed-off-by: Tsukasa OI <[email protected]>
@a4lg
Copy link
Contributor Author

a4lg commented Sep 17, 2023

Failing to build the documentation seems a problem on mathematical. Manually installing mathematical version 1.6.14 should work (as a workaround) and we could also adopt container-based building implemented in the upstream riscv/docs-spec-template.

@a4lg
Copy link
Contributor Author

a4lg commented Sep 17, 2023

Some background of this topic:

  • We (at toolchain side) feel hesitant to adopt those new extension as-is because most of new extensions' descriptions are too brief. I think most of them are necessarily that simple but there is a room for improvements. This PR is a part of describing the extension and make them more like regular extensions.
  • The recent discussion at Toolchain SIG suggested that extension implication requires the explicit writing by the specification. On the other hand, some (including I) consider "an extension" equals "a feature" approach is simpler for feature detection. This PR adopts this principle and adds explicit implications to achieve this principle only by explicitly written rules.

@aswaterman
Copy link
Member

Sorry this fell by the wayside.

The Ssstateen change should not be addressed within this document, since that extension is defined in a different document (the PrivArch spec). Also, M-mode extensions like Smstateen are not visible to S-mode, and so they shouldn't be discussed in the normative description of an S-mode profile.

(FWIW, it looks to me like Binutils is correct. As the Smstateen spec says, it comprises a superset of the CSRs that Ssstateen comprises. So it's implicitly true that an implementation with both Smstateen and S-mode also implements Ssstateen.)

As for the other changes, I agree with their premise, but I wonder if it's best to hold off on them until we've fleshed out these other extensions into proper sections within the PrivArch spec.

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.

2 participants