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 pseudoinstruction VNEG #33

Open
wants to merge 26 commits into
base: JSON
Choose a base branch
from
Open

Conversation

Linda-Njau
Copy link

This change adds the pseudoinstruction VNEG that maps to VX_VRSUB, following the steps outlined in issue #6.

From the RISC-V Instruction Set Manual Volume I, VNEG is described as:

vneg.v vd,vs = vrsub.vx vd,vs,x0

After running make json the output for this pseudoinstruction is:

{
  "mnemonic": "vneg.v",
  "name": "TBD",
  "operands": [ { "name": "vd", "type": "regidx", "optional": false },{ "name": "vs", "type": "regidx", "optional": false } ],
  "syntax": "vd,vs",
  "format": "TBD",
  "fields": [ { "field": "encdec(VXTYPE(VX_VRSUB,0b0,vs,reg_name(x0),vd))", "size": 0 } ],
  "extensions": [ "V" ],
  "function": "execute(VXTYPE(VX_VRSUB, 0b0, vs, reg_name(\"x0\"), vd))",
  "description": "TBD"
}

ThinkOpenly and others added 26 commits June 19, 2024 07:43
Produces JSON representation of instructions, operands, opcode layouts, etc.
Not claimed to be exhaustive.

Depends on https://github.com/ThinkOpenly/riscvdecode.

Signed-off-by: Paul A. Clarke <[email protected]>
More support for ThinkOpenly#4.
Using the `extension()` function in `mapping clause encdec` expressions for extensions allows a parser to clearly know when a function is part of an extension (or set of extensions).
…e.sail

Added instruction names and descriptions

* Adds names for instructions in `riscv_insts_base.sail` using the attributes approach

* Descriptions are added to instructions in the same file using the doc comments approach
Currently, when executing `make json`, the command itself is echoed,
causing issues with the JSON representation.

This PR Suppress this echoing, ensuring a proper JSON output
without the need for any manual editing later on.
Some instructions are grouped in a single `mapping clause assembly`
where the respective mnemonics are embedded within a separate `mapping`.

For instructions which are _not_ grouped, the name of the instruction
can be added as an attribute to any of the instruction-specific constructs.

For grouped instructions, the attribute needs to be added to a construct
at the granularity of the specific instruction. Here, we attach the
attribute within the individual element of the `mapping` that includes
the actual instruction mnemonic.

This approach is still insufficient for mnemonics that are constructed:
```
mapping clause assembly = VLSEGTYPE(nf, vm, rs1, width, vd)
  <-> "vl" ^ nfields_string(nf) ^ "e" ^ vlewidth_bitsnumberstr(width) ^ ".v" [...]
```
...so more thought is needed here.

Also, I'm settling on a convention of using lower case (not Leading Caps),
at least for now. This matches how the instruction names are written in
the ISA specification, at least for those instructions for which the name
is actually provided. :-/
* Move definition of function of `extension`
* Utilize extension() instead of `haveZmmul()`
* Utilize extension() instead of `haveUsrMode()`
* Utilize extension() instead of `haveSupMode()`
* Utilize extension() instead of `haveNExt()`
* Utilize extension() instead of `haveZkr()`
* Utilize extension() instead of `haveUsrMode()`
* Move comments from `have*` functions into `extension` function
* Delete all unused  `have*` definitions of various extensions

Fixes ThinkOpenly#4 .
GitHub CI is complaining:
```
fix end of files.....................Failed
- hook id: end-of-file-fixer
- exit code: 1
- files were modified by this hook

Fixing doc/JSON.md
[...]
All changes made by hooks:
diff --git a/doc/JSON.md b/doc/JSON.md
index 82170ef..fc7f9cc 100644
[...]
-4.  Within that clone : `make json`
\ No newline at end of file
+4.  Within that clone : `make json`
Error: Process completed with exit code 1.
```
The JSON backend extracts function code verbatim into JSON format.
Unfortunately, the Unicode "Less-than Or Equal To" character, when embedded in
JSON, even when escaped using OCaml `String.escaped` does not successfully parse
with Python (3.10.12).

