How to understand these two same condition?

R

Robert Willy

Guest
Hi,

I have the following code which is generated by Simulink HDL coder. It has
two same if clause:

if (dout_re_rdenb == 1)

In verilog simulation, it is correct. I just feel the two same if clause
weird. Could you explain its meaning in the test bench?


Thanks,




always @ (posedge clk or posedge reset) // checker_dout_re
begin
if (reset == 1) begin
dout_re_timeout <= 0;
dout_re_testFailure <= 0;
dout_re_errCnt <= 0;
end
else begin
if (dout_re_rdenb == 1 ) begin
dout_re_timeout <= 0;
if (dout_re !== dout_re_repected[dout_re_addr]) begin
end

end
else if (dout_re_timeout > MAX_TIMEOUT && dout_re_rdenb == 1 ) begin
dout_re_errCnt <= dout_re_errCnt + 1;
dout_re_testFailure <= 1;
$display ("Error: Timeout not received for dout_re.");
$stop;
end
else if (dout_re_rdenb == 1) begin
dout_re_timeout <= dout_re_timeout + 1 ;
end
end
end // checker_dout_re
 
On 4/16/2015 11:44 AM, Robert Willy wrote:
Hi,

I have the following code which is generated by Simulink HDL coder. It has
two same if clause:

if (dout_re_rdenb == 1)

In verilog simulation, it is correct. I just feel the two same if clause
weird. Could you explain its meaning in the test bench?


Thanks,


always @ (posedge clk or posedge reset) // checker_dout_re
....snip...
if (dout_re_rdenb == 1 ) begin
dout_re_timeout <= 0;
if (dout_re !== dout_re_repected[dout_re_addr]) begin
end

end
ELSE if (dout_re_timeout > MAX_TIMEOUT && dout_re_rdenb == 1 ) begin
dout_re_errCnt <= dout_re_errCnt + 1;
dout_re_testFailure <= 1;
$display ("Error: Timeout not received for dout_re.");
$stop;
end
else if (dout_re_rdenb == 1) begin
dout_re_timeout <= dout_re_timeout + 1 ;
end
end
end // checker_dout_re
Click to expand...

This code is not correct. The relevant IF is dout_re_rdenb == 1 which
means the ELSE is dout_re_rdenb == 0. So a test for dout_re_rdenb == 1
in the ELSE clause will never evaluate to true. So none of the IF
dout_re_rdenb == 1 ELSE clause will ever be executed... unless I am not
following how the code is structured.

I would have indented the ELSE clauses once to show them subordinate to
the IF statement. These are nested IF statements but the formatting
doesn't clearly show that. Or am I mistaken about the nesting of the
IFs? Verilog is not my first (HD)language.

--

Rick
 
rickman wrote:
On 4/16/2015 11:44 AM, Robert Willy wrote:
Hi,

I have the following code which is generated by Simulink HDL coder. It
has
two same if clause:

if (dout_re_rdenb == 1)

In verilog simulation, it is correct. I just feel the two same if clause
weird. Could you explain its meaning in the test bench?


Thanks,


always @ (posedge clk or posedge reset) // checker_dout_re
....snip...
if (dout_re_rdenb == 1 ) begin
dout_re_timeout <= 0;
if (dout_re !== dout_re_repected[dout_re_addr]) begin
end

end
ELSE if (dout_re_timeout > MAX_TIMEOUT && dout_re_rdenb == 1 )
begin
dout_re_errCnt <= dout_re_errCnt + 1;
dout_re_testFailure <= 1;
$display ("Error: Timeout not received for dout_re.");
$stop;
end
else if (dout_re_rdenb == 1) begin
dout_re_timeout <= dout_re_timeout + 1 ;
end
end
end // checker_dout_re

This code is not correct. The relevant IF is dout_re_rdenb == 1 which
means the ELSE is dout_re_rdenb == 0. So a test for dout_re_rdenb == 1
in the ELSE clause will never evaluate to true. So none of the IF
dout_re_rdenb == 1 ELSE clause will ever be executed... unless I am not
following how the code is structured.

I would have indented the ELSE clauses once to show them subordinate to
the IF statement. These are nested IF statements but the formatting
doesn't clearly show that. Or am I mistaken about the nesting of the
IFs? Verilog is not my first (HD)language.
Click to expand...

Actually this is a common indentation style for Verilog. "else if" in
Verilog is equivalent to "elsif" in VHDL. So these elses are at the
same indentation as the original "if". However you are correct that
none of the "else" clauses should fire because dout_re_rdenb should
be 0 (or at least not 1 for simulation). I was wondering if there
was some code missing between the orignal "if" and the first "else if".

--
Gabor
 
