Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Incorrect syntax for some instructions' "definedBy" extensions #319

Closed
ThinkOpenly opened this issue Nov 27, 2024 · 2 comments · Fixed by #350
Closed

Incorrect syntax for some instructions' "definedBy" extensions #319

ThinkOpenly opened this issue Nov 27, 2024 · 2 comments · Fixed by #350
Assignees
Labels
bug Something isn't working

Comments

@ThinkOpenly
Copy link
Collaborator

Some instructions' "definedBy" fields appear to be incorrect, using an open comma-separated list, rather than a subfield refinement with "allOf".

./arch/inst/F/fmaxm.s.yaml:definedBy: F, Zfa
./arch/inst/F/froundnx.s.yaml:definedBy: F, Zfa
./arch/inst/F/fround.s.yaml:definedBy: F, Zfa
./arch/inst/F/fltq.s.yaml:definedBy: F, Zfa
./arch/inst/F/fleq.s.yaml:definedBy: F, Zfa
./arch/inst/F/fli.s.yaml:definedBy: F, Zfa
./arch/inst/F/fminm.s.yaml:definedBy: F, Zfa

I think each of these should be expanded to:

definedBy:
  allOf:
  - F
  - Zfa

Although, since Unpriv spec Version 20240411 says:

The Zfa extension depends on the F extension

maybe only the Zfa extension needs to be called out as the "definedBy" extension?

@ThinkOpenly ThinkOpenly added the bug Something isn't working label Nov 27, 2024
@AFOliveira AFOliveira self-assigned this Dec 4, 2024
@AFOliveira
Copy link
Collaborator

AFOliveira commented Dec 4, 2024

This is a nice catch and those instructions are missing on the output manual. I'll fix this ASAP. @dhower-qc Do you think it is worth it to add a test on this to the smoke tests?

@dhower-qc
Copy link
Collaborator

Do you think it is worth it to add a test on this to the smoke tests?

definitely!!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants