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

Draft for PseudoInstructions #301

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

Conversation

AFOliveira
Copy link
Collaborator

I have this format being automatically generated for all pseudo-instructions. Does this reflect the discussion we had on last week's meeting?

The corner case of conditional pseudo-instructions is not automatable.

Signed-off-by: Afonso Oliveira <[email protected]>
Copy link
Collaborator

@dhower-qc dhower-qc left a comment

Choose a reason for hiding this comment

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

Thanks for doing this!

A few comments/questions:

  • origin_instruction(s?) should be an array so we can support the multi-inst sequences
  • Let's make the instruction reference a $ref (I think we are moving that direction universally in the db).
  • Instead of capturing encoding/match, could we specify variable values of the parent instruction? I think it captures the same information but might be more informative to a reader. (not sold on this, just thinking out loud). For example:
origin_instructions:
- parent: { $ref: inst/I/add.yaml# }
  encoding:
    variables:
      rs1:
        const: 0
      rs2:
        const: 5
      rd:
        const: 0

@AFOliveira
Copy link
Collaborator Author

AFOliveira commented Nov 22, 2024

Thanks for the review.

  • origin_instruction(s?) should be an array so we can support the multi-inst sequences

Idk about how the backend would be easier, so just asking, but if we define them as you did couldn't we just add another entry? e.g.

origin_instructions:
- parent: { $ref: inst/I/add.yaml# }
  encoding:
    variables:
      rs1:
        const: 0
      rs2:
        const: 5
      rd:
        const: 0
- parent: { $ref: inst/I/other.yaml# }
  encoding:
    variables:
      rs1:
        const: 0
      rs2:
        const: 5
      rd:
        const: 0

Let's make the instruction reference a $ref (I think we are moving that direction universally in the db).

Ok.

Instead of capturing encoding/match, could we specify variable values of the parent instruction? I think it captures the same information but might be more informative to a reader. (not sold on this, just thinking out loud).

I can do this, since it was already being done but in a different way for previous pseudoinstructions definition.

@dhower-qc
Copy link
Collaborator

Idk about how the backend would be easier, so just asking, but if we define them as you did couldn't we just add another entry? e.g.

Yep!

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