updating opcodes of riscv-p-spec - v0.9.11 (c3409c8)

[eflaner]

• Reorganized 'p' into sub-extensions as described here
  • created zpn, zpfs & zbpbo (along with appropriate 32 & 64 variants)
  • removed rv_p, rv64_p ( according to RISC-V sub-extension policy -- it's invalid )
• zbpbo
  • Replaced bitrevi with grevi)
  • Replaced BPICK with CMIX
• zpsf (zpsfoperand)
  • created zpsf as specified in specification
  • SMMUL replacement in RV32 with MULH
  • MAXW and MINW removed (replaced with MAX and MIN of Zbb respectively)
  • Removed SWAP8 (replaced with REV8.H)
  • Removed WEXT and WEXTI (replaced with FSR and FSRI respectively)
• zpn
  • Removed CLO* variants
  • Seperated INSB, SRAI.u instructions into RV32 and RV64 equivalents
  • PKBB32, PKTT32 removed (Replaced by PACK and PACKU of Zbp respectively)
  • PKTT16 and PKBB16 restricted to RV64
  • Added CLROV and RDOV

[neelgala]

LGTM
@aswaterman could you enable the workflow on this PR ? if they pass I think it can be merged.

[aswaterman]

Yeah, thanks for reviewing, @neelgala

[aswaterman]

@eflaner Did you ever make a Spike PR associated with these changes? Spike no longer builds because it depends on the opcode definitions for the removed instructions like PKBB32.

Without a corresponding Spike PR, I'll have to revert this PR, since we can't make progress on other extensions in Spike in the meantime.

[aswaterman]

@scottj97 Do you think we should augment riscv-opcodes' CI flow to make sure that the HEAD of master on riscv-isa-sim can build with the generated encoding.h? It couples these repos awkwardly, but it would prevent this kind of breakage going forward.

For a change like this one, which both adds and deletes opcodes, we'd either need to stage it as multiple PRs, or allow the corresponding Spike PR to be merged before rerunning CI over here. Fortunately, this kind of PR is not the common case.

[scottj97]

@scottj97 Do you think we should augment riscv-opcodes' CI flow to make sure that the HEAD of master on riscv-isa-sim can build with the generated encoding.h? It couples these repos awkwardly, but it would prevent this kind of breakage going forward.

Fortunately this is rare, and I think we have a suitable workaround for this one.

For a change like this one, which both adds and deletes opcodes, we'd either need to stage it as multiple PRs, or allow the corresponding Spike PR to be merged before rerunning CI over here. Fortunately, this kind of PR is not the common case.

I don't think Spike should commit an encoding.h to its master if the source for that file is not yet in master here. We don't want a rebased PR here to invalidate the hashes that are permanently committed into Spike's history.

[aswaterman]

Reasonable.

[eflaner]

@aswaterman

@eflaner Did you ever make a Spike PR associated with these changes? Spike no longer builds because it depends on the opcode definitions for the removed instructions like PKBB32.

No PR was created in Spike for these changes. Neither am i aware of spike internals to make it compliant with 'latest' spike.

Without a corresponding Spike PR, I'll have to revert this PR, since we can't make progress on other extensions in Spike in the meantime.

Hmm, okay, i could see the #147 is excluded using the #157.
IDK whether this is right place to ask - what should be done to fix the spike build failure.

on that note, which all repos (like spike) should be fixed before a PR is made in riscv-opcodes ?

[aswaterman]

The general rule is that Spike must build against the output of this repository.

Normally, adding new functionality to this repository poses no issues. The problems arise when existing functionality is deleted.

Given there is no apparent plan to update Spike in accordance to #147 etc., we will revert those changes. We'll happily re-accept them once there is a plan to make them work with Spike.