combinational logic coverage

[Andy Botterill]

I'm in the final stages of my current design block. I thought lets just
check that the RTL coverage is good. Using the public domain tool
covered I got a 100% line coverage but only 97% combinational coverage.

I agree that some of this reduced coverage is due to my testbench not
checking some combinations. This has now been improved.

With the re-written testbench my combinational coverage is now 99%.

To fix some of the poor coverage I had to change the design. This
reduced the lines that had issues from 12 to 6.

was
if( Rm_contents[31]==1'b0)
begin
shifter_operand=0;

shifter_carry_out=Rm_contents[31];
end
else
begin
shifter_operand=32'hffffffff;

shifter_carry_out=Rm_contents[31];
end

is now
shifter_carry_out=Rm_contents[31];
if( Rm_contents[31]==1'b0)
shifter_operand=0;
else
shifter_operand=32'hffffffff;

This is just combinational logic.

My questions are:-
Is it normal practice to see if *every* statement is actually necessary?

Would a paid for RTL coverage tool be smart enough to detect the
coverage in my "was" code?

My reason for these questions is to figure out what is actually best (or
standard) practice. Reducing the number of if statements will help to
make the design operate faster. Thanks for your comments. Andy

 

[LittleAlex]

On Dec 30, 10:55 am, Andy Botterill <a...@plymouth2.demon.co.uk>
wrote:

I'm in the final stages of my current design block. I thought lets just
check that the RTL coverage is good. Using the public domain tool
covered I got a 100% line coverage but only 97% combinational coverage.

I agree that some of this reduced coverage is due to my testbench not
checking some combinations. This has now been improved.

With the re-written testbench my combinational coverage is now 99%.

To fix some of the poor coverage I had to change the design. This
reduced the lines that had issues from 12 to 6.

was
if( Rm_contents[31]==1'b0)
begin
shifter_operand=0;

shifter_carry_out=Rm_contents[31];
end
else
begin
shifter_operand=32'hffffffff;

shifter_carry_out=Rm_contents[31];
end

is now
shifter_carry_out=Rm_contents[31];
if( Rm_contents[31]==1'b0)
shifter_operand=0;
else
shifter_operand=32'hffffffff;

This is just combinational logic.

My questions are:-
Is it normal practice to see if *every* statement is actually necessary?

Would a paid for RTL coverage tool be smart enough to detect the
coverage in my "was" code?

My reason for these questions is to figure out what is actually best (or
standard) practice. Reducing the number of if statements will help to
make the design operate faster. Thanks for your comments. Andy

Looking at your "was" code:

1: if( Rm_contents[31]==1'b0)
2: begin
3: shifter_operand=0;
4: shifter_carry_out=Rm_contents[31];
5: end

shifter_carry_out is never assigned to a '1' in line 4. All coverage
tools I know of will flag this.

A good synthesizer will spot this, and change the code to:

1: if( Rm_contents[31]==1'b0)
2: begin
3: shifter_operand=0;
4: shifter_carry_out=1'b0;
5: end

Your statement "Reducing the number of if statements will help to make
the design operate faster." is not true in practice. Yes, there are
coding techniques that result in faster simulations or faster
synthesized results, but simply reducing the number of IF statements
is not the answer.

AL

 

[Andy Botterill]

LittleAlex wrote:

On Dec 30, 10:55 am, Andy Botterill <a...@plymouth2.demon.co.uk
wrote:
My questions are:-
Is it normal practice to see if *every* statement is actually necessary?
Is there a check in the design process or as a side effect of running

the design tools that would indicate this sort of issue?
This section is covered thanks very much.

Would a paid for RTL coverage tool be smart enough to detect the
coverage in my "was" code?


My reason for these questions is to figure out what is actually best (or
standard) practice. Reducing the number of if statements will help to
make the design operate faster. Thanks for your comments. Andy

Looking at your "was" code:

1: if( Rm_contents[31]==1'b0)
2: begin
3: shifter_operand=0;
4: shifter_carry_out=Rm_contents[31];
5: end

shifter_carry_out is never assigned to a '1' in line 4. All coverage
tools I know of will flag this.

Re-reading the report I now see the problem. The conditional statement
makes it impossible to get the opposite state for both decision states.
Modifying the code was the right thing to do.

A good synthesizer will spot this, and change the code to:

1: if( Rm_contents[31]==1'b0)
2: begin
3: shifter_operand=0;
4: shifter_carry_out=1'b0;
5: end

Your statement "Reducing the number of if statements will help to make
the design operate faster." is not true in practice. Yes, there are
coding techniques that result in faster simulations or faster
synthesized results, but simply reducing the number of IF statements
is not the answer.

Oh. I am still learning verilog so I must try increasing the speed of a
working design to work out some methods.

AL

Thanks I've learned something.