[PATCH] D137434: [lit] Add `target=<triple>` as a feature keyword

Wed Nov 9 12:23:34 PST 2022

dblaikie added a comment.

In D137434#3913191 <https://reviews.llvm.org/D137434#3913191>, @probinson wrote:

> In D137434#3913083 <https://reviews.llvm.org/D137434#3913083>, @dblaikie wrote:
>
>> This is to detect the current/default target?
>
> This is to allow checking the default target in a more principled way.  Currently UNSUPPORTED/XFAIL (but not REQUIRES) can use arbitrary substrings of the default target triple as "feature keywords" and the end goal here is to replace all those with regexes against `target=<triple>` (thus allowing REQUIRES to check the target triple as well).
>
>> Should we instead support something more like the one in cross-project-tests, that adds a feature for every registered target? (that way a test can declare that it runs any time x86-registered-target is available, and it can specify the x86 target, so the test would run even on arm-default builds, for instance, so long as they also included x86?)
>
> That's not what I see cross-project-tests doing; it's checking whether any of the registered targets match the _host_ because it wants to be able to build/test executable programs running on the host.

Not sure I follow - this code ( https://github.com/llvm/llvm-project/blob/e044796132abd9e10e21093822dc234003628fbd/cross-project-tests/lit.cfg.py#L256-L258 ) appears to be adding a `<arch>-registered-target` for every architecture that's enabled, not related to whether they match the host triple. Oh, and that's already in clang and used a lot there, so maybe porting that feature over to LLVM?

But registered-target is for all the registered targets (& only the architecture, not the OS, etc, in the triple) - what's the particular need for the default target (unless it's native - which is useful for executing the code).

I guess it means a test can be written generically rather than REQUIRES target + hardcoded target on the test execution, etc?

If there's a discourse thread as @ldionne mentioned, yeah, be good to have a link for context, etc.

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

https://reviews.llvm.org/D137434