intel / graph-compiler

MLIR-based toolkit targeting intel heterogeneous hardware
Apache License 2.0
32 stars 15 forks source link

Run tests in CI #38

Open kurapov-peter opened 6 months ago

kurapov-peter commented 6 months ago
Devjiu commented 6 months ago

To run lit tests just build "gc-check"

cmake --build . --target gc-check

Currently it's just a stub - we don't have passes and correct use-cases to run lits.

Menooker commented 6 months ago

Shall we let llvm-lit to drive all of the tests including gtest? I have enabled a MLIR-related unittest at https://github.com/intel/graph-compiler/pull/44

leshikus commented 6 months ago

@Devjiu gc-check does not work for me, it reports

/bin/sh: 1: /llvm-lit: not found
FAILED: test/CMakeFiles/gc-check /runner/_work/graph-compiler/graph-compiler/build/test/CMakeFiles/gc-check 
cd /runner/_work/graph-compiler/graph-compiler/build/test && /llvm-lit -sv /runner/_work/graph-compiler/graph-compiler/build/test
ninja: build stopped: subcommand failed.

probably llvm should be rebuilt to add lit, or it should be made available in some other way. What is recommended way to resolve this?

Devjiu commented 6 months ago

@Devjiu gc-check does not work for me, it reports

/bin/sh: 1: /llvm-lit: not found
FAILED: test/CMakeFiles/gc-check /runner/_work/graph-compiler/graph-compiler/build/test/CMakeFiles/gc-check 
cd /runner/_work/graph-compiler/graph-compiler/build/test && /llvm-lit -sv /runner/_work/graph-compiler/graph-compiler/build/test
ninja: build stopped: subcommand failed.

probably llvm should be rebuilt to add lit, or it should be made available in some other way. What is recommended way to resolve this?

This pr added llvm-lit into build: https://github.com/intel/graph-compiler/pull/39/files So it should be in a tarball of llvm.

another way is to install lit in pip, but I recommend use the same lit as it was in llvm.

kurapov-peter commented 6 months ago

Shall we let llvm-lit to drive all of the tests including gtest? I have enabled a MLIR-related unittest at #44

What benefit does it bring?

Menooker commented 6 months ago

Shall we let llvm-lit to drive all of the tests including gtest? I have enabled a MLIR-related unittest at #44

What benefit does it bring?

It provides a unified entry of all tests, as done in LLVM project. Most (all?) FileCheck/Python/Gtest are driven by lit.

Another benefit is that, we can reuse the gtest headers and static libraries exported by LLVM, and we don't need to manage gtest as a 3rdparty on our own. We can use the cmake function add_unittest provided by LLVM for linking gtest.

Devjiu commented 6 months ago

Shall we let llvm-lit to drive all of the tests including gtest? I have enabled a MLIR-related unittest at #44

What benefit does it bring?

It provides a unified entry of all tests, as done in LLVM project. Most (all?) FileCheck/Python/Gtest are driven by lit.

Another benefit is that, we can reuse the gtest headers and static libraries exported by LLVM, and we don't need to manage gtest as a 3rdparty on our own. We can use the cmake function add_unittest provided by LLVM for linking gtest.

Currently we also need gtests that test the oneDNN API integration.

If in this case everything works fine, we can consider it; if not, then the idea of a common runner for all tests does not work and a different structure needs to be discussed.

We will also need integration tests that run the pipeline starting from the pytorch model, which can also be inconvenient to embed in cmake. It seems to me that we will have several test runners in any case, since there is a need for different types of testing.

In general, such functionality at the current stage is simply a matter of taste.

Menooker commented 6 months ago

In general, such functionality at the current stage is simply a matter of taste.

OK, sure! But I would suggest to drive all unittests directly testing MLIR with lit. I mean the C++ tests for the MLIR components. As it matches the MLIR upstream's way and makes it easier to upstream.

leshikus commented 5 months ago

@kurapov-peter should I add more tests to CI?

kurapov-peter commented 3 weeks ago

@kurapov-peter should I add more tests to CI?

I think we are missing default tests for mlir. Those are useful to catch broken llvm versions. Can be run with cmake --build build --target check-mlir for example.