Skip to content

RISC-V Opcodes Data Cross-Validation - #449

Closed
Afonso Oliveira (AFOliveira) wants to merge 35 commits into
mainfrom
BrianOpcodes
Closed

RISC-V Opcodes Data Cross-Validation#449
Afonso Oliveira (AFOliveira) wants to merge 35 commits into
mainfrom
BrianOpcodes

Conversation

@AFOliveira

Copy link
Copy Markdown
Member

I integrated Brian (@BrianAnakPintar) with normal UDB directory for this. I also added the generation of riscv-opcodes outputs, such as encodings.out.h that is what ayosher (@ayosher) was looking for if I am not in error. I added a README.md for detailed instructions.

ayosher (@ayosher) Until next meeting I won't have enough bandwidth to test this with SPIKE and Qualcomm new extensions, but to do so, you should just go to the folder /ext/opcodes_maker and run make YAML_DIR=../../cfgs/qc_iu/arch_overlay/inst/ and look for output/encoding.out.h as an input for SPIKE.

@ThinkOpenly

Copy link
Copy Markdown
Collaborator

Before you merge, I ask that you change:

Integrate Brain branch ...
to
"Integrate Brian branch ..."

Although, Brian (@BrianAnakPintar) is a really smart guy. ;-)

@AFOliveira

Copy link
Copy Markdown
Member Author

Paul Clarke (@ThinkOpenly) Ups! I'll do so :)

@dhower-qc Derek Hower (dhower-qc) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Can we move this to backends/ rather than ext/?
  • Did you mean to include instr_dict.json? If so, we should figure out how to make sure the checked-in version is always consistent with the state of the database.

@AFOliveira

Copy link
Copy Markdown
Member Author
  1. Sure!
  2. If I did it was by accident. Default Path takes us to arch/inst and configurable user path can use whatever folder, like cfgs/qc_ui/inst

Afonso Oliveira added 4 commits February 3, 2025 11:57
…he UDB with riscv-opcodes generation process to generate riscv-opcodes outputs.

Signed-off-by: Afonso Oliveira <Afonso.Oliveira@synopsys.com>
Signed-off-by: Afonso Oliveira <Afonso.Oliveira@synopsys.com>
…s qc_iu

Signed-off-by: Afonso Oliveira <Afonso.Oliveira@synopsys.com>
Signed-off-by: Afonso Oliveira <Afonso.Oliveira@synopsys.com>
Signed-off-by: Afonso Oliveira <Afonso.Oliveira@synopsys.com>
@dhower-qc

Copy link
Copy Markdown
Contributor

Cool. Now let's just make sure this gets added to the smoke tests. Currently, that takes two changes:

  • Update test:smoke task in Rakefile (for local runs)
  • Update .github/workflows/regress.yml (for GitHub runs)

Now that you've told me there is a tool to run GitHub workflows locally, we should be able to get rid of step 1 eventually...

@AFOliveira

Copy link
Copy Markdown
Member Author

Derek Hower (@dhower-qc) This is only generating riscv-opcodes outputs, therefore, shouldn't this be a separate task and not be inside the smoke test? This does not test anything, only generates. Probably I could put it into some gen:opcodes_outputs task?

@ayosher

Copy link
Copy Markdown
Contributor

I integrated Brian (@BrianAnakPintar) with normal UDB directory for this. I also added the generation of riscv-opcodes outputs, such as encodings.out.h that is what ayosher (@ayosher) was looking for if I am not in error. I added a README.md for detailed instructions.

ayosher (@ayosher) Until next meeting I won't have enough bandwidth to test this with SPIKE and Qualcomm new extensions, but to do so, you should just go to the folder /ext/opcodes_maker and run make YAML_DIR=../../cfgs/qc_iu/arch_overlay/inst/ and look for output/encoding.out.h as an input for SPIKE.

I will try to run it. I hope it is merged with latest main branch to include all fixes to Xqci/Xqccmp. Now dealing with building environment to work on Spike, will take few days. Would be great to see this pull request in main...

@AFOliveira

Copy link
Copy Markdown
Member Author

ayosher (@ayosher) Just merged this with main to ensure you have the most recent version.
There is an FYI to be made that UDB and riscv-opcodes don't describe all instructions in the same manner, so some parts of the output are different from what Spike usually intakes, this is more related to the V extension so it likely won't affect your use case. There is need for a bit of discussion on this, so I'll bring it up in the next meeting.

@dhower-qc

Copy link
Copy Markdown
Contributor

Derek Hower (@dhower-qc) This is only generating riscv-opcodes outputs, therefore, shouldn't this be a separate task and not be inside the smoke test? This does not test anything, only generates. Probably I could put it into some gen:opcodes_outputs task?

Sorry, I meant test:regress 😄

@ayosher

Copy link
Copy Markdown
Contributor

Should I expect down the road this generation to be integrated in some "do" task?

@AFOliveira

Afonso Oliveira (AFOliveira) commented Feb 3, 2025

Copy link
Copy Markdown
Member Author

Should I expect down the road this generation to be integrated in some "do" task?

That is what I was discussing with Derek. I would say yes, this is an output as many others are; however, I'm not entitled to answer it with certainty by myself. Derek Hower (@dhower-qc) any thoughts on it?

@ayosher

Copy link
Copy Markdown
Contributor

I tried to generate opcodes for qc_iu/Xqci, faced the following error message:
Successfully processed 157 YAML files Output written to: output/instr_dict.json python3 generator.py output/instr_dict.json -c -chisel -spinalhdl -sverilog -rust -go -latex Traceback (most recent call last): File "/quic/riscv-unified-db/backends/opcodes_maker/generator.py", line 33, in <module> from c_utils import make_c ModuleNotFoundError: No module named 'c_utils' make: *** [Makefile:38: generate] Error 1

@dhower-qc

Copy link
Copy Markdown
Contributor

absolutely. we should have something like do gen:opcode_header.

@AFOliveira

Copy link
Copy Markdown
Member Author

ayosher (@ayosher) riscv-opcodes was not updated in remote branch. My bad. There should not be any errors now.

…uts YAML_DIR=optional_path

Signed-off-by: Afonso Oliveira <Afonso.Oliveira@synopsys.com>
@AFOliveira

Copy link
Copy Markdown
Member Author

Updated Rakefile. Example usage:

./do gen:opcode_outputs YAML_DIR=cfgs/qc_iu/arch_overlay/inst/

YAML_DIR is optional and can be left blank to default to normal arch/inst. I'll add to regression test later.

@ayosher

Copy link
Copy Markdown
Contributor

Updated Rakefile. Example usage:

./do gen:opcode_outputs YAML_DIR=cfgs/qc_iu/arch_overlay/inst/

YAML_DIR is optional and can be left blank to default to normal arch/inst. I'll add to regression test later.

Tried that. I still have the same issue. Now with ./do way.
I believe I lack some package somewhere - but I have no idea which one...

Successfully processed 157 YAML files
Output written to: /quic/riscv-unified-db/gen/opcodes_outputs/instr_dict.json
/quic/riscv-unified-db/.home/.venv/bin/python3 /quic/riscv-unified-db/backends/opcodes_maker/generator.py /quic/riscv-unified-db/gen/opcodes_outputs/instr_dict.json -c -chisel -spinalhdl -sverilog -rust -go -latex
Traceback (most recent call last):
File "/quic/riscv-unified-db/backends/opcodes_maker/generator.py", line 33, in
from c_utils import make_c
ModuleNotFoundError: No module named 'c_utils'
/quic/riscv-unified-db/.home/.gems/ruby/3.2.0/gems/rake-13.2.1/lib/rake/file_utils.rb:65:in block in create_shell_runner': Command failed with status (1): [/quic/riscv-unified-db/.home/.venv/bin/python3 /quic/riscv-unified-db/backends/opcodes_maker/generator.py /quic/riscv-unified-db/gen/opcodes_outputs/instr_dict.json -c -chisel -spinalhdl -sverilog -rust -go -latex] (RuntimeError) from /quic/riscv-unified-db/.home/.gems/ruby/3.2.0/gems/rake-13.2.1/lib/rake/file_utils.rb:57:in sh'
from /quic/riscv-unified-db/Rakefile:49:in block (2 levels) in <top (required)>' from /quic/riscv-unified-db/.home/.gems/ruby/3.2.0/gems/rake-13.2.1/lib/rake/task.rb:281:in block in execute'
from /quic/riscv-unified-db/.home/.gems/ruby/3.2.0/gems/rake-13.2.1/lib/rake/task.rb:281:in each' from /quic/riscv-unified-db/.home/.gems/ruby/3.2.0/gems/rake-13.2.1/lib/rake/task.rb:281:in execute'
from /quic/riscv-unified-db/.home/.gems/ruby/3.2.0/gems/rake-13.2.1/lib/rake/task.rb:219:in block in invoke_with_call_chain' from /quic/riscv-unified-db/.home/.gems/ruby/3.2.0/gems/rake-13.2.1/lib/rake/task.rb:199:in synchronize'
from /quic/riscv-unified-db/.home/.gems/ruby/3.2.0/gems/rake-13.2.1/lib/rake/task.rb:199:in invoke_with_call_chain' from /quic/riscv-unified-db/.home/.gems/ruby/3.2.0/gems/rake-13.2.1/lib/rake/task.rb:188:in invoke'
from /quic/riscv-unified-db/.home/.gems/ruby/3.2.0/gems/rake-13.2.1/lib/rake/application.rb:188:in invoke_task' from /quic/riscv-unified-db/.home/.gems/ruby/3.2.0/gems/rake-13.2.1/lib/rake/application.rb:138:in block (2 levels) in top_level'
from /quic/riscv-unified-db/.home/.gems/ruby/3.2.0/gems/rake-13.2.1/lib/rake/application.rb:138:in each' from /quic/riscv-unified-db/.home/.gems/ruby/3.2.0/gems/rake-13.2.1/lib/rake/application.rb:138:in block in top_level'
from /quic/riscv-unified-db/.home/.gems/ruby/3.2.0/gems/rake-13.2.1/lib/rake/application.rb:147:in run_with_threads' from /quic/riscv-unified-db/.home/.gems/ruby/3.2.0/gems/rake-13.2.1/lib/rake/application.rb:132:in top_level'
from -e:1:in `

'

@AFOliveira

Copy link
Copy Markdown
Member Author

ayosher (@ayosher) sorry for the delay.

Tried that. I still have the same issue. Now with ./do way. I believe I lack some package somewhere - but I have no idea which one...

I still think its a riscv-opcodes problem, because it seems you can not import a file that did not exist in previous version, please try

git submodule update --init --recursive
git submodule update --remote ext/riscv-opcodes

Still, if it does appear the same error, try pip install pyyaml since it the only non-standard python package in the repo.

If it still does not work, please ping me again and so I can take a deeper look trying to figure it out.

@ayosher

Copy link
Copy Markdown
Contributor

Afonso Oliveira (@AFOliveira) - now it worked, thank you!

@AFOliveira

Copy link
Copy Markdown
Member Author

Afonso Oliveira (@AFOliveira) - now it worked, thank you!

Great!

Afonso Oliveira added 3 commits February 28, 2025 16:07
Signed-off-by: Afonso Oliveira <Afonso.Oliveira@synopsys.com>
Signed-off-by: Afonso Oliveira <Afonso.Oliveira@synopsys.com>
Signed-off-by: Afonso Oliveira <Afonso.Oliveira@synopsys.com>
Afonso Oliveira added 3 commits March 7, 2025 10:54
Signed-off-by: Afonso Oliveira <Afonso.Oliveira@synopsys.com>
Signed-off-by: Afonso Oliveira <Afonso.Oliveira@synopsys.com>
…a better way than looking into files

Signed-off-by: Afonso Oliveira <Afonso.Oliveira@synopsys.com>
@AFOliveira

Afonso Oliveira (AFOliveira) commented Mar 7, 2025

Copy link
Copy Markdown
Member Author

FYI, this is close to working, but C extensions has shown to be a pain - there's like 20+ operands that need to be further defined by hand, so expect this to take a while to get to 100%.

This is the point we are at:

============================ 64 failed, 1067 passed in 0.81s ============================

Signed-off-by: Afonso Oliveira <Afonso.Oliveira@synopsys.com>
@AFOliveira Afonso Oliveira (AFOliveira) changed the title Brian opcodes RISC-V Opcodes Data Cross-Validation Mar 12, 2025
Afonso Oliveira added 5 commits March 12, 2025 09:55
Signed-off-by: Afonso Oliveira <Afonso.Oliveira@synopsys.com>
Signed-off-by: Afonso Oliveira <Afonso.Oliveira@synopsys.com>
Signed-off-by: Afonso Oliveira <Afonso.Oliveira@synopsys.com>
Signed-off-by: Afonso Oliveira <Afonso.Oliveira@synopsys.com>
Signed-off-by: Afonso Oliveira <Afonso.Oliveira@synopsys.com>
Afonso Oliveira added 3 commits March 13, 2025 15:02
…t comparison of outputs.

Signed-off-by: Afonso Oliveira <Afonso.Oliveira@synopsys.com>
Afonso Oliveira added 2 commits March 19, 2025 18:03
…-opcodes.

Signed-off-by: Afonso Oliveira <Afonso.Oliveira@synopsys.com>
…f errors when UDB has more info than riscv-opcodes.

Signed-off-by: Afonso Oliveira <Afonso.Oliveira@synopsys.com>
@dhower-qc

Copy link
Copy Markdown
Contributor

Afonso Oliveira (@AFOliveira), how does this PR relate to #553 and #526 ?

@AFOliveira

Copy link
Copy Markdown
Member Author

Afonso Oliveira (@AFOliveira), how does this PR relate to #553 and #526 ?

Well, unfortunately, it does not at all. I realized that this task was way to challenging for a "sole" reason: RISC-V opcodes variable naming is some sort of black magic that is almost impossible to solve. This PR serves to show that we are in a comparable state in terms of instructions to what riscv-opcodes has, but I don't think we should integrate it into any workflow, since that there is no clean way reproducing that .json and leaves this code as a really nice spaghetti mess that we don't want to deal with in the future

@dhower-qc

Copy link
Copy Markdown
Contributor

Ok, should we close this then, or is there a value to keeping it open for the time being?

@AFOliveira

Copy link
Copy Markdown
Member Author

Closing it for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants