-
Notifications
You must be signed in to change notification settings - Fork 307
Adds detection and reporting for ISA extensions that are implemented but have no selected tests. #1315
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
base: act4
Are you sure you want to change the base?
Adds detection and reporting for ISA extensions that are implemented but have no selected tests. #1315
Changes from 3 commits
1123a3f
7e02285
7f49b14
fc11345
4d9eac5
d7ccfcb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,7 +22,7 @@ | |
| from act.coverreport import print_coverage_summary | ||
| from act.parse_test_constraints import TestYamlHeaderError, generate_test_dict | ||
| from act.parse_udb_config import generate_udb_files, get_config_params, get_implemented_extensions | ||
| from act.select_tests import select_tests | ||
| from act.select_tests import get_untested_implemented_extensions, select_tests | ||
|
|
||
| # CLI interface setup | ||
| act_app = typer.Typer(context_settings={"help_option_names": ["-h", "--help"]}) | ||
|
|
@@ -100,6 +100,7 @@ def run_act( | |
|
|
||
| config_names: list[str] = [] | ||
| tasks: list[BuildTask] = [] | ||
| excluded_extensions = {ext.strip() for ext in exclude.split(",") if ext.strip()} | ||
| for config_file in config_files: | ||
| # Load configuration | ||
| config = load_config(config_file) | ||
|
|
@@ -115,6 +116,20 @@ def run_act( | |
| selected_tests = select_tests( | ||
| full_test_dict, implemented_extensions, config_params, include_priv_tests=config.include_priv_tests | ||
| ) | ||
| if extensions == "all": | ||
| untested_extensions = get_untested_implemented_extensions( | ||
| selected_tests, | ||
| implemented_extensions, | ||
| include_priv_tests=config.include_priv_tests, | ||
| excluded_extensions=excluded_extensions, | ||
| ) | ||
| if untested_extensions: | ||
| print( | ||
| f"Warning: {config.name}: no applicable tests selected for implemented extension(s): " | ||
| f"{', '.join(untested_extensions)}. Tests may be missing, excluded, or " | ||
| "otherwise inapplicable for the current configuration.", | ||
| file=sys.stderr, | ||
| ) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we want to limit this to just when |
||
| mxlen = config_params["MXLEN"] | ||
| if not isinstance(mxlen, int): | ||
| raise TypeError(f"MXLEN must be an integer, got {type(mxlen)}: {mxlen!r}") | ||
|
Comment on lines
103
to
125
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
excluded_extensionsis derived from the CLI--excludestring, whichgenerate_test_dict()applies as a test directory filter (it compares againsttest_file.parent.name, e.g.Sv). Passing this set intoget_untested_implemented_extensions()(which compares against ISA extension tokens likeSv39,Zba, etc.) means excludes likeSvwon’t suppress warnings for implementedSv32/Sv39/..., leading to false-positive warnings. Consider translating excluded directory names into ISA extension names (e.g., treat an excluded prefix likeSvas excluding any implemented extension starting withSv), or only applying this filter when the excluded value is known to be in the same namespace asimplemented_extensions.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot apply changes based on this feedback
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like that didn't work.