[PATCH] D139444: [ZOS] Convert tests to check 'target={{.*}}-zos'

[Ulrich Weigand via Phabricator via cfe-commits]

uweigand added a comment.

In D139444#3975182 <https://reviews.llvm.org/D139444#3975182>, @probinson wrote:

> The changes in this patch assume that there aren't any possible suffixes after the `-zos` part of the triple (no version numbers, like you might find with darwin or macos, and nothing like `-elf` or `-eabi` like some targets have).  If there are suffixes, I'll happily revise to put `{{.*}}` after everything.

I think for consistency with other targets, and to be safe for future extensions of the target triple, it would be better to add the `{{.*}}`

> The one test I could not verify is llvm/test/Support/encoding.ll, because I don't have the utilities that it needs.  But `UNSUPPORTED: !` was the workaround for not having triples allowed in `REQUIRES:` so I think it's the right change.

The question here is, what are the specific requirements for the test to run.   One part is that the test just calls plain `llc` but expects this will generate code appropriate for z/OS.  That's not how this is usually done.  Instead, you should add an explicit target triple to the `llc` invocation, e.g. `llc -mtriple=s390x-ibm-zos`.   However, as this requires that support for the SystemZ target is built into LLVM, you then also need to add the following:

  REQUIRES: systemz-registered-target

This has the advantage that the test will actually be run on any host platform, as long as the target support is built in (which is is by default e.g. in the CI builders).

However, it might also be the case that the test case requires a z/OS host environment to run on.  I believe this is true here, since `chtag` seems to be a z/OS specific tool, and `iconv` (in particular when using the `IBM-1047` code page) also may not be universally available.

To express that restriction on the *host* system, you should be using a `REQUIRES: system-zos` line.   However, it looks like this capability is not actually currently implemented - you'll have to add it to the code in `utils/lit/lit/llvm/config.py` here:

  [...]
          elif platform.system() == 'NetBSD':
              features.add('system-netbsd')
          elif platform.system() == 'AIX':
              features.add('system-aix')
          elif platform.system() == 'SunOS':
              features.add('system-solaris')

(Note that you probably still should add the `-mtriple` because the test case requires *both* running on a z/OS host *and* compiling for the z/OS target.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139444/new/

https://reviews.llvm.org/D139444