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.

Sign up for GitHub

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

Jump to bottom

Modify coding style to improve CC #1607

Merged
merged 1 commit into from
Nov 10, 2023
Merged

Modify coding style to improve CC #1607

merged 1 commit into from
Nov 10, 2023

Conversation

JeanRochCoulon
Copy link
Contributor

Modify coding style to improve Code Coverage.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 8, 2023

✔️ successful run, report available here.

@JeanRochCoulon
Copy link
Contributor Author

@zarubaf I submitted this PR to improve Code coverage (see below). It is not the classical way to implement verilog "case". That's why I would like to get your feedback on this.

image

@JeanRochCoulon
Copy link
Contributor Author

@Gchauvon @cathales can you give me your feedback on this PR ?

@cathales
Copy link
Contributor

If I understand correctly there were:

  • missing else so a potential latch as there is no default assignment to result_o at the beginning of the process
  • several untested conditions as we do not test FPU

With this PR we:

  • do not have a potential latch anymore
  • have only one untested condition (better coverage?)
  • introduce an unreachable statement in the nested default

cathales
cathales approved these changes Nov 10, 2023
View reviewed changes
Copy link
Contributor

@cathales cathales left a comment

Choose a reason for hiding this comment

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

* introduce an unreachable statement in the nested `default`

Ah sorry I didn’t notice the default case was reached so the explicit cases were not exhaustive. So you are not introducing an unreachable case.

@github-actions GitHub Actions
Copy link
Contributor

✔️ successful run, report available here.

@JeanRochCoulon JeanRochCoulon merged commit 7872d01 into openhwgroup:master Nov 10, 2023
13 checks passed
@JeanRochCoulon JeanRochCoulon deleted the dev/fcsr branch November 10, 2023 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cathales cathales cathales approved these changes

@zarubaf zarubaf Awaiting requested review from zarubaf zarubaf is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@JeanRochCoulon @cathales