Improved Windows Test Coverage

Hello Swift Users,

I have nearly completed the work to enable additional testing on Windows. We were previously running the basic test suite. At this point, we nearly have the validation test suite enabled as well. I have preemptively enabled this on the Azure CI for getting some signal. For some reason, on the CI hosts, I am seeing 18 failures whilst locally I see 3.

I shall enumerate the local test failures as I believe that the increased count on Azure is due to pending patches and configuration issues (which need to be sorted out of course).

  1. Index.index_swift_only_systemmodule

This is a scripting failure with find not properly enumerating a file by glob. This probably will just require a tweak to the way that we actually write the test RUN line.

  1. IRGen/multithread_module

This is a known failure, that had disappeared but seems to have reappeared. The problem is that when building with parallel threads, the second IRGen instance will create a declaration for the nominal type metadata which will also be granted a COMDAT association. However, declarations may not have a COMDAT, so we need to identify where we are failing to identify that we have created a declaration only and are not removing the COMDAT. [ CC: @Joe_Groff @Slava_Pestov @John_McCall ]

  1. Evolution/test_rth

This is a problem with the test and a known one at that. There was a FIXME in the test that indicated that the path separation is less than ideal and that is resulting in us failing to identify the basename of the process which causes a test failure. I'm hoping to be able to resolve this test shortly.

At this point, the entire swift tree is actually being compiled for all the targets. This is a fairly large milestone for the Windows port. I've opted to extract the remaining tasks for Windows out of the master task and closed the original task tracking the Windows support at this point. There are still improvements to be done, but, the swift tree should be considered on parity with the other targets at this point.

I believe that the next target for improved coverage should be Foundation which does have the test suite building though there are still failures and some crashers to work through.

14 Likes

Awesome job man. Do you (or any of the attached maintainers) have any idea when it will be officially listed as a supported port on the primary GitHub repository?

Correct me if I'm wrong but I think the conversation for this last left off with having the test suite work with Jenkins before this can be listed alongside the linux port.

I think LLVM requires linkonce_odr declarations to be definitions, and those are the only ones that should have COMDATs, so something weird is happening here in any case.

Yes, that is absolutely correct. I had gotten to that point, the problem I am having is identifying the earliest site where we know that the Global Variable that we have constructed is known to be a declaration rather than a definition so that we can remove the COMDAT associated with it (since we apply the COMDAT in the creation of the IR construct if it is linkonce_odr since we would have to change all the sites where we create any IR construct otherwise).

Thanks! If you look at GitHub - apple/swift: The Swift Programming Language you will find that Windows 2019 is part of the Community CI section :-)

Aren't we already set up to know whether we're building a definition when creating an IR construct? I'm pretty sure I set things up that way, but maybe it's not being propagated.

No, we aren't always setup to know if it is for definition or not. I had tried to use that as deep as that got propagated, but it broke something else. I'll do that change again to actually get the actual failure (sorry, I don't remember it off the top of my head).

Okay. That ought to be reliable information, and this seems like a legitimate use of it.

It's possible that there's code that generates declarations or definitions bypassing IRGen's getAddrOfLLVMVariable. getAddrOfLLVMVariable's various flavors all look like they take a ForDefinition_t, or else something that can serve as a proxy for it (such as a possibly-null ConstantInit).

Yeah, there is at least one site which overrides the information which leads to confusion. That is where I had tried to revert but things didn't take too kindly to that. I need to still get the exact failure.