Add mnemonic mapping for pseudoinstructions #45

Linda-Njau · 2024-11-12T14:56:13Z

This PR introduces functionality to retrieve the mnemonic for a base instruction associated with a pseudoinstruction.
This lays the groundwork for converting pseudoinstruction data into JSON.

It includes the following key updates:

Parse pseudoinstruction data

Add functionality to parse AST data and store pseudoinstruction-to-base-instruction mappings in a Hashtable.
Example:

LA: [
  assembly(UTYPE(imm[31..12], rd, RISCV_AUIPC)),
  assembly(ITYPE(imm[11..0], reg_name("x0"), rd, RISCV_ADDI))
]

Stored as:

Adding to hashtable with key: LA, id_inner: UTYPE, args_inner_list: [subrange_bits(imm, 31, 12), rd, RISCV_AUIPC]
Adding to hashtable with key: LA, id_inner: ITYPE, args_inner_list: [subrange_bits(imm, 11, 0), reg_name_backwards("x0"), rd, RISCV_ADDI]

Mnemonic retrieval (get_mnemonic)
Add logic to extract a mnemonic by either direct extraction or mapping parameter to argument through map_param_to_arg and argument to mnemonic through map_arg_to_mnemonic.

Direct retrieval:

C_ZEXT_W: c.zext.w, creg_name(rsdc)
Returns: c.zext.w

param-arg-mnemonic:

ITYPE: itype_mnemonic(op), spc, reg_name(rd), sep, reg_name(rs1), sep, hex_bits_signed_12(imm)
Returns: op -> RISCV_ADDI -> addi

Map parameter to argument (map_param_to_arg)
Add function to match parameter with its corresponding argument in the parsed data.

Example:

Input list: ITYPE:  imm, rs1, rd, op
Argument list: [subrange_bits(imm, 11, 0), reg_name("x0"), rd, RISCV_ADDI]
Returns: RISCV_ADDI for op.

Map mnemonic to argument (map_arg_to_mnemonic)
Add function to match argument to its corresponding mnemonic using a predefined enum-mnemonic mapping.

Example:

Matched RISCV_AUIPC with mnemonic auipc.
Matched RISCV_ADDI with mnemonic addi.

These changes enable the process_base_instruction function to complete the first step of converting raw pseudoinstruction data into a JSON by retrieving mnemonics for all associated base instructions.

Each entry in the `base_instructions` Hashtbl uses the pseudoinstruction AST key as the key, with the value being a tuple that contains: 1. The base instruction's type. 2. The base instruction's argument list. Each base instruction is stored as its own entry. ``` Adding to hashtable with key: LA, id_inner: UTYPE, args_inner_list: [subrange_bits(imm, 31, 12), rd, RISCV_AUIPC] Adding to hashtable with key: LA, id_inner: ITYPE, args_inner_list: [subrange_bits(imm, 11, 0), reg_name_backwards("x0"), rd, RISCV_ADDI] ```

`process_base_instruction` iterates over `base_instructions` Hashtbl and calls `get_mnemonic` for each entry to retrieve its mnemonic.

`get_mnemonic` retrieves a mnemonic in one of two ways: 1. Parameter extraction: If a parameter exists in the assembly string, it is extracted and mapped to an argument using `map_param_to_arg`. The result is then processed by `map_arg_to_mnemonic` to get the final mnemonic. For example in the assembly string: ``` ITYPE:itype_mnemonic(op), spc, reg_name(rd), sep, reg_name(rs1), sep, hex_bits_signed_12(imm) ``` `op` is mapped to `RISCV_ADDI`, which results in `addi`. 2. Direct mnemonic: If no parameter is found, the mnemonic is directly retrieved from the assembly string and cross-checked against `assembly_clean` Hashtbl for accuracy. Example: ``` C_ZEXT_W:c.zext.w, spc, creg_name(rsdc) ``` Returns: ``` c.zext.w ``` Returns `None` if no mnemonic is found in either path.

