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

Improved Reserved Field Mechanism #35

Open
wants to merge 34 commits into
base: json
Choose a base branch
from

Conversation

joydeep049
Copy link

@joydeep049 joydeep049 commented Jul 1, 2024

Changes in support of ThinkOpenly/sail-riscv#24

ThinkOpenly and others added 30 commits June 17, 2024 09:51
In the JSON backend, currently all messages (debug text and JSON data) are emitted to stdout.

Change debug messages to be emitted to stderr by using `prerr_endline` instead of `print_endline`.

Fixes ThinkOpenly#4.
In the JSON Backend, currently all the debug output is emitted to
stderr by default.

Introduce a conditional environment variable `DEBUG_OUT` to control debug output.

Fixes ThinkOpenly#6.
New JSON field "syntax" which facilitates displaying
parenthesized operands in syntax like "lw rd,offset(rs)".

Extract operands from assembly representation ("mapping clause assembly")
instead of function signatures ("union clause ast").

Remove "identity" functions in assembly representation.
In actuality, these functions transform an opcode field into the expected
assembly representation, which could be a well-known register name or the
actual value of an offset where the field is a truncated representation.
Hardcoding these Sail function names here is at least far from ideal,
and at worst, wrong (calling a truncated value and the full value the same
name).
More thought is needed as to the best realization of assembly syntax
from the assembly representation.

Ignore "opt_spc" in assembly representations.

