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

[Uncertain]Incorrect behavior of non-blocking assignment in simulation result #79

Closed
wswongat opened this issue Jan 31, 2022 · 4 comments · Fixed by #80
Closed

[Uncertain]Incorrect behavior of non-blocking assignment in simulation result #79

wswongat opened this issue Jan 31, 2022 · 4 comments · Fixed by #80
Labels
bug Something isn't working

Comments

@wswongat
Copy link
Contributor

Describe the bug

The waveform generated by the ROHD simulator doesn't have same behavior like other simulators such as modelsim.
I need some helps to clarify this bug.

To Reproduce

Full dart code file
delay_signal.txt
Please change the file extension to dart.
Core part:

    List<Logic> _z = List<Logic>.generate(depth, (index) => Logic(width: bitWidth));

    var out = addOutput('out', width: bitWidth);

    List<ConditionalAssign> _zList = [_z[0] < inputVal];
    for(int i = 0; i<_z.length;i++){
    // for(int i = _z.length-1; i>=0;i--){
      if(i == _z.length-1){
        _zList.add(out < _z[i]);
      }else{
        _zList.add(_z[i+1] < _z[i]);
      }
    }

    Sequential(clk, [
      IfBlock([
        Iff(en, _zList
        ),
        Else([
          out < 0,
        ])
      ])
    ]);
  }

Steps to reproduce the behavior:

  1. Run a simulation with ROHD
  2. See the waveform from gtkwave

Expected behavior

The input value should be appeared in the out value after few clock cycles.

Generated systemverilog:

//  sequential
always_ff @(posedge clk) begin
  if(en) begin
      s0 <= inputVal;
      s1 <= s0;
      s2 <= s1;
      s3 <= s2;
      s4 <= s3;
      out <= s4;
  end   else begin
      out <= 0;
  end 
end

modelsim/others result: <- Correct result
image
ROHD result:
image

Actual behavior

The out value directly equal to the input val at the first positive edge.

Additional details

Please paste the output of dart --version

> dart --version
Dart SDK version: 2.15.1 (stable) (Unknown timestamp) on "linux_x64"

Please paste the contents of your pubspec.yaml file and identify the version of ROHD you're using

name: delay_signal
description: A simple command-line application.
version: 1.0.0

environment:
  sdk: '>=2.14.3 <3.0.0'


# dependencies:
#   path: ^1.8.0
dependencies:
  rohd:
    git:
      url: https://github.com/intel/rohd.git
      ref: main


dev_dependencies:
  lints: ^1.0.0
@wswongat wswongat added the bug Something isn't working label Jan 31, 2022
@mkorbel1
Copy link
Contributor

Thank you for filing this! It's surprising to see this behavior reported, since I thought unit tests covered this quite well. I'll take a closer look, try to reproduce, and keep this ticket updated on findings.

@wswongat
Copy link
Contributor Author

@mkorbel1 I was trying to write the rohd version of MovingSum3 example in chisel as self-learning, and I found this bug.
Just want to know. Is my understanding correct in terms of verilog simulation? I think I am new in both verilog and dart.
Please let me know If you need any helps.

@mkorbel1
Copy link
Contributor

I believe this is a real critical bug, thank you for finding and reporting it! Under certain situations, such as the one you wrote, signals can pass straight through rather than be properly sampled on edges. I used your example code to add a new test that confirms a fix and will protect this functionality.

Let me know if the changes in #80 resolve your issue! I plan on merging it in later today.

@wswongat
Copy link
Contributor Author

wswongat commented Feb 1, 2022

@mkorbel1 Thank you. It resolved my issue.

@mkorbel1 mkorbel1 linked a pull request Feb 9, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants