Skip to content

ExceptionsZicboU/S and ExceptionsF#811

Open
Marina-Bellido wants to merge 20 commits intoriscv:act4from
Marina-Bellido:new-last
Open

ExceptionsZicboU/S and ExceptionsF#811
Marina-Bellido wants to merge 20 commits intoriscv:act4from
Marina-Bellido:new-last

Conversation

@Marina-Bellido
Copy link
Contributor

ExceptionsF -> 90% coverage
ExceptionsZicboU and S -> 100% coverage.

Working on fixing coverage for ExceptionsF.

@Marina-Bellido Marina-Bellido changed the title New last ExceptionsZicboU/S and ExceptionsF Dec 27, 2025
wildcard bins fmadd = {32'b?????_??_?????_?????_111_?????_1000011};
wildcard bins fsqrt = {32'b01011_??_00000_?????_111_?????_1010011};
wildcard bins fround = {32'b01000_??_00100_?????_111_?????_1010011};
`ifdef F_SUPPORTED
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why would ExceptionsF ever be run without F supported? Drop all of the ifdef F_SUPPORTED in both the coverpoints and the test.

wildcard bins fmadd = {32'b?????_??_?????_?????_111_?????_1000011};
wildcard bins fsqrt = {32'b01011_??_00000_?????_111_?????_1010011};
`ifdef D_SUPPORTED
wildcard bins fcvt_f_f = {32'b01000_??_?????_?????_111_?????_1010011};
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is an interesting one. It should probably be tested if D or Zfhmin is supported, not just D.

// building blocks for the main coverpoints
cbo_inval: coverpoint ins.current.insn {
wildcard bins cbo_inval = {32'b000000000000_?????_010_00000_0001111};
`ifdef ZICBOM_SUPPORTED
Copy link
Collaborator

Choose a reason for hiding this comment

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

Doesn’t this ifdef need to go around the entire coverpoint, not just the bins? I think right now this will create a coverpoint with bins for all possible combinations of a 32 bit instruction when Zicbom is not supported.

This applies to several coverpoints in this file.

}
menvcfg_cbcfe: coverpoint ins.current.csr[12'h30A][6] {
}
menvcfg_cbze: coverpoint ins.current.csr[12'h30A][7] {
Copy link
Collaborator

Choose a reason for hiding this comment

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

All of these envcfg coverpoints need ifdefs too.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is making me question if it makes sense to keep this all as one covergroup. Maybe we should split this into ExceptionsZicbomS and ExceptionsZicbozS (and the same for U). @davidharrishmc, any thoughts?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd prefer not to keep splitting if possible. We are up to about 250 test suites already.

///////////////////////////////////////////

##### START_TEST_CONFIG #####
# REQUIRED_EXTENSIONS: [I, Zicsr, Sm]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn’t this require F?

# CONFIG_DEPENDENT: true
##### END_TEST_CONFIG #####

//Zfa D, F, _zfa
Copy link
Collaborator

Choose a reason for hiding this comment

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

Delete this comment.

/////////////////////////////////

// Function that executes select floating point instructions in the F extension.
// Also test selected Zfa instructions, which will trap if Zfa is not supported.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Comment needs to be updated now that there are ifdefs.

// Initialize floating point registers used
li t0, 1
fcvt.s.w f0, t0
#ifdef F_SUPPORTED
Copy link
Collaborator

Choose a reason for hiding this comment

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

F should always be supported if this test is run.

This was referenced Dec 28, 2025
@jordancarlin jordancarlin added the ACT4 Issues or PRs applicable to ACT4 label Feb 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ACT4 Issues or PRs applicable to ACT4

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants