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

Fix comparison of data types #5005

Open
tastynoob opened this issue Mar 20, 2024 · 11 comments
Open

Fix comparison of data types #5005

tastynoob opened this issue Mar 20, 2024 · 11 comments
Labels
status: asked reporter Bug is waiting for reporter to answer a question

Comments

@tastynoob
Copy link
Contributor

related issue #4443

i found commit 5126689 cause unpredictable problems in rtl emulation

and this bug exists in subsequent versions

it may produce unpredictable conditional logic and value errors

@tastynoob tastynoob added the new New issue not seen by maintainers label Mar 20, 2024
@tastynoob
Copy link
Contributor Author

i think verilator need add large projects to the test set
there must be many more bugs introduced like this

@wsnyder wsnyder added status: asked reporter Bug is waiting for reporter to answer a question and removed new New issue not seen by maintainers labels Mar 20, 2024
@wsnyder
Copy link
Member

wsnyder commented Mar 20, 2024

We need a small self-checking test case to know what to do to fix this.

Larger test cases are in https://github.com/verilator/verilator_ext_tests if you have a project that meets the criteria please open a separate issue on that and we'll discuss.

@wsnyder
Copy link
Member

wsnyder commented Mar 20, 2024

Of course a pull to fix it would also be appreciated (with the small test).

@tastynoob
Copy link
Contributor Author

Of course a pull to fix it would also be appreciated (with the small test).

sure, i will try to fix it

@tastynoob
Copy link
Contributor Author

a stupid way to fix it:
add code
if (node1p->dtypep() != node2p->dtypep()) return false;
after this

if (node1p->type() != node2p->type()) return false;

i found it may produce wrong logic at complex scene
I really have no way to find a simple test to reproduce this bug

in my CPU, this bug will cause a module's output not assigned correctly
i extract this module separately, it can't reproduce
but at least after applying this fix, my CPU works perfectly

@tastynoob
Copy link
Contributor Author

NOTE: this fix will cause #4308 test fail
i will find a better way to fix it

@tastynoob
Copy link
Contributor Author

tastynoob commented Mar 20, 2024

it should use:
if (!VN_IS(node1p->dtypep(), BasicDType) && (node1p->dtypep() != node2p->dtypep())) return false;
and it works perfectly at my designed CPU and #4308

if you has no objection, i will create one pr
@wsnyder

@wsnyder
Copy link
Member

wsnyder commented Mar 20, 2024

Sure, please make a PR, we'll need a small test with that, thanks!

tastynoob added a commit to tastynoob/verilator that referenced this issue Mar 21, 2024
@tastynoob
Copy link
Contributor Author

can i ask about the progress?

@wsnyder
Copy link
Member

wsnyder commented Mar 22, 2024

Waiting on your pull request.

@tastynoob
Copy link
Contributor Author

tastynoob commented Mar 22, 2024

Waiting on your pull request.

here #5008

@wsnyder wsnyder linked a pull request Mar 22, 2024 that will close this issue
@wsnyder wsnyder changed the title commit 5126689 introduce unpredictable bugs Fix comparison of data types Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: asked reporter Bug is waiting for reporter to answer a question
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants