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

CV32E40Pv2 Verification update #2385

Merged

Conversation

XavierAubert
Copy link

This pull request contains the past two weeks efforts on CV32E40Pv2 verification done by @dd-baoshan, @dd-BeeNee and I.
Main parts of this PR are waivers after coverage analysis, RISCOF architecture-tests bring up for all configurations and a first batch of updates for DV plans annotation.

On other specific topics, commit c68ac90 from @dd-baoshan adds a comment on how illegal instructions are generated (related to issue #2382). @MikeOpenHWGroup please tell us if this comment block is sufficient enough.

Regarding DV plan annotation, the first batch targets plans of XPULP (non-hwloop) instructions. As you participated in the first round of reviews, @MikeOpenHWGroup and @MarioOpenHWGroup can you have a look and tell me if the way I did the annotation seems fine to you ? Please note that the file mentionned in the DV Plan that describes UVM TB plusargs is still a WIP, I will push it as soon as possible.

I also had a quick discussion with Mike on how to update v1 coverage links in DV plans for v2 TB and keeping track as well of history, and I agree we should open an issue to discuss it

dd-baoshan and others added 29 commits February 28, 2024 14:22
…s_WW08

Prevent code space corruption due to post incr load store
…fig. This code is unreachable in all conf we verify in v2.

Signed-off-by: Bee Nee Lim <[email protected]>
move one line of code coverage waiver to exclusion file applicable for all config. This code is unreachable in all config we verify in v2.
Update tb files: Separate RISCV Architecture Test for config pulp and pulp_fpu
Bring up RISCOF test on CFG_P_F1, CFG_P_F2.
Bring up RISCOF test on CFG_P, F0, F1, F2.
Add code coverage waiver for CFG_P_F0, after review
…es for fp cov usage; Update F-ext cov to optimize and fix some of the uncovered cp or cross; Update Zfinx cov to use new marcro define

Signed-off-by: dd-baoshan <[email protected]>
…s_WW09

Cv32e40p/bsm update tb files ww09
…nal defines for zfinx cov usage; Update zfinx to optimize and fix some of the uncovered cp or cross; Update coverage option at_least and weight for both f and zfinx

Signed-off-by: dd-baoshan <[email protected]>
…s_WW09

Update macro file: F2F and MV are not supported in zfinx. Add additio…
Move waiver that is applicable to pulp config only to cv32e40pv2_code_pulp_cfg_waiver.do
…s_WW10

Clean-up/Updates done on dvplan which some of them related to functio…
updated columns with new info required by Mike & name changes
Copy link
Member

@MikeOpenHWGroup MikeOpenHWGroup left a comment

Choose a reason for hiding this comment

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

cv32e40p/tb/uvmt/uvmt_cv32e40p_tb_ifs.sv has become very large, containing eight SV-interfaces and several of these interfaces support complex logic. I think it is time to consider refactoring this file into two or more files. Issue #2386 has been created to discuss this.

In the meantime, the rest of this pull-request looks good and a local CI regression passes, so I am approving this PR.

@MikeOpenHWGroup
Copy link
Member

On other specific topics, commit c68ac90 from @dd-baoshan adds a comment on how illegal instructions are generated (related to issue #2382). @MikeOpenHWGroup please tell us if this comment block is sufficient enough.

Yes, the comment was great! Thanks for adding it. I will close #2382

@MikeOpenHWGroup
Copy link
Member

@MikeOpenHWGroup and @MarioOpenHWGroup can you have a look and tell me if the way I did the annotation seems fine to you ?

Let's discuss at our next weekly meeting.

@MikeOpenHWGroup MikeOpenHWGroup merged commit 4aeed1e into openhwgroup:cv32e40p/dev Mar 14, 2024
1 check passed
@pascalgouedo
Copy link

@MikeOpenHWGroup and @MarioOpenHWGroup can you have a look and tell me if the way I did the annotation seems fine to you ?

Let's discuss at our next weekly meeting.

Hi @mike

I think weekly's are not technical meetings. And this question from Xavier looks rather technical to me.
Moreover as mentioned during last weekly we can't afford to loose any time as only 4 weeks are left.
So please don't wait 3 days to give your feedback to Xavier.

Thanks.

@MikeOpenHWGroup
Copy link
Member

@MikeOpenHWGroup and @MarioOpenHWGroup can you have a look and tell me if the way I did the annotation seems fine to you ?

Let's discuss at our next weekly meeting.

Hi @mike

I think weekly's are not technical meetings. And this question from Xavier looks rather technical to me. Moreover as mentioned during last weekly we can't afford to loose any time as only 4 weeks are left. So please don't wait 3 days to give your feedback to Xavier.

I have opened #2388 to discuss this topic.

@XavierAubert XavierAubert deleted the cv32e40p/dev_dd_w11a branch March 22, 2024 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants