This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Add end-to-end codegen tests for wasm_simd128.h
ClosedPublic

Authored by tlively on Mar 14 2022, 8:48 PM.

Details

Summary

Add a test checking that each SIMD intrinsic produces the expected instruction.
Since this test spans both clang and LLVM, place it in a new
intrinsic-header-tests subdirectory of cross-project-tests.

This revives D101684 now that cross-project-tests exists. In practice, the tests
of lowering from wasm_simd128.h to LLVM IR were not as useful as this end-to-end
test.

Updates the version check of gdb in cross-project-tests/lit.cfg.py so that
unexpected version formats do not prevent the new tests from running.

Depends on D121661.

Diff Detail

Event Timeline

tlively created this revision.Mar 14 2022, 8:48 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 14 2022, 8:48 PM
tlively requested review of this revision.Mar 14 2022, 8:48 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 14 2022, 8:48 PM

cc @Orlando and @jhenderson, who have worked on cross-project-tests. I've got ninja check-intrinsic-headers kind of working, but the test doesn't run because cross-project-tests/lit.cfg.py checks for a good gdb version (which I don't have) even though I don't want to run the debuginfo tests. Would you be able to help move that configuration code into cross-project-tests/debuginfo-tests?

also cc @dblaikie, who reviewed the last iteration of this test. Now that cross-project-tests exists, do you think this test is reasonable to add? On a practical note, I continued using this test in a local branch as I finished work on the Wasm SIMD intrinsics because it was so much more valuable than the individual clang->LLVM or LLVM->Wasm tests.

cc @Orlando and @jhenderson, who have worked on cross-project-tests. I've got ninja check-intrinsic-headers kind of working, but the test doesn't run because cross-project-tests/lit.cfg.py checks for a good gdb version (which I don't have) even though I don't want to run the debuginfo tests. Would you be able to help move that configuration code into cross-project-tests/debuginfo-tests?

FWIW, when I created cross-project-tests, I left the check-debuginfo-tests target there for backwards compatibility reasons. The intent was that people use check-cross-project-tests instead. That being said, I don't think it hurts to test a subset of them.

Are you sure the GDB version check is what is causing the issue? As far as I can tell, that should just impact tests that have the gdb-clang-incompatibility feature in the REQUIRES/XFAIL/... list. What output do you get from the test being marked as UNSUPPORTED if you run the test with lit directly and the -a option, e.g. like the following?

$ llvm-lit my-test.test -a

Right now (after trying to upgrade my gdb) I get this error

$ bin/llvm-lit ~/code/llvm-project/cross-project-tests/intrinsic-header-tests/wasm_simd128.c
...
llvm-lit: /usr/local/google/home/tlively/code/llvm-project/llvm/utils/lit/lit/TestingConfig.py:103: fatal: unable to parse config file '/usr/local/google/home/tlively/code/llvm-project/cross-project-tests/lit.cfg.py', traceback: Traceback (most recent call last):
  File "/usr/local/google/home/tlively/code/llvm-project/llvm/utils/lit/lit/TestingConfig.py", line 92, in load_from_path
    exec(compile(data, path, 'exec'), cfg_globals, None)
  File "/usr/local/google/home/tlively/code/llvm-project/cross-project-tests/lit.cfg.py", line 249, in <module>
    if StrictVersion(gdb_version_string) < StrictVersion('10.1'):
  File "/usr/lib/python3.9/distutils/version.py", line 40, in __init__
    self.parse(vstring)
  File "/usr/lib/python3.9/distutils/version.py", line 137, in parse
    raise ValueError("invalid version number '%s'" % vstring)
ValueError: invalid version number '10.1.90.20210103'

Before trying to upgrade my gdb, I had run this command:

$ ninja check-intrinsic-headers
[0/1] Running intrinsic header tests
...
XFAIL some tests: use gdb version >= 10.1 to restore test coverage

Testing Time: 0.04s
  Unsupported: 1

So aside from the problems with my invalid gdb version string, it looks like my test is still accidentally depending on having the correct gdb version.

Right now (after trying to upgrade my gdb) I get this error

$ bin/llvm-lit ~/code/llvm-project/cross-project-tests/intrinsic-header-tests/wasm_simd128.c
...
llvm-lit: /usr/local/google/home/tlively/code/llvm-project/llvm/utils/lit/lit/TestingConfig.py:103: fatal: unable to parse config file '/usr/local/google/home/tlively/code/llvm-project/cross-project-tests/lit.cfg.py', traceback: Traceback (most recent call last):
  File "/usr/local/google/home/tlively/code/llvm-project/llvm/utils/lit/lit/TestingConfig.py", line 92, in load_from_path
    exec(compile(data, path, 'exec'), cfg_globals, None)
  File "/usr/local/google/home/tlively/code/llvm-project/cross-project-tests/lit.cfg.py", line 249, in <module>
    if StrictVersion(gdb_version_string) < StrictVersion('10.1'):
  File "/usr/lib/python3.9/distutils/version.py", line 40, in __init__
    self.parse(vstring)
  File "/usr/lib/python3.9/distutils/version.py", line 137, in parse
    raise ValueError("invalid version number '%s'" % vstring)
ValueError: invalid version number '10.1.90.20210103'

This SO answer states that StrictVersion is undocumented (https://stackoverflow.com/a/11887885), and should be avoided in favour of something else. I'm inclined to agree, although don't have a clear answer as to what that something else should be. The docstrings (https://github.com/python/cpython/blob/main/Lib/distutils/version.py) of StrictVersion require the version number to be made up of exactly 2 or 3 components, which that gdb docstring does not conform to. I think you're going to need to fix this part of the config to resolve this, to do the version check some other way (LooseVersion should be sufficient, although that is as undocumented as StrictVersion).

Before trying to upgrade my gdb, I had run this command:

$ ninja check-intrinsic-headers
[0/1] Running intrinsic header tests
...
XFAIL some tests: use gdb version >= 10.1 to restore test coverage

Testing Time: 0.04s
  Unsupported: 1

So aside from the problems with my invalid gdb version string, it looks like my test is still accidentally depending on having the correct gdb version.

I don't think so: that "XFAIL some tests" message is to do with gdb versions, and not to do with the individual tests being run (see the code around here https://github.com/llvm/llvm-project/blob/989f1c72e0f4236ac35a35cc9998ea34bc62d5cd/cross-project-tests/lit.cfg.py#L253). Your test is probably UNSUPPORTED for other reasons. If you run llvm-lit -a on the test directly, rather than via a check target, you'll be told what the reason for it being marked UNSUPPORTED is. My expectation is that webassembly-registered-target is not an available feature. This is because the *-registered-targets features are part of the main llvm/test (and equivalent clang/test etc) lit.cfg.py, not a part of lit itself. Consequently, such features don't exist within other projects automatically. You'll see code like https://github.com/llvm/llvm-project/blob/bab468f2536219ec824a2b6fe959fcaa7abf5685/llvm/test/lit.cfg.py#L201 in llvm/test/lit.cfg.py to enable it. You will need to add this to the cross-project-tests lit.cfg.py to get these features.

Hope that helps.

tlively updated this revision to Diff 415952.Mar 16 2022, 1:18 PM
  • Update version check and available_features in lit.cfg.py

Thanks, @jhenderson! You were right on all counts and I have the test working now. This should be ready for proper review.

tlively retitled this revision from [WIP][WebAssembly] Add end-to-end codegen tests for wasm_simd128.h to [WebAssembly] Add end-to-end codegen tests for wasm_simd128.h.
tlively edited the summary of this revision. (Show Details)

Test config changes look good. I don't really know anything about wasm to advise about the test itself.

aheejin accepted this revision.Mar 17 2022, 2:47 PM
This revision is now accepted and ready to land.Mar 17 2022, 2:47 PM