[PATCH] D107162: [lit] Have REQUIRES support the target triple

Paul Robinson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 5 13:05:36 PDT 2021


probinson added a comment.

In order to find other affected tests, I enabled as many projects as possible; this turns out to be all of them except `libc` and `cross-project-tests`.  (`libc` won't build with gcc 7.5, apparently, and `cross-project-tests` appears to have an error that keeps it from building at all.)

The test summary counts show there are two Unsupported tests that change to Pass, and one Fail that changes to Pass (which really doesn't make sense, maybe it's just a flaky test).  I'm rerunning with and without the patch, collecting the complete lists of Unsupported tests in order to identify which ones have started passing.  Most likely they are in the same category as the one I fixed previously, i.e. some inadvertent mistake made when writing the REQUIRES clause, except they don't happen to fail when actually run.

I'll get the docs fixed up so they can be reviewed before I push this, and also report back on which tests were affected and why.

Thanks all, especially for the correction about which cases allow regexes.



================
Comment at: llvm/utils/lit/tests/Inputs/xunit-output/lit.cfg:10
 config.test_exec_root = None
-config.target_triple = None
+config.target_triple = ''
----------------
delcypher wrote:
> probinson wrote:
> > jdenny wrote:
> > > Why does this need to change?
> > Because the test execution goes through the REQUIRES path, and the target_triple check gets a Python error if it's `None` (says something like "not iterable").  
> To avoid breaking any existing test suite that sets `config.target_triple` to `None` we could do this.
> 
> ```lang=python
>    def getMissingRequiredFeaturesFromList(self, features):
>         triple = getattr(self.suite.config, 'target_triple', "")
>         if triple is None:
>           triple = ''
>         try:
> 
> ```
> 
> However `getUnsupportedFeatures()` doesn't do this currently so that would be a bit inconsistent.
In order for an existing test suite to have `config.target_triple = None` and not get this error today, all tests controlled by that lit.cfg file would have to avoid using UNSUPPORTED and XFAIL.  That seems unlikely.
It comes up here only because we're looking at lit's own tests, which each tend to have their own config file.
I think the risk is miniscule, but I'm willing to make the change (for all 3 clauses) if you think it's appropriate.


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

https://reviews.llvm.org/D107162



More information about the llvm-commits mailing list