On 4/16/2015 1:44 PM, GaborSzakacs wrote:
rickman wrote:
On 4/16/2015 11:44 AM, Robert Willy wrote:
Hi,

I have the following code which is generated by Simulink HDL coder.
It has
two same if clause:

if (dout_re_rdenb == 1)

In verilog simulation, it is correct. I just feel the two same if clause
weird. Could you explain its meaning in the test bench?


Thanks,


always @ (posedge clk or posedge reset) // checker_dout_re
....snip...
if (dout_re_rdenb == 1 ) begin
dout_re_timeout <= 0;
if (dout_re !== dout_re_repected[dout_re_addr]) begin
end

end
ELSE if (dout_re_timeout > MAX_TIMEOUT && dout_re_rdenb == 1 )
begin
dout_re_errCnt <= dout_re_errCnt + 1;
dout_re_testFailure <= 1;
$display ("Error: Timeout not received for dout_re.");
$stop;
end
else if (dout_re_rdenb == 1) begin
dout_re_timeout <= dout_re_timeout + 1 ;
end
end
end // checker_dout_re

This code is not correct. The relevant IF is dout_re_rdenb == 1 which
means the ELSE is dout_re_rdenb == 0. So a test for dout_re_rdenb ==
1 in the ELSE clause will never evaluate to true. So none of the IF
dout_re_rdenb == 1 ELSE clause will ever be executed... unless I am
not following how the code is structured.

I would have indented the ELSE clauses once to show them subordinate
to the IF statement. These are nested IF statements but the
formatting doesn't clearly show that. Or am I mistaken about the
nesting of the IFs? Verilog is not my first (HD)language.


Actually this is a common indentation style for Verilog. "else if" in
Verilog is equivalent to "elsif" in VHDL. So these elses are at the
same indentation as the original "if". However you are correct that
none of the "else" clauses should fire because dout_re_rdenb should
be 0 (or at least not 1 for simulation). I was wondering if there
was some code missing between the orignal "if" and the first "else if".
Click to expand...

Yes, I guess there isn't much difference from the VHDL ELSIF, I'm just
more used to reading VHDL. Doesn't mean I have to like it though. lol

You mean something missing that would put all this code inside the IF
part rather than the ELSE of the IF (dout_re_rdenb == 1 )? In that case
the subsequent IF (dout_re_rdenb == 1 ) becomes superfluous. I suppose
there could be code missing that would put them outside the first IF.
But this would still be odd code since it could be incorporated in the
first IF (dout_re_rdenb == 1 ) eliminating redundancy.

if (dout_re_rdenb == 1 ) begin
...stuff...
if (dout_re_timeout > MAX_TIMEOUT) begin
...more stuff...
end
else begin
...final stuff...
end
end

I think this is a *lot* more clear than the code provided.

--

Rick
 
On Thursday, April 16, 2015 at 11:44:15 AM UTC-7, rickman wrote:
this would still be odd code since it could be incorporated in the
first IF (dout_re_rdenb == 1 ) eliminating redundancy.

if (dout_re_rdenb == 1 ) begin
...stuff...
if (dout_re_timeout > MAX_TIMEOUT) begin
...more stuff...
end
else begin
...final stuff...
end
end

I think this is a *lot* more clear than the code provided.

--

Rick
Click to expand...

Thank both of you very much. I post the full part of the code.
Hopefully it is clear now that there is a redundant line.

