Update LLVM reference

[CaseyCarter]

Update our LLVM reference to pickup test suite changes, unskip three tests that now pass, and skip 21 tests that fail.

Also fixes a bug in std::atomic_init found by new tests: WG21-P0883 deprecated atomic_init and changed its effects to be equivalent to a relaxed store. We had implemented the deprecation, but missed the effect change.

Incorporates a change to our test machinery by Curtis to avoid reliance on libc++'s lit.cfg.

[CaseyCarter]

llvm/llvm-project@0c66af9 has broken our test machinery again.

[cbezault]

Okay I'll go make a PR to fix this sometime this week.

[CaseyCarter]

Okay I'll go make a PR to fix this sometime this week.

I'm sure it's nothing complicated: our "run specific tests" incantations still work properly, it's only invocation via ctest that complains that "You seem to be running Lit directly -- you should be running Lit through <build>/bin/llvm-lit, which will ensure that the right Lit configuration file is used."

[advancedwebdeveloper]

I am curious when libc++-abi would have an option to switch to your STL implementation (not Intel's one), during CMake's execution

[miscco]

Dropping this here for added visibility https://reviews.llvm.org/D81866

At least it is intented to enable support for other vendors such as MSVC

[ldionne]

Hey folks! Next time feel free to drop me a line personally, I'll be happy to help out.

So, first question to help out is: do you run the CMake configuration step prior to running the tests? If you do (which is the intended way to use the test suite), then you can put the Lit configuration file of your choice in libcxx/test/configs/msvc-stl-whatever.cfg.in and then use -DLIBCXX_TEST_CONFIG=/path/to/libcxx/test/configs/msvc-stl-whatever.cfg.in when configuring CMake.

The second question I have is what config file do you currently use? I can help you write the from-scratch config file from the one you're using today. The upside is that you'll have a lot more control over how you're running the test suite, it'll be a lot simpler and it can be checked into libcxx's repo as-is.

[ldionne]

@cbezault So I spent some more time looking at your setup and it looks like you've got quite a bit of copy-pasted code from libcxx in your tree :)

Ideally, the minimal integration would be:

LLVM_ROOT=<path-to-llvm-submodule>
BUILD_DIR=<wherever>

(cd "${BUILD_DIR}/libcxx" &&
    cmake "${LLVM_ROOT}/libcxx" -GNinja \
                           -DLLVM_PATH="${LLVM_ROOT}" \
                           -DLIBCXX_INCLUDE_TESTS=ON \
                           -DLIBCXX_TEST_CONFIG="${LLVM_ROOT}/libcxx/test/configs/<your-config-file>.in")

Then, just running ${BUILD_DIR}/bin/llvm-lit -sv ${LLVM_ROOT}/libcxx should work and use the configuration you picked. The format of this configuration file should follow what we're doing for "new" configurations, see for example libcxx/test/configs/libcxx-trunk-shared.cfg.in. These configuration files are a lot simpler than they used to be -- you'll have full power over how the test suite invokes the compiler, etc.

Note that this CMake configuration step is really necessary because Lit won't find the right config file if there's no config map generated for the test source directory you're running into. It's inconvenient and I'd rather have something like lit -C <path-to-config> <path-to-test-tree> instead, but that's not what we have today.

At least, the CMake configuration step doesn't configure all of LLVM, only libcxx. If that's still too much, we can probably work on adding a -C option of sorts to lit. Another option is to hack up the config map like (I think) you're already doing in https://github.com/microsoft/STL/blob/master/tests/utils/stl-lit/stl-lit.in.

[ldionne]

Oh, and the benefit is that you should not, in theory, need any Python code beyond that config file.

[cbezault]

Hi @idionne. We do not currently run any of the the LLVM CMake scripts. The only thing we take from the submodule is the lit.cfg for libcxx and lit itself. You are right I do already hack up the config map so that if someone uses the stl-lit script everything should "just work" without needing to configure libcxx.

As to your point concerning not needing any python besides the lit.cfg file, I REALLY wish that was the case. Unfortunately we need a special TestFormat in order to run the tests under all the configurations necessary. Each test file is run under multiple configurations which is not a concept that lit handles by default. I've been thinking about how to fit our needs into the libcxx test format so I could unify and work upstream but I haven't had time to dive deeper.

[idionne]

@ldionne

Looks like that’s for you, not me! :)

[ldionne]

Looks like that’s for you, not me! :)

Thanks!

As to your point concerning not needing any python besides the lit.cfg file, I REALLY wish that was the case. Unfortunately we need a special TestFormat in order to run the tests under all the configurations necessary. Each test file is run under multiple configurations which is not a concept that lit handles by default. I've been thinking about how to fit our needs into the libcxx test format so I could unify and work upstream but I haven't had time to dive deeper.

Would it be an option to simply run lit multiple times under different configurations? But regardless, it should be possible to set the compiler flags, search paths, etc. using the new from-scratch config and get rid of config.py, which is pretty complicated. Something like this (taken from libcxx-trunk-shared.cfg.in):

@AUTO_GEN_COMMENT@

LIBCXX_ROOT = "@LIBCXX_SOURCE_DIR@"
INSTALL_ROOT = "@CMAKE_BINARY_DIR@"
COMPILER = "@CMAKE_CXX_COMPILER@"
EXEC_ROOT = "@LIBCXX_BINARY_DIR@"

import os
import pipes
import site
import sys
site.addsitedir(os.path.join(LIBCXX_ROOT, 'utils'))
import libcxx.test.features
import libcxx.test.format
import libcxx.test.newconfig
import libcxx.test.params

