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

Are there some bugs in clockDomain.waitSampling() API? #1382

Open
yuqi-ali opened this issue Apr 7, 2024 · 10 comments
Open

Are there some bugs in clockDomain.waitSampling() API? #1382

yuqi-ali opened this issue Apr 7, 2024 · 10 comments

Comments

@yuqi-ali
Copy link

yuqi-ali commented Apr 7, 2024

I have a StreamMonitor Written by myself,the core function is running this function by fork:

    while (true) {
      log.info(s"bus_valid:${bus.valid.toBoolean}\tbus_ready:${bus.ready.toBoolean}")
      if (busValidPtr.toBoolean/*.manager.getLong(busValidPtr.signal)!=0*/) {
        active.set()
        if (busReadyPtr.toBoolean/*.manager.getLong(busReadyPtr.signal)!=0*/) {
          recv(sample())
        }
      } else {
        active.clear()
      }
      busReadyPtr.manager.setLong(busReadyPtr.signal,(!streamPause() && !full).toInt)
      clockDomain.waitSampling()
    }

In a design of my company, I need two StreamMonitors. When running the case, one of the cases failed. By adding printing, I found that the clockDomain.waitSampling() in the StreamMonitor of an interface missed a sample, which caused the simulation fail.
Since it is the company's code, the complete code cannot be posted here. I tried to test in VCS and Verilator. This case failed due to the same error reason, while other cases were successful.

@yuqi-ali
Copy link
Author

yuqi-ali commented Apr 7, 2024

@Dolu1990

@andreasWallner
Copy link
Collaborator

To answer your question directly: maybe, but none that we know of ATM.

One question that I'd have is: what is driving the clock in the CD? Is it driven from forkStimulus?

waitSampling is used in the library verification itself and in many other places, we've yet to see an issue. Without code to reproduce it on a freely available simulator we'll not really be able to help much.
(if you want to compare your implementation to the StreamMonitor in lib:

class StreamMonitor[T <: Data](stream : Stream[T], clockDomain: ClockDomain){
)

@Dolu1990
Copy link
Member

Dolu1990 commented Apr 9, 2024

clockDomain.waitSampling() in the StreamMonitor

This is weird, there hasn't been a waitSampling since quite long in the StreamMonitor, which version of SpinalHDL are you using ?

Else, can you send a minimal self contained example for us to reproduce ? (verilator is the reference implementation of SpinalSim)

@yuqi-ali
Copy link
Author

SpinalHDL Version:1.10.1
Scala:2.11.12
The clockDomain is driven by forkStimulus.
For my design, I have a dozen cases for the same simulation environment, and only one of them has such a problem(either Verilator or VCS), causing the simulation to fail.

@yuqi-ali
Copy link
Author

I have two stream monitor in my simualtion enviroment,one is ok.
the core base class is this :
https://github.com/xiaochuang-lxc/SpinalHDL-TB/blob/main/src/main/scala/tb/monitor/StreamSink.scala

@Dolu1990
Copy link
Member

I would say, would need to first clockDomain.waitSampling() in the loop, then check for the bus status

But overall would need a small fully self contained example of weird behaviour to reproduce.

@zyn810039594
Copy link

zyn810039594 commented Apr 14, 2024

So, maybe this's the same as my problem in issues 1334?
In my issue, the problem is the clockDomain.onSampling API.

@Readon
Copy link
Collaborator

Readon commented Apr 14, 2024

This discussion has been last long, I am trying to find the difference between StreamMonitor and class you describe.

waitSampling is different from the onSampling, it waits specified number of valid samplings. onSampling triggered each valid sampling.
So some user's calling on waitSampling might interference to the one in the Monitor's main loop.
That's why onSampling is now used in the StreamMonitor design.

Another difference is that StreamMonitor is not to create a new process by fork. It uses the same process of simulation by default.

@yuqi-ali
Copy link
Author

Get Some Error Info:
The Failed Case has a lot of fork thread(dma queue,each queue has a fork thread),when test 96 fork threads, the case passed.The case failed when fork 100 threads

@andreasWallner
Copy link
Collaborator

Again: please post a self contained example that reproduces the issue - if not we can't help you.

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

No branches or pull requests

5 participants