[SYCL][E2E] Add test to check REQUIRES

[KornevNikita]

This test checks the content of every "REQUIRES" to avoid typos and non-existing requirements.

This PR also updates some tests requirements found during test development.

[KornevNikita]

The test will fail in pre-commit as we have REQUIRES: TEMPORARY_DISABLED. There is a PR to fix this - #15946

[bader]

We must import features from llvm/sycl/test-e2e/lit.cfg.py instead of building a manually maintained copy of this list.

[AlexeySachkov]

We must import features from llvm/sycl/test-e2e/lit.cfg.py instead of building a manually maintained copy of this list.

Can we really do that in a reliable manner? Those features depend on the environment (OS, available tools, build configuration, enabled projects, etc.).

I.e. it is expected that REQUIRES may reference features which are not registered. But some of those could be known and therefore legal, whilst others could be a result of a typo and therefore should be fixed.

[KornevNikita]

We must import features from llvm/sycl/test-e2e/lit.cfg.py instead of building a manually maintained copy of this list.

+1 to Alexey's question. lit.cfg.py generates the set of available features "online" according to the specific env. Do you know if it's possible to get the whole set regardless of env? I agree it's not the best idea to maintain this set manually, but I'm not sure if we have another options.

[bader]

I have an idea, but it requires refactoring of llvm/sycl/test-e2e/lit.cfg.py.
We can have a dictionary of all features with callbacks like this:

features = {
'x': available, # 'available' function just returns true, i.e. make 'x' always available.
'y': is_y_available, # 'is_y_available' function returns true if feature 'y' is available.
'z': available, # 'available' function just returns true
...
}

1. To get the list of all features that can be used by e2e tests we just need to get the keys (i.e. features.keys())
2. To build the list of available features, we just go over all the items in the dictionary and use functions to decide which features are available.

The benefit of this implementation, you don't need to update your test when new feature is added.

[KornevNikita]

Are there always available features?
I like that the suggested approach is more "centralized", but I guess it will take some time as it requires some further discussion. I'd prefer to merge this PR as is, since I don't think we get new features really often. Then we can proceed with a follow-up patch to move this set to lit.cfg.py.

[bader]

we get new features really often.

Considering that, do we need to rush with merging this solution? I suggest we improve the solution before merging.

[KornevNikita]

I'm trying to prototype this and have a few questions:

1. Do we really need to proceed with such dictionary and not the set? Such dictionary requires the developer to not only add a key for new feature, but also a function to detect the availability of this feature.
2. It leads to dozens of these "detect" functions.
3. And it looks like then we can't auto generate aspect and architecture features (I mean as it's done now in this patch), as they may require some specific approach to detect them.

In total from my POV the idea of such dictionary looks too complicated. Looks like it requires a thorough refactoring now and a lot of work in future to support it, however, I don't see much benefit from this.

I suggest to keep it as set and move it to lif.cfg.py as function, so we can call it from this test. Something like:

def get_all_features:
    all_features {
        "cpu",
        "gpu",
        ...
    }
    all_features.update(generate_aspects())
    all_features.update(generate_archs())
    return all_features

What do you think?

[bader]

I'm trying to prototype this and have a few questions:

1. Do we really need to proceed with such dictionary and not the set? Such dictionary requires the developer to not only add a key for new feature, but also a function to detect the availability of this feature.

The main benefit of using dictionary is that it is a single source of all features. What I'd like to avoid is to build two separate lists of features: all possible features and all available features. The idea with dictionary allows us to build "all available features" list automatically. Current proposal requires developers to add features into two lists separately and it's easy to miss. I'm open to alternative ideas, which will guarantee that if developer adds new llvm-lit feature to the list of "available features", there is no way to skip the feature in the list of "all possible features".

Your test is kind of guarantees that to some extent i.e. it's supposed to fail if e2e tests will use some llvm-lit feature X, which is not in the list of available_features defined by sycl/test/e2e_test_requirements/check-correctness-of-requirements.py. Am I right?
 

2. It leads to dozens of these "detect" functions.

Right. Is that a problem? We have all this logic already. It's just not structured as separate "detect" functions.

3. And it looks like then we can't auto generate aspect and architecture features (I mean as it's done now in this patch), as they may require some specific approach to detect them.

Why not?

In total from my POV the idea of such dictionary looks too complicated. Looks like it requires a thorough refactoring now and a lot of work in future to support it, however, I don't see much benefit from this.

I agree that it requires refactoring now, but I hope you see the benefit for future changes, which I tried to explain above. If you have any better ideas, I'm all ears.

I suggest to keep it as set and move it to lif.cfg.py as function, so we can call it from this test. Something like:

def get_all_features:
    all_features {
        "cpu",
        "gpu",
        ...
    }
    all_features.update(generate_aspects())
    all_features.update(generate_archs())
    return all_features

What do you think?

How is this different from your current patch?