-
Notifications
You must be signed in to change notification settings - Fork 468
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
[Debug] why are Spike target ISA/architecture specifiers inconsistent? #568
Comments
Yes and no :) . It's a complicated topic. We can go on case-by-case basis. The thing is that there is lot's of configuration options that affect OpenOCD behavior. I guess that the diversity in configuration is not a bad thing. For example in Syntacore we have like 20 configurations based on spike to test OpenOCD. I doubt that we need such diversity in open source, though. That being said I guess that these configurations could use better naming instead of general "spike32"/"spike64" and the like. It also does not help that aside from ISA string we have difference in non-isa stuff (like hasel) which is not reflected in any way in platform description. Changing this naming scheme will cause compatibility problems for users of this testsuite. If there is a need or desire to switch to a new naming scheme I would prefer to get an explicit approval from @aswaterman before addressing this. Now, going case-by-case.
C, D, V extensions affect how OpenOCD behaves in certain scenarios. So having platforms that toggle this support help to increase coverage. Obviously, some care is needed to cherry-pick only "inserting" combinations (or you get a
I guess the same sitation. My guess (never bothered to check) that the ones that you list as "None" are derived from misa directly. In the end it's up to the users to decide which combinations they need. If someone wants to have a specific combination to be tested - they can create MR. TLDR:
|
I don't object to renaming these targets. I leave the decision to y'all. But note that Spike does rely on their names in its CI flow: https://github.com/riscv-software-src/riscv-isa-sim/blob/1b1a333763eae2e74dbf38b39d9adab39c4bed7c/.github/workflows/debug-smoke.yml So, if we do rename them, please make a corresponding PR to Spike that updates the |
I noticed this when dealing with this:
targets/RISC-V/spike32.py
tests failing riscv-collab/riscv-openocd#1098The Spike targets for debugging use different ISAs/architecture specifications:
riscv-tests/debug/targets/RISC-V/spike-multi.py
Line 26 in b0eb63a
riscv-tests/debug/targets/RISC-V/spike32-2-hwthread.py
Line 16 in b0eb63a
riscv-tests/debug/targets/RISC-V/spike32-2.py
Line 15 in b0eb63a
riscv-tests/debug/targets/RISC-V/spike32.py
Line 24 in b0eb63a
riscv-tests/debug/targets/RISC-V/spike64-2-hwthread.py
Line 17 in b0eb63a
riscv-tests/debug/targets/RISC-V/spike64.py
Line 24 in b0eb63a
Is there a reason for the lack of consistency here?
Should there be more consistency?
The text was updated successfully, but these errors were encountered: