YosysHQ / nextpnr

nextpnr portable FPGA place and route tool
ISC License
1.32k stars 245 forks source link

Fix handling of RNG seed #1369

Closed jthornblad closed 2 months ago

jthornblad commented 2 months ago

Hi,

There are a couple of problems with the handling of the RNG seed.

First, the seed value used for the deterministic RNG is being truncated from 64 bits to 32 (int instead of uint64) when written to the header of the output JSON file.

When using the --seed option on the command line, the argument needs to be a signed value.

If using the --seed or --randomize-seed option, it is not the originally supplied or randomized seed value that is written to the output JSON file, but a value that has been run through the rngseed() function, which permutes the value before it is stored and then used for the output JSON file.

This patch fixes these problems.

Regards Jonas

jthornblad commented 2 months ago

Does this look ok?

jthornblad commented 2 months ago

Good suggestion, done.

gatecat commented 2 months ago

Thanks!

jthornblad commented 2 months ago

I see that the CI is failing for this patch. I'm not quite sure what the problem is, do you have any idea?

gatecat commented 2 months ago

Looks like one of the tests is timing out (probably a latent instability)

rowanG077 commented 2 months ago

Input seed handling has changed which means some fixed seeds in the test suite are causing issues. https://github.com/YosysHQ/nextpnr-tests/pull/12 should fix it.