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

The hazards of different LMULs have not been considered in their entirety. #239

Open
WEIhabi opened this issue Aug 21, 2023 · 1 comment
Labels
bug Something isn't working

Comments

@WEIhabi
Copy link

WEIhabi commented Aug 21, 2023

It was mentioned earlier #111 that a different LMUL might affect hazard, and the response given by the development team was to utilize stalling when the LMUL is changed (WAIT_STATE in the ara_dispatcher) to avoid this situation.

However, when we verified it using formal verification, we found the following errors.

image
image
image
image
image

When the first incoming instruction vslideup.vx,vl=144,EW16,LMUL=2,vs2=v0,vd=6,vm=1,stride=128
Next instruction is vadd vl=16,EW16,LMUL=1/2,vs1=v11, vs2=v7,vd=v31,vadd.vv v31, v7, v11

The original slideup instruction that would have written vd=6 would have written to v7 (because stride and LMUL=2), but the next vadd instruction would have already taken out the data from the source register, v7, and done the math.
This causes the vadd instruction, which is supposed to stall and wait for v7 to be written back before taking out the data, to get the data that has not yet been written back beforehand, causing an error.

The main reason for this failure is that the stall mechanism in ara_dispatcher is not well thought out (only LMUL[2]=0 && LMUL_old > LMUL_new are taken into account).

To complete the above consider that it might be possible to rewrite it as follows:

if ((!vtype_q.vlmul[2] && (vtype_d.vlmul[2:0] < vtype_q.vlmul[2:0])) || (vtype_q.vlmul[2] && (vtype_d.vlmul[2:0] > vtype_q.vlmul[2:0])))
                  state_d = WAIT_IDLE;
end
@mp-17
Copy link
Collaborator

mp-17 commented Sep 6, 2023

Hello @WEIhabi, thanks for your extensive verification effort. Very appreciated.
You are right, we did not take into account fractional LMULs when LMUL_new > LMUL_old.
I will fix this in the next round of fixes.

Thanks again,
Matteo

@mp-17 mp-17 added the bug Something isn't working label Jun 15, 2024
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

No branches or pull requests

2 participants