Implement Zimop and Zcmop Extensions

[jordancarlin]

Extracted and updated the Zimop and Zcmop extensions from #377.

One of the tricky parts with this extension is that it is designed to have its instructions overridden by other extensions (like Zicfiss). The implementation in #377 had a function dedicated to Zimop and Zcmop in each of their files that listed which specific instructions were overridden. I'd rather avoid the need to modify this file each time a new extension is added that reuses one of its encodings, so I added new scattered encdec_overrides and encdec_compressed_overrides mappings for this purpose. These mappings are checked first, and only if there is no match does it check the normal encdec mappings. This allows each new extension (like Zicfiss and Zicflp) to be self-contained. It also opens the door to properly supporting hints from extensions like Zicbop and Zihintpause. While the hints don't do anything architecturally, having the correct assembly in the log would still be beneficial.

This new mapping can be split out into a separate PR, but I'm introducing it here to clarify the motivation behind it.

[github-actions]

Test Results

392 tests  ±0   392 ✅ ±0   1m 21s ⏱️ -1s
  1 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit 6fec913. ± Comparison against base commit 80c4928.

♻️ This comment has been updated with latest results.

[Timmmm]

Makes sense. It's a bit hard to follow the flow though... What if you have:

• encdec_hints: Just the mappings for hints. Doesn't chain to any other mappings.
• encdec: Leave this completely unmodified.
• encdec_with_hints:

mapping encdec_with_hints : bits(32) -> ast = {
    encdec_hints(s) <-> s
    encdec(s) <-> s
}

I think that's a lot easier to follow.

[jrtc27]

Why do we need all this rather than just having multiple encdec clauses in decreasing precedence order, i.e. compiling Zicfiss before Zimop?

[jordancarlin]

Makes sense. It's a bit hard to follow the flow though... What if you have:

• encdec_hints: Just the mappings for hints. Doesn't chain to any other mappings.
• encdec: Leave this completely unmodified.
• encdec_with_hints:

mapping encdec_with_hints : bits(32) -> ast = {
    encdec_hints(s) <-> s
    encdec(s) <-> s
}

I think that's a lot easier to follow.

Good suggestion. That is definitely easier to follow. I think I still prefer encdec_overrides as the name since things like Zicfiss which reuse the Zimop encoding space are explicitly described in the ISA manual and NOT hints.

[jordancarlin]

Why do we need all this rather than just having multiple encdec clauses in decreasing precedence order, i.e. compiling Zicfiss before Zimop?

See previous related discussion here: #538 (comment). The general sense is having different results for the model based on the compilation order seems unnecessarily fragile. Especially as we (hopefully) move to the new module system that makes it easier than ever to include or exclude various files from compilation.

[jordancarlin]

Makes sense. It's a bit hard to follow the flow though... What if you have:

• encdec_hints: Just the mappings for hints. Doesn't chain to any other mappings.
• encdec: Leave this completely unmodified.
• encdec_with_hints:

mapping encdec_with_hints : bits(32) -> ast = {
    encdec_hints(s) <-> s
    encdec(s) <-> s
}

I think that's a lot easier to follow.

Good suggestion. That is definitely easier to follow. I think I still prefer encdec_overrides as the name since things like Zicfiss which reuse the Zimop encoding space are explicitly described in the ISA manual and NOT hints.

Hmm. That approach may actually not work after all. Somewhat surprisingly, the error messages come about in the C compilation after sail itself is done.

