This is an archive of the discontinued LLVM Phabricator instance.

[Flang] Convert a test to check 'target=...'
ClosedPublic

Authored by probinson on Dec 13 2022, 1:18 PM.

Details

Summary

Part of the project to eliminate special handling for triples in lit
expressions.

Diff Detail

Event Timeline

probinson created this revision.Dec 13 2022, 1:18 PM
Herald added a project: Restricted Project. · View Herald Transcript
Herald added a subscriber: jdoerfert. · View Herald Transcript
probinson requested review of this revision.Dec 13 2022, 1:18 PM
awarzynski added inline comments.Dec 14 2022, 2:39 AM
flang/test/lit.site.cfg.py.in
8

Is this needed here? I guess that's where target in a LIT test takes the triple from?

probinson added inline comments.Dec 14 2022, 4:29 AM
flang/test/lit.site.cfg.py.in
8

This is needed in order for target checks to work when you don't use -DLLVM_TARGET_TRIPLE_ENV or -DFLANG_TEST_TARGET_TRIPLE.

awarzynski accepted this revision.Dec 14 2022, 8:20 AM

Thanks for the clean-up! LGTM

This revision is now accepted and ready to land.Dec 14 2022, 8:20 AM

So, it appears that a follow-up to D138675 changed the check from powerpc to powerpc-registered-target which I think is incorrect.
I believe this patch will do the right thing, and I'm going to go for it.

tblah added a subscriber: tblah.Dec 14 2022, 8:34 AM
This revision was landed with ongoing or failed builds.Dec 14 2022, 8:34 AM
This revision was automatically updated to reflect the committed changes.

So, it appears that a follow-up to D138675 changed the check from powerpc to powerpc-registered-target which I think is incorrect.
I believe this patch will do the right thing, and I'm going to go for it.

Thanks! Is the canonical way for these things documented anywhere? I checked https://llvm.org/docs/CommandGuide/lit.html, but no luck.

So, it appears that a follow-up to D138675 changed the check from powerpc to powerpc-registered-target which I think is incorrect.
I believe this patch will do the right thing, and I'm going to go for it.

Thanks! Is the canonical way for these things documented anywhere? I checked https://llvm.org/docs/CommandGuide/lit.html, but no luck.

The lit feature names are not well documented. foo-registered-target means that target foo was included in LLVM_TARGETS_TO_BUILD. So, powerpc-registered-target means only that powerpc is one possible target, not necessarily the default target or the target you are actually running on. This set of feature names is typically in a REQUIRES directive when a test is in a non-target-specific directory but intends to generate code (or otherwise invoke target-specific code) for the given target. For example, if the test specifies -mtriple=powerpc-ibm-aix in llvm/test/Transforms somewhere, you'd need to have REQUIRES: powerpc-registered-target so the test wouldn't fail for builds that didn't happen to include powerpc.

target=foobar is defined where foobar is the _default_ target triple. This is typically used for generic tests that work most places, and don't specify a particular triple, but are known to fail (or otherwise not be supported) for the specific target. lit supports regular-expression matching on keywords, so you can de-support a test for all builds with a powerpc default triple by saying UNSUPPORTED: target=powerpc{{.*}} in your test.

Up until recently, we didn't have target=foobar but instead you could put an arbitrary substring of the triple in your UNSUPPORTED or XFAIL directives, and lit would automatically compare an unrecognized feature name against the triple. My current project is to get rid of that special handling and replace it with target=foobar checks, which is why I came across these flang tests.

(The reason why UNSUPPORTED: powerpc wasn't working is that lit depends on having config.target_triple set, in order to know what the triple is. Flang wasn't doing that, so the triple-based checks weren't working. Now they are.)

There's another set of keywords, which have a system- prefix (e.g. system-windows). These describe the OS for the system the test is _running on_ regardless of what targets are enabled or which one is the default. This is used mainly for cases where an OS doesn't support something-or-other that the test depends on.

I have a longterm goal of documenting all this stuff in a reasonable place, but hopefully this is enough to help you navigate lit.

So, it appears that a follow-up to D138675 changed the check from powerpc to powerpc-registered-target which I think is incorrect.
I believe this patch will do the right thing, and I'm going to go for it.

Thanks! Is the canonical way for these things documented anywhere? I checked https://llvm.org/docs/CommandGuide/lit.html, but no luck.

The lit feature names are not well documented. foo-registered-target means that target foo was included in LLVM_TARGETS_TO_BUILD. So, powerpc-registered-target means only that powerpc is one possible target, not necessarily the default target or the target you are actually running on. This set of feature names is typically in a REQUIRES directive when a test is in a non-target-specific directory but intends to generate code (or otherwise invoke target-specific code) for the given target. For example, if the test specifies -mtriple=powerpc-ibm-aix in llvm/test/Transforms somewhere, you'd need to have REQUIRES: powerpc-registered-target so the test wouldn't fail for builds that didn't happen to include powerpc.

target=foobar is defined where foobar is the _default_ target triple. This is typically used for generic tests that work most places, and don't specify a particular triple, but are known to fail (or otherwise not be supported) for the specific target. lit supports regular-expression matching on keywords, so you can de-support a test for all builds with a powerpc default triple by saying UNSUPPORTED: target=powerpc{{.*}} in your test.

Up until recently, we didn't have target=foobar but instead you could put an arbitrary substring of the triple in your UNSUPPORTED or XFAIL directives, and lit would automatically compare an unrecognized feature name against the triple. My current project is to get rid of that special handling and replace it with target=foobar checks, which is why I came across these flang tests.

(The reason why UNSUPPORTED: powerpc wasn't working is that lit depends on having config.target_triple set, in order to know what the triple is. Flang wasn't doing that, so the triple-based checks weren't working. Now they are.)

There's another set of keywords, which have a system- prefix (e.g. system-windows). These describe the OS for the system the test is _running on_ regardless of what targets are enabled or which one is the default. This is used mainly for cases where an OS doesn't support something-or-other that the test depends on.

I have a longterm goal of documenting all this stuff in a reasonable place, but hopefully this is enough to help you navigate lit.

Makes sense, thanks for this through explanation and for driving this change! We are in good hands :)