Skip to content

Fixed typo in constants.py #199

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

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

georgiatai
Copy link

Fixed typo of Zve512b to Zvl512b in Zvl_extensions.

Signed-off-by: georgiatai <[email protected]>
@jordancarlin
Copy link

@UmerShahidengr

@@ -8,7 +8,7 @@
platform_schema = os.path.join(root, 'schemas/schema_platform.yaml')
custom_schema = os.path.join(root, 'schemas/schema_custom.yaml')
Zvl_extensions = [
"Zvl32b", "Zvl64b", "Zvl128b", "Zvl256b", "Zve512b", "Zvl1024b"
"Zvl32b", "Zvl64b", "Zvl128b", "Zvl256b", "Zvl512b", "Zvl1024b", "Zvl2048b"

Choose a reason for hiding this comment

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

Doesn't look like Zvl2048b is an actual extension. From the "Zvl*: Minimum Vector Length Standard Extensions" section of the spec:

image

Copy link
Author

Choose a reason for hiding this comment

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

Spike is, however, configured with Zvl*, including Zvl2048b. If it is not included here, then when using riscof, we will be unable to configure with that VLEN. Should Zvl2048b still be included in here in this case, despite not being on the Table 60?

Copy link
Author

Choose a reason for hiding this comment

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

There is also a quote from the spec:

Longer vector length extensions should follow the same pattern.

Thus I think it would be fine to include longer Zvl extensions, and beneficial to match spike.

Signed-off-by: georgiatai <[email protected]>
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