Skip to content
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

Brian opcodes #449

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
Draft

Brian opcodes #449

wants to merge 12 commits into from

Conversation

AFOliveira
Copy link
Collaborator

I integrated @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 was looking for if I am not in error. I added a README.md for detailed instructions.

@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.

@AFOliveira AFOliveira requested a review from ayosher January 30, 2025 13:27
@ThinkOpenly
Copy link
Collaborator

Before you merge, I ask that you change:

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

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

@AFOliveira
Copy link
Collaborator Author

@ThinkOpenly Ups! I'll do so :)

Copy link
Collaborator

@dhower-qc dhower-qc left a comment

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
Collaborator 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

…he UDB with riscv-opcodes generation process to generate riscv-opcodes outputs.

Signed-off-by: Afonso Oliveira <[email protected]>
@dhower-qc
Copy link
Collaborator

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
Collaborator Author

@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
Collaborator

ayosher commented Feb 3, 2025

I integrated @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 was looking for if I am not in error. I added a README.md for detailed instructions.

@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
Collaborator Author

@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
Collaborator

@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
Collaborator

ayosher commented Feb 3, 2025

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

@AFOliveira
Copy link
Collaborator Author

AFOliveira commented Feb 3, 2025

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. @dhower-qc any thoughts on it?

@ayosher
Copy link
Collaborator

ayosher commented Feb 3, 2025

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
Collaborator

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

@AFOliveira
Copy link
Collaborator Author

@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 <[email protected]>
@AFOliveira
Copy link
Collaborator 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
Collaborator

ayosher commented Feb 3, 2025

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
Collaborator Author

@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
Collaborator

ayosher commented Feb 3, 2025

@AFOliveira - now it worked, thank you!

@AFOliveira
Copy link
Collaborator Author

@AFOliveira - now it worked, thank you!

Great!

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.

5 participants