# Configure basic properties of the test suite
config.name = 'msvc-stl'
config.test_source_root = os.path.join(LIBCXX_ROOT, 'test')
config.test_format = libcxx.test.format.CxxStandardLibraryTest() # Use your format here instead
config.recursiveExpansionLimit = 10
config.test_exec_root = EXEC_ROOT

# Configure basic substitutions
runPy = os.path.join(LIBCXX_ROOT, 'utils', 'run.py')
config.substitutions.append(('%{cxx}', '<msvc-compiler>'))
config.substitutions.append(('%{flags}', '<your-msvc-compile-flags>'))
config.substitutions.append(('%{compile_flags}', '<your-msvc-compile-flags>'))
config.substitutions.append(('%{link_flags}', '<your-msvc-link-flags>'))
config.substitutions.append(('%{exec}', '{} {} --execdir %T -- '.format(pipes.quote(sys.executable), pipes.quote(runPy))))

# Add parameters and features to the config
libcxx.test.newconfig.configure(
    libcxx.test.params.DEFAULT_PARAMETERS,
    libcxx.test.features.DEFAULT_FEATURES,
    config,
    lit_config
)

[cbezault]

In the case of the libcxx teststuite it would probably be fine to re-run the tests as two passes under two configurations. However, for our own tests we run each individual test under a comprehensive (and sometimes different per test) set of configurations. Those configurations live in our *.lst files.

If the setup you're proposing above could be adapted so that it can be changed for each test that would work.

Right now I create a LIT test for each of the configurations we want to run each of the actual test cpp files under. If the substitutions can be modified on a per LIT test basis inside of our test format this could work.

[ldionne]

In the case of the libcxx teststuite it would probably be fine to re-run the tests as two passes under two configurations. However, for our own tests we run each individual test under a comprehensive (and sometimes different per test) set of configurations. Those configurations live in our *.lst files.

If the setup you're proposing above could be adapted so that it can be changed for each test that would work.

Right now I create a LIT test for each of the configurations we want to run each of the actual test cpp files under. If the substitutions can be modified on a per LIT test basis inside of our test format this could work.

Hmm, although that isn't the way it's intended to work, I believe you could duplicate the config object in your test format, change the substitutions as desired, and then call the new CxxStandardLibraryTest test format twice, once with each configuration.

[cbezault]

The way forward I see is to have a TestFormat which derives from the libcxx.test.format.CxxStandardLibraryTest and overrides the getTestsInDirectory function to do what I want. This is very similar to the current setup.

[ldionne]

The way forward I see is to have a TestFormat which derives from the libcxx.test.format.CxxStandardLibraryTest and overrides the getTestsInDirectory function to do what I want. This is very similar to the current setup.

Ah, right. Indeed, you can tweak the config object when creating a lit.Test in that function. It looks like the best approach to me too.

[cbezault]

@ldionne Something that I noticed was pretty pervasive in format.py examples I looked at was a tendency to not respect the results of different Test functions. For example _getTempPaths in libcxx's format.py creates unique directories to run the tests in by not respecting Test.getExecPath in my opinion it makes more sense from a flexibility standpoint to just define a new test class which extends lit.Test and whose attribute functions actually get respected. What do you think?

[ldionne]

@ldionne Something that I noticed was pretty pervasive in format.py examples I looked at was a tendency to not respect the results of different Test functions. For example _getTempPaths in libcxx's format.py creates unique directories to run the tests in by not respecting Test.getExecPath in my opinion it makes more sense from a flexibility standpoint to just define a new test class which extends lit.Test and whose attribute functions actually get respected. What do you think?

I don't understand what you're suggesting. getTempPaths() is not a method of the lit.Test class, it's a free function (and we do use a different implementation of it).

I think it might be simpler for me to understand if you open a simple Phabricator review for discussion -- WDYT?

[cbezault]

_getTempPaths calls lit.Test.getExecPath (which is meant to be the test declaring where its executable lives) and then mangles it to guarantee that all tests run in separate directories. However, this naming scheme doesn't mesh well with the fact that for us each individual test file can get split into many tests and there's no way to communicate an alternative naming scheme to the libcxx test format.

When I have a minute I'll make a Phabricator review with my proposed solution.

[CaseyCarter]

For posterity: @cbezault's changes seem to have the test runner working again, I pushed a change to skip a couple of new failing tests. Once I have a clean run I'm going to bump the LLVM reference up to current again, cleanup any new failures, and publish this PR (make it non-draft).

[CaseyCarter]

Fixed a bug in <atomic>, reported a bug in libc++ <atomic>, pushed 8 fixes upstream for new test failures, and added a few new skips.

I had a clean local test run at one point, but now libc++ tests runs are failing again:

stl-lit.py: C:\a\1\s\llvm-project\llvm\utils\lit\lit\TestingConfig.py:99: fatal: unable to parse config file 'C:/a/1/s/tests/libcxx/lit.cfg'

[barcharcraz]

Update our LLVM reference to pickup test suite changes. Unskip three tests that now pass, and skip 21 tests that fail.

Fix a bug in std::atomic_init found by new tests: P0083R2 deprecated atomic_init and changed its effects to be equivalent to a relaxed store. We had implemented the deprecation, but missed the effect change.

Incorporates a change to our test machinery by Curtis to avoid reliance on libc++'s lit.cfg.

I can't see such a change in P0083R2 (Splicing maps and sets), perhaps that's the wrong paper number?

[CaseyCarter]

I can't see such a change in P0083R2 (Splicing maps and sets), perhaps that's the wrong paper number?

Yes it is - sorry. P0883R2 was the winning number.

[StephanTLavavej]

Thanks for getting us back in sync with LLVM! 🚀