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

Add argparse #185

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

charlie-rivos
Copy link
Contributor

@charlie-rivos charlie-rivos commented Aug 4, 2023

Argparse will be helpful to make more complex changes to the parsing of command line arguments. This causes breaking changes to the parse.py API but the Makefile is appropriately updated.

I am not sure if this was intended, but logically it seems the "include_pseudo" flag should only include pseudo ops if it is True. The previous behavior was to include pseudo_ops even when the flag was False but the pseudo_op was not already included. Changing the flag to True would then force a pseudo_op to be included even if it was already included.

Argparse will be helpful to make more complex changes to the parsing of
command line arguments.

Signed-off-by: Charlie Jenkins <[email protected]>
@aswaterman aswaterman requested a review from neelgala August 4, 2023 23:22
@charlie-rivos
Copy link
Contributor Author

RISC-V Linux would like to use this repo for instruction creation but it seems like the repo is heavily geared toward SPIKE instead of being multipurpose for C. I am opening this pull request to start a conversation about supporting Linux.

Features that I will work on if it is acceptable is to allow more configuration from the command line. The standard on this repo appears to hardcode fixes in for every project that wants support, but I would rather not do that. I would like to have an option to specify the output file, selectively build non-instruction code like the CSRs, break off all of the SPIKE specific things from the -c flag so that Linux can also use the -c flag as well, and create a framework for auto generating immediates (which is currently hardcoded as a latex table).

These are a lot of changes that Linux would need to use the C generation and it seems like it would be beneficial to apply these to all targets rather than creating a custom Linux target.

@aswaterman
Copy link
Member

I think generalizing this repo is fine, so long as we make sure we don't break the existing use cases, and so long as we don't get into any licensing issues (e.g. no importing of GPL code).

Of course, we'll have to see exactly what changes would be required and agree that they're all within the scope of what we want to have to support as part of this project.

@charlie-rivos
Copy link
Contributor Author

charlie-rivos commented Aug 4, 2023

The main thing I am confused about is why the targets so heavily coupled with this repository. Is it expected that every new project hard codes in the way they want code to be generated?

@aswaterman
Copy link
Member

That’s historical. We don’t need that degree of tight coupling going forward: we can just emit collateral that people manually copy into the appropriate place.

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.

2 participants