Move function "dequote" up due to a new use.
Use the newly available `Reporting.loc_range_to_src` function
to extract the text of the function bodies verbatim from the
Sail source. Previously, the json contained the AST text for
the functions.
Extract the instruction syntax from the `mapping clause assembly`
(rather than presuming that the operands captured separately can
accurately represent syntax. They can't.)

Make sure to preserve separators (",") and parentheses.
A previous commit broke extraction of instruction operands/types.
Fix it.
Many instructions are grouped around a similar, common implementation
with the mnemonics embedded within a list of mnemonic:name mapping, such as:
```
mapping utype_mnemonic : uop <-> string = {
  RISCV_LUI   <-> "lui",
  RISCV_AUIPC <-> "auipc"
}

mapping clause assembly = UTYPE(imm, rd, op)
  <-> utype_mnemonic(op) ^ spc() ^ reg_name(rd) ^ sep() ^ hex_bits_20(imm)
```

Add support for extracting instruction names from within the mappings if
the names are added within as attributes, for example:
```
mapping utype_mnemonic : uop <-> string = {
  $[name Load Upper Immediate]
    RISCV_LUI   <-> "lui",
  $[name Add Upper Immediate to Program Counter]
    RISCV_AUIPC <-> "auipc"
}
```
This commit enhances the JSON Backend to properly handle instructions with optional operands.
Previously, these operands were not handled correctly.

The implementation introduces two new fields:
`optional`, which indicates whether an operand is optional (boolean),
and `default`, which holds the default value if no value is specified.

for 'vle16.v':

Before:
```
{
  "operands": [
    ...
    { "name": "vm", "type": "bits(1)" }
    ...
  ]
}
```
After:
```
{
  "operands": [
    ...
    { "name": "vm", "type": "bits(1)", "optional": true, "default": "v0.t" }
    ...
  ]
}
```
Fixes: ThinkOpenly#2
This fixes the ci workflows.
For now it is configured to runs exclusively on ocaml version`5.0.0`,
unlike the upstream which also uses `4.08.1`.
Support for earlier versions will be added gradually.
The tail end of the new "functions" content from
commit 4aa4024
has a lingering comma after the last element:
```
  {
    "name": "riscv_f32Div",
    "source": "{\n  [...]}"
  },
  ]
}
```

Fix this by using `String.concat` to join the "function"
elements with a comma between them, instead of emitting
each of them with a trailing comma.
* CI for JSON validation. Fixes ThinkOpenly#30.
* Run build CI on new pull-request.
@joydeep049
Copy link
Author

(Waiting for @ThinkOpenly to approve workflows.)

@joydeep049
Copy link
Author

joydeep049 commented Jul 18, 2024

I think there should be no problem here.
Here's an output when I use make on my branch.

jd@jd-virtual-machine:~/Desktop/New Folder/sail$ git checkout reserved-fields
Branch 'reserved-fields' set up to track remote branch 'reserved-fields' from 'origin'.
Switched to a new branch 'reserved-fields'
jd@jd-virtual-machine:~/Desktop/New Folder/sail$ make
dune build --release
(cd _build/default/src/lib && /home/jd/.opam/default/bin/ott -sort false -generate_aux_rules true -o ast.lem -picky_multiple_parses true ../../language/sail.ott)
Ott version 0.33   distribution of Mon 16 Jan 15:32:01 GMT 2023
(cd _build/default/src/lib && /home/jd/.opam/default/bin/ott -sort false -generate_aux_rules true -o jib.lem -picky_multiple_parses true ../../language/jib.ott)
Ott version 0.33   distribution of Mon 16 Jan 15:32:01 GMT 2023
jd@jd-virtual-machine:~/Desktop/New Folder/sail$ 

@ThinkOpenly

@joydeep049
Copy link
Author

joydeep049 commented Aug 3, 2024

CI is failing with the error message :

#=== ERROR while compiling sail_json_backend.0.17 =============================#
# context              2.2.0 | macos/x86_64 | ocaml-base-compiler.4.08.1 | pinned(git+file:///Users/runner/work/sail/sail#HEAD#3ce8299ccf1e955dde1e8cc1586c69163de2f553)
# path                 ~/.opam/4.08.1/.opam-switch/build/sail_json_backend.0.17
# command              ~/.opam/opam-init/hooks/sandbox.sh build dune build -p sail_json_backend -j 3 --promote-install-files=false @install
# exit-code            1
# env-file             ~/.opam/log/sail_json_backend-21990-5f8711.env
# output-file          ~/.opam/log/sail_json_backend-21990-5f8711.out
### output ###
# (cd _build/default && /Users/runner/.opam/4.08.1/bin/ocamlc.opt -w -40 -w -33 -w -27 -w -32 -w -26 -w -37 -g -bin-annot -I src/sail_json_backend/.sail_plugin_json.eobjs/byte -I /Users/runner/.opam/4.08.1/lib/bytes -I /Users/runner/.opam/4.08.1/lib/dune-private-libs/dune-section -I /Users/runner/.opam/4.08.1/lib/dune-site -I /Users/runner/.opam/4.08.1/lib/dune-site/private -I /Users/runner/.op[...]
# File "src/sail_json_backend/json.ml", line 97, characters 5-23:
# 97 |   if String.starts_with ~prefix:"\"" qs && String.ends_with ~suffix:"\"" qs then
#           ^^^^^^^^^^^^^^^^^^
# Error: Unbound value String.starts_with

Which is totally unrelated to the changes I made. There is no error when I am running it using make in local.
How should we deal with this? @ThinkOpenly

@ThinkOpenly
Copy link
Owner

CI is failing with the error message :

#=== ERROR while compiling sail_json_backend.0.17 =============================#
# context              2.2.0 | macos/x86_64 | ocaml-base-compiler.4.08.1 | pinned(git+file:///Users/runner/work/sail/sail#HEAD#3ce8299ccf1e955dde1e8cc1586c69163de2f553)
# path                 ~/.opam/4.08.1/.opam-switch/build/sail_json_backend.0.17
# command              ~/.opam/opam-init/hooks/sandbox.sh build dune build -p sail_json_backend -j 3 --promote-install-files=false @install
# exit-code            1
# env-file             ~/.opam/log/sail_json_backend-21990-5f8711.env
# output-file          ~/.opam/log/sail_json_backend-21990-5f8711.out
### output ###
# (cd _build/default && /Users/runner/.opam/4.08.1/bin/ocamlc.opt -w -40 -w -33 -w -27 -w -32 -w -26 -w -37 -g -bin-annot -I src/sail_json_backend/.sail_plugin_json.eobjs/byte -I /Users/runner/.opam/4.08.1/lib/bytes -I /Users/runner/.opam/4.08.1/lib/dune-private-libs/dune-section -I /Users/runner/.opam/4.08.1/lib/dune-site -I /Users/runner/.opam/4.08.1/lib/dune-site/private -I /Users/runner/.op[...]
# File "src/sail_json_backend/json.ml", line 97, characters 5-23:
# 97 |   if String.starts_with ~prefix:"\"" qs && String.ends_with ~suffix:"\"" qs then
#           ^^^^^^^^^^^^^^^^^^
# Error: Unbound value String.starts_with

Which is totally unrelated to the changes I made. There is no error when I am running it using make in local. How should we deal with this? @ThinkOpenly

If I understand correctly, this is only failing in the macOS-12 build, and not macOS-latest, nor ubuntu-latest. Given how OS-agnostic this project is, I think we should drop macOS-12 from CI if it causes us any issues. The failure you are seeing appears to be some dependency is installed at a level where String.starts_with is not supported.

@joydeep049
Copy link
Author

CI is failing with the error message :

#=== ERROR while compiling sail_json_backend.0.17 =============================#
# context              2.2.0 | macos/x86_64 | ocaml-base-compiler.4.08.1 | pinned(git+file:///Users/runner/work/sail/sail#HEAD#3ce8299ccf1e955dde1e8cc1586c69163de2f553)
# path                 ~/.opam/4.08.1/.opam-switch/build/sail_json_backend.0.17
# command              ~/.opam/opam-init/hooks/sandbox.sh build dune build -p sail_json_backend -j 3 --promote-install-files=false @install
# exit-code            1
# env-file             ~/.opam/log/sail_json_backend-21990-5f8711.env
# output-file          ~/.opam/log/sail_json_backend-21990-5f8711.out
### output ###
# (cd _build/default && /Users/runner/.opam/4.08.1/bin/ocamlc.opt -w -40 -w -33 -w -27 -w -32 -w -26 -w -37 -g -bin-annot -I src/sail_json_backend/.sail_plugin_json.eobjs/byte -I /Users/runner/.opam/4.08.1/lib/bytes -I /Users/runner/.opam/4.08.1/lib/dune-private-libs/dune-section -I /Users/runner/.opam/4.08.1/lib/dune-site -I /Users/runner/.opam/4.08.1/lib/dune-site/private -I /Users/runner/.op[...]
# File "src/sail_json_backend/json.ml", line 97, characters 5-23:
# 97 |   if String.starts_with ~prefix:"\"" qs && String.ends_with ~suffix:"\"" qs then
#           ^^^^^^^^^^^^^^^^^^
# Error: Unbound value String.starts_with

Which is totally unrelated to the changes I made. There is no error when I am running it using make in local. How should we deal with this? @ThinkOpenly

If I understand correctly, this is only failing in the macOS-12 build, and not macOS-latest, nor ubuntu-latest. Given how OS-agnostic this project is, I think we should drop macOS-12 from CI if it causes us any issues. The failure you are seeing appears to be some dependency is installed at a level where String.starts_with is not supported.

Yes. Should I make the changes in the GH Action to drop that particular job?

@joydeep049
Copy link
Author

Also, If it is required in the future, we could replace String.starts_with with our own implementation closer to something like what I implemented here:

let is_reserved_field field_name =
   let reserved_prefix = "reserved_bits_" in
   String.length field_name >= String.length reserved_prefix
   && String.sub field_name 0 (String.length reserved_prefix) = reserved_prefix

This also checks if the reserved bits string starts with the substring reserved_bits_.

@ThinkOpenly

@ThinkOpenly
Copy link
Owner

CI is failing[...]:

If I understand correctly, this is only failing in the macOS-12 build, and not macOS-latest, nor ubuntu-latest. Given how OS-agnostic this project is, I think we should drop macOS-12 from CI if it causes us any issues. The failure you are seeing appears to be some dependency is installed at a level where String.starts_with is not supported.

Yes. Should I make the changes in the GH Action to drop that particular job?

That would be fine with me, yes.

@joydeep049
Copy link
Author

The macos-12 4.08.1 build has been removed from CI.
Can we merge this now?
Or should I add an empty commit and let the new CI run once more??
@ThinkOpenly

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.

5 participants