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
No more testsuite failures on questa-2023.1 and vivado-2023.1 #2595
Conversation
tests/Main.hs
Outdated
] | ||
, clashFlags=["-fclash-hdlsyn", "Vivado"] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
] | |
, clashFlags=["-fclash-hdlsyn", "Vivado"] | |
} | |
] | |
, hdlLoad = hdlLoad def \\ [Vivado] | |
, hdlSim = hdlSim def \\ [Vivado] | |
} | |
in runTest "DDRout" _opts | |
, let _opts = def{ buildTargets = BuildSpecific [ "testBenchUA" | |
, "testBenchUS" | |
, "testBenchGA" | |
, "testBenchGS" | |
] | |
, hdlLoad = [Vivado] | |
, hdlSim = [Vivado] | |
, clashFlags=["-fclash-hdlsyn", "Vivado"] | |
} | |
in runTest "DDRout" _opts |
This way, we test the other HDL tools without -fclash-hdlsyn Vivado
and only Vivado with that set. Otherwise, our test bench doesn't exercise the non-Vivado path, and we are free to introduce errors there that will not be spotted by CI...
Slight disadvantage: there now are two tests called ..tests.shouldwork.DDR.DDRout.VHDL.clash (gen)
, same for Verilog and SystemVerilog (and note that the latter is only the clash (gen)
stage as we don't do SystemVerilog in Vivado yet). If we'd like to deal with this properly, we could special-case this circumstance and add hdlSyn :: [Sim]
to TestOptions
, which would instruct the machinery to add -fclash-hdlsyn X
to every sim X
listed in hdlSyn
. Then we can also vary the test names based on that. This might be a good long-term solution, that way we can mark those tests which fail without a proper -fclash-hdlsyn
. But for now, I don't think we really need it yet, if you'd rather get on with more important stuff :-).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably log this as an improvement on the issue tracker once this PR is merged.
For now, I've added your suggestion to the testsuite.
982237d
to
3684354
Compare
Without them, the following SV tests fail on QuestaSim Intel Starter Edition 2023.1: * T1524 * Transpose * VMerge
3684354
to
3b4c7f9
Compare
Running:
Used to fail on the following tests:
On my machine, which has:
With the commits in this PR, all tests pass on my machine.