Page MenuHomePhabricator

Introduce a direct LLVM IR execution UnitTests framework, and add the first such test.
Needs ReviewPublic

Authored by chandlerc on Apr 27 2018, 7:13 PM.

Details

Reviewers
rnk
hfinkel
Summary

The framework should allow us to easily add execution test coverage to
LLVM without having to necessarily be able to convince a particular
frontend to generate a particular IR pattern. Instead, the tests can be
written directly in LLVM IR and converted to bitcode to avoid churn when
the textual IR format changes in uninteresting ways.

This also adds the first test which does basic large integer math to
exercise the legalization logic in LLVM. Right now we have massive
FileCheck tests in LLVM that provide low value and are impossible to
review updates for because they are checking each individual
instruction. Instead, we can use execution tests to both ensure that
LLVM can successfully code generate these IR patterns, and that the
resulting instructions behave as expected.

For example, this test uses a specific pattern of inputs designed to
stress test the carry handling in an large integer multiplication.

Event Timeline

chandlerc created this revision.Apr 27 2018, 7:13 PM

I wonder whether it's better/useful to implement these eventually as Google Test / Google Mock matchers. Something to think about in the future maybe.

Bitcode/UnitTests/README.md
23–24

I don't see an update_ll.sh file, does it already exist in the parent directory?

Bitcode/UnitTests/large_int/driver.cpp
51–53

Reformat? I suspect you intended to indent the LHS.Chunks[i] = -1 statement?

Bitcode/UnitTests/update_bc.sh
1–4

My CMake-fu is not strong, but is it possible/valuable to make this run as part of the testing process? i.e. checking that we can generate the .bc files from the .ll files?

I wonder whether it's better/useful to implement these eventually as Google Test / Google Mock matchers. Something to think about in the future maybe.

I'm curious how you're imagining doing this? If there is a cleaner way of wiring this up I'm quite interested, as I think we may start to grow more of this style of test once the basic approach is fleshed out...

I don't want to rely on the JIT here, but I actually want to *execute* a program generated by LLVM...

craig.topper added inline comments.
Bitcode/UnitTests/README.md
3

Either "store" or "maintain" needs to be removed from the second sentence I think?

I wonder whether it's better/useful to implement these eventually as Google Test / Google Mock matchers. Something to think about in the future maybe.

I'm curious how you're imagining doing this? If there is a cleaner way of wiring this up I'm quite interested, as I think we may start to grow more of this style of test once the basic approach is fleshed out...

I don't think this is going to be that different from what you're already doing.

Essentially, you're defining extern (I presume extern "C" { ... }) function stubs which are then code-gen'ed later by some tool (llvm-as or something).

You can make the check(expected, output) be one of the Google Mock matchers, which can be used in the EXPECT_THAT(...) or ASSERT_THAT statements in Google Test. Reference: https://github.com/google/googletest/blob/master/googlemock/docs/CookBook.md#writing-new-parameterized-matchers-quickly

If these were Google Test unit tests, you'd get the benefit of being able to define the output format, and getting the benefit of the global fixtures along with re-use.

I don't want to rely on the JIT here, but I actually want to *execute* a program generated by LLVM...

Indeed -- the only difference would be that you can use the existing Google Test macros/matchers for some things, and making the raw byte/data matching more extensible/re-usable.

I can see matchers for the different LLVM-specific data types to be deserving of their own matchers and formatting in case of failure.

Keeping these tests as checked in bitcode is to ensure less churn as the IR changes since these tests are likely to be relatively static? Given all the IR in test cases we generally have to upgrade/modify when changing the format - is it much of a savings to have these in bitcode?

Bitcode/UnitTests/README.md
3

"We store maintain them" - guess you want only one of "store" or "maintain".

rnk added a comment.May 8 2018, 6:20 PM

I think the overall idea is great!

Keeping these tests as checked in bitcode is to ensure less churn as the IR changes since these tests are likely to be relatively static? Given all the IR in test cases we generally have to upgrade/modify when changing the format - is it much of a savings to have these in bitcode?

I think we might as well compile the .ll file instead of the .bc file instead of checking in the .bc. The IR printed from llvm-dis changes style frequently, but most boring .ll inputs have stayed valid over a long period of time.

AlexDenisov added inline comments.
Bitcode/UnitTests/update_bc.sh
1–4

It is certainly possible to do via CMake. One could add custom command/target, this way there won't be a need to store the bc file in the repo.