always @ (posedge clk or posedge reset) // checker_dout_re
begin
if (reset == 1) begin
dout_re_timeout <= 0;
dout_re_testFailure <= 0;
dout_re_errCnt <= 0;
end
else begin
if (dout_re_rdenb == 1 ) begin
dout_re_timeout <= 0;
if (dout_re !== dout_re_re_expected[dout_re_addr] ||
dout_im !== dout_re_im_expected[dout_re_addr]) begin
dout_re_errCnt <= dout_re_errCnt + 1;
dout_re_testFailure <= 1;
$display("ERROR in dout_re/dout_im at time %t :
Expected (real) '%h' Actual (real) '%h',
Expected (imaginary) '%h' Actual (imaginary) '%h'",
$time, dout_re_re_expected[dout_re_addr],
dout_re, dout_re_im_expected[dout_re_addr],
dout_im, );
if (dout_re_errCnt >= MAX_ERROR_COUNT)
$display("Warning: Number of errors for dout_re/dout_im
have exceeded the maximum error limit");
end

end
else if (dout_re_timeout > MAX_TIMEOUT && dout_re_rdenb == 1 ) begin
dout_re_errCnt <= dout_re_errCnt + 1;
dout_re_testFailure <= 1;
$display ("Error: Timeout - Data was not received for dout_re.");
$stop;
end
else if (dout_re_rdenb == 1) begin
dout_re_timeout <= dout_re_timeout + 1 ;
end
end
end // checker_dout_re
 
On Friday, April 17, 2015 at 8:45:35 PM UTC-7, Robert Willy wrote:

Thank both of you very much. I post the full part of the code.
Hopefully it is clear now that there is a redundant line.

always @ (posedge clk or posedge reset) // checker_dout_re
begin
if (reset == 1) begin
dout_re_timeout <= 0;
dout_re_testFailure <= 0;
dout_re_errCnt <= 0;
end
else begin
if (dout_re_rdenb == 1 ) begin
dout_re_timeout <= 0;
if (dout_re !== dout_re_re_expected[dout_re_addr] ||
dout_im !== dout_re_im_expected[dout_re_addr]) begin
dout_re_errCnt <= dout_re_errCnt + 1;
dout_re_testFailure <= 1;
$display("ERROR in dout_re/dout_im at time %t :
Expected (real) '%h' Actual (real) '%h',
Expected (imaginary) '%h' Actual (imaginary) '%h'",
$time, dout_re_re_expected[dout_re_addr],
dout_re, dout_re_im_expected[dout_re_addr],
dout_im, );
if (dout_re_errCnt >= MAX_ERROR_COUNT)
$display("Warning: Number of errors for dout_re/dout_im
have exceeded the maximum error limit");
end

end
else if (dout_re_timeout > MAX_TIMEOUT && dout_re_rdenb == 1 ) begin
dout_re_errCnt <= dout_re_errCnt + 1;
dout_re_testFailure <= 1;
$display ("Error: Timeout - Data was not received for dout_re.");
$stop;
end
else if (dout_re_rdenb == 1) begin
dout_re_timeout <= dout_re_timeout + 1 ;
end
end
end // checker_dout_re
Click to expand...

It is confirmed that in simulation the code below the last if does not run.
After removing the last "if (dout_re_rdenb == 1)", I feel that it is
correct with the timer counter increases at the beginning and the end of
the simulation. Thanks again.
 
On 4/18/2015 12:28 AM, Robert Willy wrote:
On Friday, April 17, 2015 at 8:45:35 PM UTC-7, Robert Willy wrote:

Thank both of you very much. I post the full part of the code.
Hopefully it is clear now that there is a redundant line.

always @ (posedge clk or posedge reset) // checker_dout_re
begin
if (reset == 1) begin
dout_re_timeout <= 0;
dout_re_testFailure <= 0;
dout_re_errCnt <= 0;
end
else begin
if (dout_re_rdenb == 1 ) begin
dout_re_timeout <= 0;
if (dout_re !== dout_re_re_expected[dout_re_addr] ||
dout_im !== dout_re_im_expected[dout_re_addr]) begin
dout_re_errCnt <= dout_re_errCnt + 1;
dout_re_testFailure <= 1;
$display("ERROR in dout_re/dout_im at time %t :
Expected (real) '%h' Actual (real) '%h',
Expected (imaginary) '%h' Actual (imaginary) '%h'",
$time, dout_re_re_expected[dout_re_addr],
dout_re, dout_re_im_expected[dout_re_addr],
dout_im, );
if (dout_re_errCnt >= MAX_ERROR_COUNT)
$display("Warning: Number of errors for dout_re/dout_im
have exceeded the maximum error limit");
end

end
else if (dout_re_timeout > MAX_TIMEOUT && dout_re_rdenb == 1 ) begin
dout_re_errCnt <= dout_re_errCnt + 1;
dout_re_testFailure <= 1;
$display ("Error: Timeout - Data was not received for dout_re.");
$stop;
end
else if (dout_re_rdenb == 1) begin
dout_re_timeout <= dout_re_timeout + 1 ;
end
end
end // checker_dout_re

It is confirmed that in simulation the code below the last if does not run.
After removing the last "if (dout_re_rdenb == 1)", I feel that it is
correct with the timer counter increases at the beginning and the end of
the simulation. Thanks again.
Click to expand...

I don't see anything different in this code from the earlier example.
Unless I have missed a mistake in how the BEGIN END blocks are
structured, the other two conditions can never be true since they are in
a section of code that is only analyzed when dout_re_rdenb is zero. So
the IF conditions can not be true.

else if (dout_re_timeout > MAX_TIMEOUT && dout_re_rdenb == 1 ) begin

else if (dout_re_rdenb == 1) begin

These conditionals are in the ELSE of this conditional

if (dout_re_rdenb == 1 ) begin

So if dout_re_rdenb must not be 1 the above two conditionals never can
be true and can never execute.

What is the intent of this design? Figure that out and fix the logic.

--

Rick
 
You must log in or register to reply here.

Welcome to EDABoard.com

Sponsor

Back
Top