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

Remove redundant CSR privilege checks #497

Merged
merged 1 commit into from
Jul 11, 2024

Conversation

Timmmm
Copy link
Collaborator

@Timmmm Timmmm commented Jun 18, 2024

These mode levels are already checked by csrPriv() so there's no need to do it again. Removing these checks from is_CSR_defined() also means that function name is now correct.

Fixes #402

Copy link

github-actions bot commented Jun 18, 2024

Test Results

712 tests  ±0   712 ✅ ±0   0s ⏱️ ±0s
  6 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit 0d8b50e. ± Comparison against base commit b6cdb53.

♻️ This comment has been updated with latest results.

@Timmmm Timmmm force-pushed the user/timh/redundant_access_checks branch from 50b6448 to f5b67c6 Compare July 4, 2024 15:25
Copy link
Collaborator

@arichardson arichardson left a comment

Choose a reason for hiding this comment

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

LGTM but I think we can simplify this even more.

model/riscv_sys_control.sail Outdated Show resolved Hide resolved
These mode levels are already checked by `csrPriv()` so there's no need to do it again. Removing these checks from `is_CSR_defined()` also means that function name is now correct.

Since the Privilege parameter is no longer used I removed it.

Fixes riscv#402
@Timmmm Timmmm force-pushed the user/timh/redundant_access_checks branch from f5b67c6 to 0d8b50e Compare July 10, 2024 15:07
@Timmmm Timmmm requested a review from arichardson July 11, 2024 13:15
Copy link
Contributor

@jordancarlin jordancarlin left a comment

Choose a reason for hiding this comment

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

LGTM

@Timmmm Timmmm merged commit 4817b64 into riscv:master Jul 11, 2024
2 checks passed
@Timmmm Timmmm deleted the user/timh/redundant_access_checks branch July 15, 2024 14:06
Timmmm pushed a commit to Timmmm/sail-riscv that referenced this pull request Oct 2, 2024
debug: Re-enable unavailable tests, and fix them for github
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.

is_CSR_defined has redundant privilege mode checks
3 participants