For now, at least, revert the Unicode symbol to the ASCII equivalent, which is
exactly what is used in the corresponding code and thus might even reduce the
chance of confusion.
@Linda-Njau
Copy link
Author

I've noticed the fields attribute in the JSON output isn't right. I'll work on it tomorrow and try to fix it before you review it. If I get stuck, I'll ask for help.

@Linda-Njau
Copy link
Author

Update on the fields attribute of the JSON output. I added the zext.w pseudoinstruction as defined in the comment on issue #6. Its JSON output for the fields attribute is similar to the output of the pseudoinstruction vneg.v that I previously added. So this leads me to think that we may need to adjust how pseudoinstructions are extracted into the JSON.

 "mnemonic": "zext.w",
  "name": "TBD",
  "operands": [ { "name": "rd", "type": "regidx", "optional": false },{ "name": "rs1", "type": "regidx", "optional": false } ],
  "syntax": "rd,rs1",
  "format": "TBD",
  "fields": [ { "field": "encdec(ZBA_RTYPEUW(reg_name(zero),rs1,rd,RISCV_ADDUW))", "size": 0 } ],
  "extensions": [ "Zba" ],
  "function": "execute(ZBA_RTYPEUW(reg_name(\"zero\"), rs1, rd, RISCV_ADDUW))",
  "description": "TBD"


@ThinkOpenly
Copy link
Owner

{
  "mnemonic": "vneg.v",
  "name": "TBD",
  "operands": [ { "name": "vd", "type": "regidx", "optional": false },{ "name": "vs", "type": "regidx", "optional": false } ],
  "syntax": "vd,vs",
  "format": "TBD",
  "fields": [ { "field": "encdec(VXTYPE(VX_VRSUB,0b0,vs,reg_name(x0),vd))", "size": 0 } ],
  "extensions": [ "V" ],
  "function": "execute(VXTYPE(VX_VRSUB, 0b0, vs, reg_name(\"x0\"), vd))",
  "description": "TBD"
}

Yes, this is pretty much what is expected at the moment.

Where we need to take these is to add some new handling, as you surmised:

  • The content above is fine, and can be left as-is
  • Since we now have the pseudo_of mapping, we can utilize that to emit new information in the JSON, something like a new top-level section:
    "pseudoinstructions": [
      {
        "mnemonic": "vneg.v",
        "name": "TBD",
        "operands": [ { "name": "vd", "type": "regidx", "optional": false },{ "name": "vs", "type": "regidx", "optional": false } ],
        "syntax": "vd,vs",
        "extensions": [ "V" ],
        "function": "vrsub.vx vd,vs,x0",
        "description": "TBD"
      },
      [...]
    
    ... or something like that.

@ThinkOpenly
Copy link
Owner

Interesting. The CI checks failed in one of the test cases, rv32mi-p-shamt.elf. It appears that they run the tests via test/run_tests.sh. Want to investigate that?

@Linda-Njau
Copy link
Author

Linda-Njau commented Jul 10, 2024

Interesting. The CI checks failed in one of the test cases, rv32mi-p-shamt.elf. It appears that they run the tests via test/run_tests.sh. Want to investigate that?

Yes, I'm looking into it.

@Linda-Njau
Copy link
Author

Where we need to take these is to add some new handling, as you surmised:

  • The content above is fine, and can be left as-is
  • Since we now have the pseudo_of mapping, we can utilize that to emit new information in the JSON, something like a new top-level section:

I see. We can discuss more on the implementation and its priority level

@ThinkOpenly
Copy link
Owner

@Linda-Njau , I see some "Add names" commits are appearing in this branch. Could you move them to a separate branch, so this branch covers only the one topic, "pseudoinstructions", please?

@Linda-Njau
Copy link
Author

@Linda-Njau , I see some "Add names" commits are appearing in this branch. Could you move them to a separate branch, so this branch covers only the one topic, "pseudoinstructions", please?

Done : )

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.

7 participants