`map_param_to_arg` gets the input list from `inputs` Hashtbl: ``` ITYPE: imm, rs1, rd, op ``` It then finds the index of the parameter (`op` at index 3) and returns the corresponding argument at that index. For example, with `op` at index 3, the function returns `RISCV_ADDI` from the argument list `[subrange_bits(imm, 11, 0), reg_name("x0"), rd, RISCV_ADDI]`.

`map_arg_to_mnemonic` looks up a list of enum_mnemonic pairs in the `mappings` Hashtbl. It then matches the provided `arg` with the enum and returns the corresponding mnemonic. ``` Matched RISCV_ADDI with mnemonic: addi ```

ThinkOpenly

Thanks for breaking this PR into a group of smaller commits.
And, it's good work, especially given the complexity! I only have minor comments.

ThinkOpenly · 2024-11-12T16:54:01Z

src/sail_json_backend/json.ml

@@ -433,10 +434,51 @@ let parse_funcl fcl =
          debug_print ("id_of_dependent: " ^ id);
          let source_code = extract_source_code (Ast_util.exp_loc e) in
          Hashtbl.add functions id source_code
-      | Pat_exp (P_aux (P_app (i, pl), _), e) | Pat_when (P_aux (P_app (i, pl), _), e, _) ->
+      | Pat_exp (P_aux (P_app (i, pl), _), e) | Pat_when (P_aux (P_app (i, pl), _), e, _) -> (
          debug_print ("FCL_funcl execute " ^ string_of_id i);
          let source_code = extract_source_code (Ast_util.exp_loc e) in


Move this line down to just above Hashtbl.add executes below, since it's not used in the pseudo_of processing.

@ThinkOpenly, I intended for the line you're suggesting to move to be part of pseudo_of processing. My thought process is that pat matches similar patterns with different id values: pseudo_of, execute, and pseudo_execute. That's why I nested the id matches within the pat match. Do you think this approach is flawed?

Just to make sure we're talking about the same line, I was referring to:

let source_code = extract_source_code (Ast_util.exp_loc e) in

Aah, okay... I had the wrong line. We're on the same page now : )

ThinkOpenly · 2024-11-12T16:57:13Z

src/sail_json_backend/json.ml

+                                  List.iteri
+                                    (fun index inner_value ->


Do you need List.iteri here? I don't see a use of index below.

ThinkOpenly · 2024-11-13T16:31:56Z

src/sail_json_backend/json.ml

+let get_mnemonic id args_list =
+  match Hashtbl.find_opt assembly id with
+  | Some (str :: _) ->
+      if Str.string_match (Str.regexp ".+(\\(.*\\))") str 0 then (


Might be nice to describe what this regex is looking for, or make it a variable with a meaningful name.

ThinkOpenly · 2024-11-13T16:34:03Z

src/sail_json_backend/json.ml

+        | Some _ -> None
+        | None -> None


These could be combined in a catch-all using "_".

ThinkOpenly · 2024-11-13T16:34:17Z

src/sail_json_backend/json.ml

+  | Some [] -> None
+  | None -> None


These could be combined in a catch-all using "_".

ThinkOpenly · 2024-11-13T16:36:14Z

src/sail_json_backend/json.ml

+let map_param_to_arg id param args_list =
+  match Hashtbl.find_opt inputs id with
+  | Some inputl -> (
+      match get_index param inputl with Some index -> List.nth_opt args_list index | None -> None


Does dune fmt leave this on a single line? If possible, I'd prefer multiple lines here.
Nice use of List.nth_opt! 🙂

Yeah, that's dune fmt
Thanks : )

ThinkOpenly · 2024-11-13T16:40:23Z

src/sail_json_backend/json.ml

+let get_index elem lst =
+  List.find_map (fun (i, x) -> if x = elem then Some i else None) (List.mapi (fun i x -> (i, x)) lst)


Can you use List.find_index instead?

ThinkOpenly · 2024-11-13T17:03:02Z

src/sail_json_backend/json.ml

+      )
+      else None
+    )
+    (Hashtbl.find_all mappings (String.lowercase_ascii (id ^ "_mnemonic")))


This is relying on a convention used to identify mappings for mnemonics in the Sail code. Since this is not guaranteed, please add a comment indicating assumptions you are making here.

Also, I see similar conventions which likely violate the assumptions, like:

mapping f_madd_type_mnemonic_D : f_madd_op_D <-> string = { FMADD_D <-> "fmadd.d", FMSUB_D <-> "fmsub.d", FNMSUB_D <-> "fnmsub.d", FNMADD_D <-> "fnmadd.d" }

Would it work to use all entries in the "mappings" table?

Decided to go the all entries route for this

Move a misplaced line in `parse_funcl` for logical flow. Replace `List.iteri` with `List.iter` for function appropriateness, as index is not used.

Update `get_index` to use `List.find_index`, simplifying the implementation while maintaining functionality.

Replace the key concatenation assumption with a fold over the entire `mappings` Hashtbl, ensuring correctness across all cases.

Linda-Njau added 6 commits November 12, 2024 15:41

Add function to process base instructions

783185a

`process_base_instruction` iterates over `base_instructions` Hashtbl and calls `get_mnemonic` for each entry to retrieve its mnemonic.

Add function to return index of an element in a list

a81da7f

Add function to retrieve mnemonic for a given argument

49dd15f

`map_arg_to_mnemonic` looks up a list of enum_mnemonic pairs in the `mappings` Hashtbl. It then matches the provided `arg` with the enum and returns the corresponding mnemonic. ``` Matched RISCV_ADDI with mnemonic: addi ```

ThinkOpenly requested changes Nov 13, 2024

View reviewed changes

Linda-Njau added 5 commits November 21, 2024 16:12

Fix misplaced line and adjust list iteration function

20cd11c

Move a misplaced line in `parse_funcl` for logical flow. Replace `List.iteri` with `List.iter` for function appropriateness, as index is not used.

Add variable for regex pattern matching the string in parentheses

eb4611c

Refactor match expressions to use a catch-all case

35af3fd

Refactor index search function

b95e0f2

Update `get_index` to use `List.find_index`, simplifying the implementation while maintaining functionality.

Refactor map_arg_to_mnemonic to use direct iteration

67fa077

Replace the key concatenation assumption with a fold over the entire `mappings` Hashtbl, ensuring correctness across all cases.

Linda-Njau force-pushed the pseudoinstructions branch from 50e5fc4 to 67fa077 Compare November 21, 2024 13:35

Remove pkg-config from macOS dependencies

e2799d9

Linda-Njau force-pushed the pseudoinstructions branch from 6369a77 to e2799d9 Compare November 26, 2024 12:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add mnemonic mapping for pseudoinstructions #45

Add mnemonic mapping for pseudoinstructions #45

Linda-Njau commented Nov 12, 2024

ThinkOpenly left a comment

ThinkOpenly Nov 12, 2024

Linda-Njau Nov 19, 2024

ThinkOpenly Nov 19, 2024

Linda-Njau Nov 19, 2024

ThinkOpenly Nov 12, 2024

ThinkOpenly Nov 13, 2024

ThinkOpenly Nov 13, 2024

ThinkOpenly Nov 13, 2024

ThinkOpenly Nov 13, 2024

Linda-Njau Nov 21, 2024

ThinkOpenly Nov 13, 2024

ThinkOpenly Nov 13, 2024

Linda-Njau Nov 21, 2024

		let get_index elem lst =
		List.find_map (fun (i, x) -> if x = elem then Some i else None) (List.mapi (fun i x -> (i, x)) lst)

Add mnemonic mapping for pseudoinstructions #45

Are you sure you want to change the base?

Add mnemonic mapping for pseudoinstructions #45

Conversation

Linda-Njau commented Nov 12, 2024

ThinkOpenly left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment