Skip to content

[dv] Add cov related targets in Makefile and coverage exclusion file#635

Merged
weicaiyang merged 1 commit into
lowRISC:masterfrom
weicaiyang:add_cov_el
Oct 26, 2019
Merged

[dv] Add cov related targets in Makefile and coverage exclusion file#635
weicaiyang merged 1 commit into
lowRISC:masterfrom
weicaiyang:add_cov_el

Conversation

@weicaiyang
Copy link
Copy Markdown
Contributor

  1. add coverage review command and common coverage exclusion file
    2 add uart coverage exclusion file

@weicaiyang
Copy link
Copy Markdown
Contributor Author

@cindychip @gaurangchitroda @shakushw
Once this PR is merged, you can use make cov_review to open verdi to review and generate coverage report. Common coverage exclusion file will be included.
Also add your IP specific exclusion file in IP Makefile.

@weicaiyang weicaiyang requested a review from eunchan October 25, 2019 20:49
@weicaiyang
Copy link
Copy Markdown
Contributor Author

@eunchan Please help to review those 2 exclusion files ending with *.el. I have added rational for each exclusion.

Comment thread hw/dv/tools/vcs/vcs.mk Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Use ${BUILD_DIR} here instead.

Comment thread hw/dv/tools/rules.mk Outdated
Copy link
Copy Markdown
Contributor

@sriyerg sriyerg Oct 25, 2019

Choose a reason for hiding this comment

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

cov_analyze is probably better.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we add cov_report as well (that runs the urg command)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

updated the name.
add cov_report but urg opts is slightly different

Comment thread hw/ip/uart/dv/cov/uart_cov_excl.el Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you update the uvmdvgen script to create this directory as well?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

Comment thread hw/dv/tools/vcs/vcs.mk Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

COV_EXCL += $(COMMON_COV_EXCL)

Comment thread hw/ip/uart/dv/Makefile Outdated
Comment thread hw/dv/tools/rules.mk Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we add cov_report as well (that runs the urg command)?

@weicaiyang weicaiyang requested a review from sriyerg October 25, 2019 23:43
Comment thread util/uvmdvgen/cov_excl.el.tpl Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think better to not add this file, just create the dir (xcelium format will be different - this assumes vcs).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed.

Comment thread hw/ip/uart/dv/Makefile Outdated
Comment thread hw/dv/tools/rules.mk Outdated
Comment on lines 16 to 25
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Move these to after 'run_result` and a new add section

##############################
## Coverage related targets ##
##############################

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Update.

Comment thread hw/ip/uart/dv/Makefile Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

change IP-> DUT

Comment thread util/uvmdvgen/gen_env.py Outdated
Copy link
Copy Markdown
Contributor

@sriyerg sriyerg Oct 26, 2019

Choose a reason for hiding this comment

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

make this:

   ('dv/cov',           '', '',           ''),

On line 53 add

  if fname == "": continue

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated. PTAL

1. add coverage review command and common coverage exclusion file
2  add uart coverage exclusion file
@weicaiyang weicaiyang requested a review from sriyerg October 26, 2019 01:11
@weicaiyang weicaiyang changed the title [dv] Add cov_review in Makefile and coverage exclusion file [dv] Add cov related targets in Makefile and coverage exclusion file Oct 26, 2019
@weicaiyang weicaiyang merged commit 9dff09b into lowRISC:master Oct 26, 2019
@weicaiyang weicaiyang deleted the add_cov_el branch October 26, 2019 01:34
@weicaiyang
Copy link
Copy Markdown
Contributor Author

@eunchan Please help to review those 2 exclusion files ending with *.el

@eunchan
Copy link
Copy Markdown
Contributor

eunchan commented Oct 28, 2019 via email

@weicaiyang
Copy link
Copy Markdown
Contributor Author

Looks good. One thing for scanmode is... UART IP will have scanmode_i from the top after PR #615 is merged. One TODO in uart.sv was related to this.

Thanks for the heads-up. Will update it later.

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.

3 participants