CI for v11-branch

[steveicarus]

Now that we are using github actions to run CI, I think we need to update the regress-v11.list list to cut out all the
tests that do not pass, so that we get a 100% success rate for the regression test. Once that is done, it's temping
to then use that test to block PR merges to that branch. Does this make sense?

@caryr and @martinwhitaker

[caryr]

I think that makes sense, but with the expected regression files and the changes Martin made earlier today I'm not certain we need to drop anything. We likely should change any unexpected failures to be an expected failures, etc. to cleanup the unexpected failure category.

Also, I think we still need to review anything we three do not author so auto merging should be out for all branches. It also requires the three of us to make sure we check the CI results any time we push to V11 directly since we need to make sure that not only are things clean on our local machine, but the integration machines. I'm not sure I want to change to using a fork with PR for my work and realize that decision comes with extra checking on my part.

[steveicarus]

The biggest issue is that we have to be pretty careful with commits to ivtest. A broken commit to ivtest could block merges to the v11-branch of iverilog, if we enforce that CI must pass for merges to that branch.

I think develop can be run a bit more lax.

[caryr]

Yes, my point exactly we mush check that V11 is clean for any change to ivtest. I am making a patch that moves all the fails in V11 to be NI. I will commit that shortly.

[caryr]

With my changes V11 currently reports the following for the VVP part of the tests:

Total=2502, Passed=2486, Failed=0, Not Implemented=16, Expected Fail=0

[caryr]

It's also currently clean on github!

[steveicarus]

BTW, PR's can be made from branches, and not just forks. Or is that what you mean? In github, forks and branches are very different things. I've been using branches a lot to allow me to check CI results before doing the merge.

[caryr]

FYI it appears with newer version of gcc the following fails because the messages appear in a different order:

pr3366217g

On 1604 both gcc and clang match the gold file and on 1804 clang matches, but gcc has a different order for the messages. CI suggest that same issue exists for 2004. I do not currently have a 2004 VM setup. Probably need to work on that at some point in time.

[martinwhitaker]

pr3366217g has been a problem on MSYS2 for a long time, also sv_wildcard_import5. It's worse than you think, the output order changes from run to run. I've never seen it on Linux up until now.

[caryr]

I have never seen the random behavior on Linux, but have seen this specific difference for a fairly long time with my 1804 VM testing. The cheap solution is to just sort the results and gold file, but unless we understand exactly why this is happening it really feels like cheating doing that. Of course our end user will not notice or care, but it's hell for test verification. I guess at some point in time one of us needs to dive into the belly of the beast and figure this out because master will have permanent CI failures until this is resolved. What's odd is I believe this has existed in some form since before we branched for 11, so I have no explanation why the 11 branch is clean. I guess part of the interesting story.

[martinwhitaker]

It's random in the CI runs too - https://github.com/steveicarus/iverilog/runs/1547050210 from 2 days ago shows no unexpected failures if you examine the logs. So I think it not showing in the v11 runs is just chance. But I never saw it on the Travis CI Linux runs, although it regularly appeared on the Windows runs until I masked it out in regress-msys2.list.

I've never spent much time trying to find the root cause because I can't reproduce it on my Linux system.

[caryr]

Well that's concerning and will block what Steve want's to do here until fixed.

Just FYI I am working on getting ivtest and iverilog better integrated into sv-tests and need to finish the null class var fix before I can look at much else. Plus add in trying to wrap up a production release before Christmas so I can take an actual break and start something new for next year.

[martinwhitaker]

I think these random failures occur when the compiler is iterating over a set of pointers. In the case of pr3366217g it is iterating over LexicalScope::enum_sets. The order in which the enum sets are elaborated is thus dependent on their relative position on the heap, and this is reflected in the order the error messages appear.

[caryr]

So this is happening because the memory is not allocated in a consistent linearly increasing manner. Certainly within the bounds of acceptable behavior so not a bug in the language/STL implementation.

[martinwhitaker]

BTW, in trying to reproduce this with a newer tool set, I discovered that our configure.in files are highly incompatible with autoconf 2.70.

[steveicarus]

Are we getting to the point where maybe we want to think about moving to i.e. meson?

[caryr]

Or is it just updating the configure.in to use more modern construct? That also hints at the whole c++11 usage conversation.

[steveicarus]

You mean c++-17. :-)

Well, if the autoconf issues are easy to fix, then that's probably best.

[caryr]

Well now that you mention it!

Unfortunately that would limit the systems we could run on so well you know sacrifices may need to be made ;-).

[martinwhitaker]

I didn't find the autoconf issues easy to fix, but someone who understands autoconf might!

[martinwhitaker]

I've modified the test suite scripts to provide the option of performing an out-of-order diff, and used it for the two tests where I know this is acceptable. Let's see if that fixes the CI.

[martinwhitaker]

After a small fix for handling DOS line endings correctly, the CI is now passing on all platforms.

[caryr]

I can confirm gcc is passing on my 18.04 VM.

Thanks!

[steveicarus]

I'm running into a problem where my branches fail CI if someone else merges a change to master and updates ivtests to reflect. The only way for me to fix that is to rebase my branch to get the new changes.

My first thought is to have branches aware of the commit of ivtests that it is using, but that might just make merging even more tricky. (test.sh now becomes a merge conflict issue.) Thoughts?

[umarcor]

I'm afraid there is no solution which is much easier than rebasing when you see that happen. ivtests might be added as a git submodule. That would ensure that all the branches/PRs are tested with a fixed version of ivtests. However, you would ignore new tests in the branches, and those might fail after merging. You'd need to remember always rebasing and running CI once before merging.

[steveicarus]

We can limp along for now. Just throwing it out there, so that people might sleep on it.

One thought I had is to move ivtests into the iverilog repository, so that they are tracked together. But if that happens, I'd be keen to modernize and clean up the ivtests run scripts first. If someone wants to take that on, that's another discussion.

[caryr]

I always pull ivtest and iverilog and verify locally before pushing. I then push ivtest first and then the various iverilog branches (V11/devel) to trigger the CI with the latest code. This requires diligence and could be almost impossible if we have too many contributions at the same time because of the builtin delay.

[caryr]

This has been completed so closing.