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

Restructured codebase #316

Merged
merged 3 commits into from
Nov 12, 2024
Merged

Restructured codebase #316

merged 3 commits into from
Nov 12, 2024

Conversation

Myrausman
Copy link
Member

Restructured the codebase by moving all opcode extension files into a single 'extensions' folder, and updated paths in the code to reflect this new structure.

Signed-off-by: Myrausman <[email protected]>
Copy link

codecov bot commented Nov 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.53%. Comparing base (25c09e6) to head (383cbca).
Report is 20 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #316      +/-   ##
==========================================
+ Coverage   95.87%   96.53%   +0.65%     
==========================================
  Files          10       10              
  Lines         752      750       -2     
==========================================
+ Hits          721      724       +3     
+ Misses         31       26       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -457,7 +457,7 @@ def process_pseudo_instructions(
logging.debug(f"Processing pseudo line: {line}")
ext, orig_inst, pseudo_inst, line_content = pseudo_regex.findall(line)[0]
ext_file = find_extension_file(ext, opcodes_dir)

# print("ext_file",ext_file)
Copy link
Member

Choose a reason for hiding this comment

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

Delete

@@ -514,6 +514,7 @@ def find_extension_file(ext: str, opcodes_dir: str):
ext_file = f"{opcodes_dir}/unratified/{ext}"
if not os.path.exists(ext_file):
log_and_exit(f"Extension {ext} not found.")
# print(ext_file)
Copy link
Member

Choose a reason for hiding this comment

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

Delete

Makefile Outdated Show resolved Hide resolved
@IIITM-Jay
Copy link
Member

@aswaterman as I know that I suggested for how to use ../extensions but this also comes up with displaying in logger INFO as Extensions selected including path as well like below:

Running with args : ['./parse.py', '-c', '-go', '-chisel', '-sverilog', '-rust', '-latex', '-spinalhdl', '../extensions/rv*', '../extensions/unratified/rv*']
Extensions selected : ['../extensions/rv*', '../extensions/unratified/rv*']

But I think, the earlier is quite good in displaying as we are already stating on LHS as Extensions selected so again getting displayed as ../extension is unnecessary. With existing one, it is more cleaner I guess, that selected extensions are rv* and the unratified ones like below:

Running with args : ['./parse.py', '-c', '-go', '-chisel', '-sverilog', '-rust', '-latex', '-spinalhdl', 'rv*', 'unratified/rv*']
Extensions selected : ['rv*', 'unratified/rv*']

But again, whichever way you like, I think would be an appropriate one.

@aswaterman
Copy link
Member

I'll let you choose :) Let me know when it's ready to merge.

@IIITM-Jay
Copy link
Member

IIITM-Jay commented Nov 9, 2024

@aswaterman I have kept the earlier things itself as it is with no change in that as before, considering cleaner logger INFO and also made changes to have optimized pseudo logic using conditional operator for a more concise approach.

I think it could be merged now.

@IIITM-Jay IIITM-Jay mentioned this pull request Nov 9, 2024
Copy link
Member

@aswaterman aswaterman left a comment

Choose a reason for hiding this comment

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

SGTM.

@aswaterman aswaterman merged commit 4d8d260 into riscv:master Nov 12, 2024
12 checks passed
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.

3 participants