Potential BUG: In consistent awfull and arempty signal

[joeldushouyu]

I haven't tried to produce this in simulation yet. I will try to do it when I am free

During testing and usage, I found something interesting.

ASIZE = 13, DSIZE = 16,AWFULLSIZE = 4096 // half of the buffer

To test the correctness of my FIFO logic, I have my FPGA setup to be in a FIFO stream-in-out testing mode.

The Verilog will first read 4096 DSIZE data from the FIFO interface provided by Cypress FX3 and then write the data back to the host laptop.

After the 4096 DSIZE read, I assert the awful flag, which should be true since I am at half of the FIFO and AWFULLSIZE == 4096.
If Awfull is true, I will move to the next state to write the data back to the host laptop. But if awfull is not true, the FPGA will enter into an error state.

At the actual testing, the awfull flag is asserted to be true at the first 2 iterations of 4096 fifo read & write. However, at the 3 iteration, the assertion statement failed!

After looking over the document of the async fifo implementation and comparing the changes that I made from the last merge, I realized the problem is AWFULLSIZE is not being bit width cast.

I was able to fix the issue of inconsistent awfull signal by making the modification to cast the parameter into wires of (ADDRESIZE+1), same size with wgraynext.

Note: after making the changes, it still passes the simulation test.

[dpretet]

Hello,

what’s your synthesis tool? Looks to me the problem comes from the code parsing and synthesis while the simulation looks good. But to be sure I’ll try to recreate the issue in simulation. Please join a log if you have one.

Maybe did you try to create the problem in simulation? At first glance, the code update is ok but I’ll do a deepest code review to ensure nothing will be break.

Best,
Damien

[joeldushouyu]

Hmm, it does seem to me it might be a synthesis tool issue.
Currently, I am using nexptr for my synthesis

I will try to reproduce the scenario in simulation and see if it happens in simulation.

Thanks!
Shouyu

[dpretet]

 It very surprising, Yosys is not yet mature. Anyway we can patch the code for it. One point is your solution uses systemverilog casting syntax and the core may be still be compiled with verilog only support. Could you try with AWFULLSIZE[ADDRSIZE:0]?Best,DamienLe 4 avr. 2024 à 15:39, joeldushouyu ***@***.***> a écrit : Hmm, it does seem to me it might be a synthesis tool issue. Currently, I am using nexptr for my synthesis I will try to reproduce the scenario in simulation and see if it happens in simulation. Thanks! Shouyu —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you commented.Message ID: ***@***.***>

[joeldushouyu]

I did try to with try with AWFULLSIZE[ADDRSIZE:0].
The specification of AWFULLSIZE also seem to solve the problem

[dpretet]

Nice, this may be the final patch 😋 I’ll spend some times to write a testcase matching your issue and do a release ASAP with it.

[joeldushouyu]

This leads me to believe that there is some weird issue with Yosys on interpreting Parameter without any casting.

Thus, for a more robust fix, I would propose to define another reg like wbinnext1 in https://github.com/dpretet/async_fifo/blob/master/rtl/wptr_full.v#L24, and replace the the wbinnext with wbinnext1 in https://github.com/dpretet/async_fifo/blob/master/rtl/wptr_full.v#L41

Any suggestions?

Or should I got with the AWFULLSIZE[ADDRSIZE:0] method?

[joeldushouyu]

Looking at the document about the fifo implementation. I have a feeling this bug might be what was described in Page 6, Figure 2 of the document.
My theory is that during compile, the compiler interprets the Parameter as same number of wire as waddr instead of wbinnext, which happens to be missing the MSB wire.

[joeldushouyu]

Because given this bug only happened when

1. I am checking at the half empty/full mark of the fifo
2. Happened at the third time when I fill & empty the fifo, see the test case in Specifc wire size on AREMPTYSIZE and AWWFULLSIZE parameter #13 (comment)

[dpretet]

No problem, everybody does mistakes buddy :)

I think I will be conservative about this update and roll-back the master with the previous version (v1.2.0, before the almost threshold setup).

I'd like you do a new merge request with all your updates (systemverilog casting style) on the branch aflags which is aligned with your last pull request. I'll try to do more extensive tests about this feature in simulation. I'd like also you take time to test your update on hardware while you looks to have a good platform to check it and give me a feedback later.

I'll merge again this feature which I think is a nice-to-have and will probably convert the whole code in systemverilog, so simplify this tricky casting thing.

Best,
Damien

[joeldushouyu]

Thanks for your kind words!!
I will definitely start working on it sometime next week (I am kind of busy this week with my schoolwork :( )

Thanks once again!

[dpretet]

Ok, I close the issue for the moment