[lit] Set the target-windows feature for any windows environment

[mstorsjo]

Summary

Other common targets are windows-gnu (MinGW) and windows-itanium, and occasionally windows-elf is used as well.

[rnk]

llvm/utils/lit/lit/llvm/config.py

96

How about -windows-[^-]*$ to handle windows-elf and the oft forgotten windows-itanium?

[mstorsjo]

Updated, with a slightly different form of the regex, that also allows the triple to just end at -windows.

Alternatively, we could just remove this part of the lit config altogether. There's no other OSes that have target-<os> as a feature (only system-<os> for the host where the test is running), and after D68133 and D68136, no tests actually use this feature any longer.

[probinson]

In D68135#1694106, @mstorsjo wrote:

Updated, with a slightly different form of the regex, that also allows the triple to just end at -windows.

Alternatively, we could just remove this part of the lit config altogether. There's no other OSes that have target-<os> as a feature (only system-<os> for the host where the test is running), and after D68133 and D68136, no tests actually use this feature any longer.

In most cases, target restrictions rely on using a component of the triple. If all Windows-target triples are guaranteed to spell the component 'windows' then REQUIRES: windows works, and we don't need target-windows at all. But I was under the impression that win32 was used in the triple sometimes?

[mstorsjo]

In D68135#1694738, @probinson wrote:

In D68135#1694106, @mstorsjo wrote:

Updated, with a slightly different form of the regex, that also allows the triple to just end at -windows.

Alternatively, we could just remove this part of the lit config altogether. There's no other OSes that have target-<os> as a feature (only system-<os> for the host where the test is running), and after D68133 and D68136, no tests actually use this feature any longer.

In most cases, target restrictions rely on using a component of the triple. If all Windows-target triples are guaranteed to spell the component 'windows' then REQUIRES: windows works, and we don't need target-windows at all. But I was under the impression that win32 was used in the triple sometimes?

At least at this point, it seems to use normalized triple names. For input to tools, win32 is a synonym to windows, and "mingw32" as OS name gets normalized to "windows-gnu".

[rnk]

In D68135#1694884, @mstorsjo wrote:

In D68135#1694738, @probinson wrote:

In D68135#1694106, @mstorsjo wrote:

Updated, with a slightly different form of the regex, that also allows the triple to just end at -windows.

Alternatively, we could just remove this part of the lit config altogether. There's no other OSes that have target-<os> as a feature (only system-<os> for the host where the test is running), and after D68133 and D68136, no tests actually use this feature any longer.

In most cases, target restrictions rely on using a component of the triple. If all Windows-target triples are guaranteed to spell the component 'windows' then REQUIRES: windows works, and we don't need target-windows at all. But I was under the impression that win32 was used in the triple sometimes?

At least at this point, it seems to use normalized triple names. For input to tools, win32 is a synonym to windows, and "mingw32" as OS name gets normalized to "windows-gnu".

Right, D47381 / rL339307 replaced all uses of REQUIRES: win32.

I'm going to assume that target-windows was added to deal with the common case of wanting to XFAIL a test for mingw and windows at the same time. Now that it's easy to match them as just XFAIL: windows or REQUIRES: windows, I don't think we really need this target-windows feature.

As you mention, there are two remaining tests using target-windows. If you want to update this patch to delete the target-windows feature and migrate them to just "windows", that works for me. Less code is always better.

[mstorsjo]

In D68135#1694961, @rnk wrote:

As you mention, there are two remaining tests using target-windows. If you want to update this patch to delete the target-windows feature and migrate them to just "windows", that works for me. Less code is always better.

I do that in D68450 (which depends on D68449 which was ok'd).

[probinson]

I'm happy to be wrong about the win32 part. And eliminating 'target-windows' in favor of using just 'windows' SGTM.

[mstorsjo]

In D68135#1695155, @probinson wrote:

I'm happy to be wrong about the win32 part. And eliminating 'target-windows' in favor of using just 'windows' SGTM.

In practice, it's removing target-windows in tests where the target availability doesn't really matter, only which host platform it runs on. All of those tests already declared REQUIRES: system-windows as well, which should be enough for those tests (this landed in D68449).

[mstorsjo]

Made redundant by D68450.

llvm/utils/lit/lit/llvm/config.py

def __init__(self, lit_config, config):
         if target_triple:
             if re.match(r'^x86_64.*-apple', target_triple):
                 host_cxx = getattr(config, 'host_cxx', None)
                 if 'address' in sanitizers and self.get_clang_has_lsan(host_cxx, target_triple):
                     self.with_environment(
                         'ASAN_OPTIONS', 'detect_leaks=1', append_path=True)
             if re.match(r'^x86_64.*-linux', target_triple):
                 features.add('x86_64-linux')
-            if re.match(r'.*-windows-msvc$', target_triple):
+            if re.match(r'.*-windows(-[^-]+)?$', target_triple):
                 features.add('target-windows')
             if re.match(r'^i.86.*', target_triple):
                 features.add('target-x86')
             elif re.match(r'^x86_64.*', target_triple):
                 features.add('target-x86_64')
 
         use_gmalloc = lit_config.params.get('use_gmalloc', None)
         if lit.util.pythonize_bool(use_gmalloc):