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

Potential Issue with vssseg2e16 Test Case Due to Storage Order Variance #59

Open
hugo-starfive opened this issue Dec 20, 2024 · 6 comments

Comments

@hugo-starfive
Copy link

Description:

When testing the vssseg2e16 instruction, the test case fails under specific conditions. Upon further analysis, it was found that when the stride value is set to 2, according to the RISC-V spec description for vssseg2e16,

image

there exists a scenario where v2[i] can overwrite the value stored by v3[i-1].

the spec does not enforce a sequential storage order for segment stores. From observations:

QEMU and Spike implement a storage order of v2[0] -> v3[0] -> v2[1] -> v3[1].
However, if a testing platform uses an order like v2[0] -> v2[1] -> v3[0] -> v3[1], the test results would deviate from expectations, leading to test failures.

I would like to ask whether this situation falls into the category of insufficiently robust test case construction.

@ksco
Copy link
Member

ksco commented Dec 20, 2024

Thanks for pointing this out, I didn't notice this, so I guess a easy fix would be align the stride to VLEN*LMUL, so there is no overlap?

@hugo-starfive
Copy link
Author

Thanks for your response! I think this is a solution. For Strided and Indexed Segment Store instruction tests, overlaps should be avoided as much as possible.

@ksco
Copy link
Member

ksco commented Dec 20, 2024

Well, for indexed stores, I think the ordered indexed stores require a specific order. And for the unordered ones, the situation is more complicated as the index is generated randomly... it's a known issue with no easy workaround.

ksco added a commit that referenced this issue Dec 20, 2024
@ksco
Copy link
Member

ksco commented Dec 20, 2024

The issue should be fixed above, please check again, thanks. And for unordered indexed stores, please open a new ticket.

@hugo-starfive
Copy link
Author

Thank you for fixing the issue! I’ll check the changes. I’ll also open a new ticket for unordered indexed stores as suggested

@hugo-starfive
Copy link
Author

After testing with the latest code, I found that when the minimum stride value is set to 32, most Strided Segment Stores instructions do not exhibit address overlap.

However, for instructions like vssseg5e64, address overlap can still occur.

The specific analysis is shown in the figure below.
image

The storage addresses of v2[1] and v6[0] can overlap. This issue primarily occurs with instructions like vssseg5e64, vssseg6e64, vssseg7e64, and vssseg8e64.

I think that setting the minimum stride value to 64 can resolve this issue.

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

2 participants