[4/6] Linking C executable c_emulator/riscv_sim_rv32d
FAILED: c_emulator/riscv_sim_rv32d 
: && /opt/rh/gcc-toolset-13/root/usr/bin/cc -O2 -g -DNDEBUG -Wl,--no-undefined c_emulator/CMakeFiles/riscv_sim_rv32d.dir/__/riscv_model_rv32d.c.o c_emulator/CMakeFiles/riscv_sim_rv32d.dir/riscv_platform.c.o c_emulator/CMakeFiles/riscv_sim_rv32d.dir/riscv_platform_impl.c.o c_emulator/CMakeFiles/riscv_sim_rv32d.dir/riscv_prelude.c.o c_emulator/CMakeFiles/riscv_sim_rv32d.dir/riscv_sim.c.o c_emulator/CMakeFiles/riscv_sim_rv32d.dir/riscv_softfloat.c.o -o c_emulator/riscv_sim_rv32d  dependencies/softfloat/libsoftfloat.a  sail_runtime/libsail_runtime.a  /usr/lib64/libgmp.so  /usr/lib64/libz.so && :
/opt/rh/gcc-toolset-13/root/usr/libexec/gcc/x86_64-redhat-linux/13/ld: c_emulator/CMakeFiles/riscv_sim_rv32d.dir/__/riscv_model_rv32d.c.o: in function `zencdec_with_overrides_forwards':
/home/jcarlin/riscv-projects/sail-riscv/build/riscv_model_rv32d.c:197075: undefined reference to `zencdec_overrides_backwards_matches'
/opt/rh/gcc-toolset-13/root/usr/libexec/gcc/x86_64-redhat-linux/13/ld: /home/jcarlin/riscv-projects/sail-riscv/build/riscv_model_rv32d.c:197082: undefined reference to `zencdec_overrides_backwards'
/opt/rh/gcc-toolset-13/root/usr/libexec/gcc/x86_64-redhat-linux/13/ld: c_emulator/CMakeFiles/riscv_sim_rv32d.dir/__/riscv_model_rv32d.c.o: in function `zencdec_with_overrides_forwards_infallible':
/home/jcarlin/riscv-projects/sail-riscv/build/riscv_model_rv32d.c:198005: undefined reference to `zencdec_overrides_backwards_matches'
/opt/rh/gcc-toolset-13/root/usr/libexec/gcc/x86_64-redhat-linux/13/ld: /home/jcarlin/riscv-projects/sail-riscv/build/riscv_model_rv32d.c:198012: undefined reference to `zencdec_overrides_backwards'
/opt/rh/gcc-toolset-13/root/usr/libexec/gcc/x86_64-redhat-linux/13/ld: c_emulator/CMakeFiles/riscv_sim_rv32d.dir/__/riscv_model_rv32d.c.o: in function `zencdec_compressed_with_overrides_forwards':
/home/jcarlin/riscv-projects/sail-riscv/build/riscv_model_rv32d.c:208184: undefined reference to `zencdec_compressed_overrides_backwards_matches'
/opt/rh/gcc-toolset-13/root/usr/libexec/gcc/x86_64-redhat-linux/13/ld: /home/jcarlin/riscv-projects/sail-riscv/build/riscv_model_rv32d.c:208191: undefined reference to `zencdec_compressed_overrides_backwards'
/opt/rh/gcc-toolset-13/root/usr/libexec/gcc/x86_64-redhat-linux/13/ld: c_emulator/CMakeFiles/riscv_sim_rv32d.dir/__/riscv_model_rv32d.c.o: in function `zencdec_compressed_with_overrides_forwards_infallible':
/home/jcarlin/riscv-projects/sail-riscv/build/riscv_model_rv32d.c:209114: undefined reference to `zencdec_compressed_overrides_backwards_matches'
/opt/rh/gcc-toolset-13/root/usr/libexec/gcc/x86_64-redhat-linux/13/ld: /home/jcarlin/riscv-projects/sail-riscv/build/riscv_model_rv32d.c:209121: undefined reference to `zencdec_compressed_overrides_backwards'
collect2: error: ld returned 1 exit status
[5/6] Building C object c_emulator/CMakeFiles/riscv_sim_rv64d.dir/__/riscv_model_rv64d.c.o
ninja: build stopped: subcommand failed.

Seems like it's not able to come up with a backwards function for the mapping. I'll see if I can manually define one that will work, but it may end up being cleaner to have the tiered mappings that are in the current version.

[Timmmm]

Hmm could be a Sail compiler bug? Can you make a small reproducible example?

Also we may be able to work around it with --c-preserve zencdec... all of those missing functions?

[Alasdair]

We shouldn't produce any broken C code for Sail code that type-checks, so certainly a bug. Looks like the tree-shaking that we do to prune unused definitions is removing things it shouldn't in this case, I'll try to create a smaller test case. I recently introduced an optimisation that introduces _infalliable versions of mappings for certain optimisations and it might be that they aren't being tracked correctly.

The original idea was to use the order of files to control this. The module system I've been working on makes this more explicit, as you can write explicitly after X or before Y in a module Z to control the ordering of scattered definitions.

[jordancarlin]

Yeah. Seems like if the scattered mapping is empty (which the overrides mappings are in this PR are), then Sail optimizes them away but still leaves the call to them in the other mapping that calls the (now pruned) scattered mapping. I can create a smaller test case if that would be helpful.

[kourzanov]

Isn't this overriding better solved by injecting overrides into the generated C zdecode/zexecute?

[jordancarlin]

Per the TGMM today, we're going to skip the overrides and rely on the upcoming module system that adds support for explicitly declaring module precedence. This is a good balance between the slightly unreliable pure file ordering and the slightly convoluted nested mappings.

[jordancarlin]

Removed the changes related to overrides. Since we are going to go with the module system, nothing related to that needs to go into this PR and this should be good to review from a purely Zimop/Zcmop perspective. The modules only become relevant when we are ready to implement extensions like Zicfiss.

[arichardson]

Not looked at the spec but seems reasonable overall

[jordancarlin]

This either needs the dec_bits.sail library file to be included locally for now or wait until the next sail release. Unless there is a high degree of urgency to get it in, I'd vote for waiting since there should be a new release soon.