cv32e40p
cv32e40p copied to clipboard
openhwgroup
ISA Change Spec for HWLoop
There are three requests for changing the HWLoop ISA spec
-
#189 Removing the possibility to write to HWLoop CSRs with CSR instructions, and only using HWLoop instructions. This requires to add two instructions lp.start and lp.end that use a register as source instead of an immediate.
-
#583 Forcing the word-alignment of HWLoop by shifting by 2 instead of by 1 the immediate value in the lp.starti and lp.endi instructions. Otherwise one bit is wasted. In case that is not accepted as change, an illegal check should be done when the LSB is not 0. Same for point 1 above, the illegal check should be added for the first 2 btis of the register in case the ISA change gets accepted.
-
https://github.com/openhwgroup/core-v-docs/issues/265 The RTL raises an illegal condition when jumps, branches, mret and dret, wfi, ecall and fences are executed inside a HWLoop. The Documentation lacks of this. Thus it is still an ISA change
Please, let's discuss here about these changes @jm4rtin , @jeremybennett , @eflamand
These have impacts on the compiler team - the current spec is already in core-v-binutils-gdb. We need a process to manage ISA changes that break software. In this case the alignment and change of shift mean the assembler is now broken.
I suggest there are three things to do.
-
We need a process to make sure software TG signs off on any changes that affect the SW world - something to raise with Duncan Bees
-
ISA changes need to be formally discussed at Cores and SW TG meetings - possibly a combined meeting for efficiency. There is a versioning mechanism for ISAs, which will need to be used by the software. So instead of
-march=Xcorev(or-march=Xcorevhwlp) we would have-march=Xcorev1p1(or-march=Xcorevhwlp2). -
In this case, engineering time has been committed to implementing the tools for the current spec. We need buy in from the GNU tools project to modify that work. Which would need sign of from TWG if it is a significant change (it isn't in this case).
Yes, creating a process to change the ISA for the CORE-V extension is a good idea; however, I'm not aware of a ratified, finalized, set in stone specification for the ISA. It may be premature to start incrementing an ISA version. There is the description of the instructions in core-v-docs that doesn't agree with the RTL design and neither that documentation or the RTL agree with the proposed (but not finalized) re-encoding of the instructions (#452). As far as I know, the encoding from #452 is being used by the GNU/LLVM toolchains.
@jeremybennett What specification is used by binutils?
@davideschiavone The encoding (#452) is still an open issue and might need to be considered part of this discussion?
@jm4rtin Yes my point was really about process. For now everything is in development (on the development branch in the corev-binutils-gdb repo).
We are working from the core-v-docs specification. We have some edits which should land as a pull-request tomorrow to clarify some details from the assembler perspective. For now we are using the existing PULP encodings, but are set up to change for the new RISC-V compliant encodings when you are ready.
At present corev-binutils-gdb has support for hardware loop and MAC extensions. General ALU extensions should be in by the end of the week. @jessicamills is coordinating this as GNU Tools Project Lead. We also now have hardware loop support in the LLVM integrated assembler in corev-llvm-project (thanks to @flip1995).
We have a lot of GCC support complete, but are waiting on a suitable test target to verify the changes. We need a target with GDB support. Unfortunately with everyone dispersed around the country, setting up FPGA boards is far from ideal, so we are waiting on me getting the Verilator model up and running with GDB.
@jeremybennett At what point would the ISA need to be frozen to make it into binutils/GCC? From my understanding it must be merged in with the new instruction encoding.
There is no rush. We've missed Stage 3 for GCC 11, so we have a year. The delay is not due to ISA freeze, the issue of vendor specific relocations needs to be solved by RISC-V International first.
The -march options for GCC and LLVM have versioning, so it is possible to support multiple versions of ISAs, but it would be much cleaner to have something stable first time round.
In my opinion, the XPULP ISA should be frozen ASAP as this may gate the TG SW team. As @jm4rtin said, #452 is still an open issue, and that is the greatest of the issue to be solved. The ones related to the HWLoop are minor changes (not even necessary, just cleaner from both ISA and HW point of view), and fortunatly, we figured out that this is not a major change in the compilers.
However, in the process of solving #452 , other changes may be required (like dropping some instructions due to lack of space or whatever else), so after the TGW meeting on monday, our (Core TG) priority should be #452 together with the RTL freeze per se.
Do you agree? @jm4rtin , @jeremybennett
le'ts bring this up on monday during the call
Stability will help the SW team - it just means we don't end up redoing work. But if the architecture needs to evolve we can handle it. The important thing is to understand the cost of such changes for the software team.
If I understand it correctly then the only real hard constraint from the sw point of view is that after upstreaming to gcc the ABI is frozen.
That is in essence the issue around vendor relocations. The RISC-V PS-ABI defines relocations 192-255 for vendor use. However there is no mechanism for how we avoid clashes between different vendors. This is an engineering issue for the various tool chains, but needs to be solved to allow the PS-ABI to be used in practice.
CORE-V will use the standard RISC-V PS-ABI. We just need to clarify this area.
Hi @davideschiavone Yes we can.
And remove Component:RTL as only one should be present at a time?
@pascalgouedo , this only applies to PRs :) issue can target both the labes