diffkemp icon indicating copy to clipboard operation
diffkemp copied to clipboard

Cleanup LLVM passes for snapshot building

Open viktormalik opened this issue 1 year ago • 11 comments

We now have two commands for creating snapshots that use Clang to build the analysed project: build and build-kernel. Each of these uses a different approach to run LLVM passes:

  • build respects whatever is the default for the analysed project. This may result in insufficient passes being applied (e.g. if -O0 is used).
  • build-kernel uses -O1 -Xclang -disable-llvm-passes and then applies custom passes by running opt (see kernel_llvm_source_builder.py:412). It originally used -O0 but 91f72dd changed that b/c of LLVM 5.0. We don't support LLVM 5.0 anymore so that can be probably dropped.

We should unify both (and any future) commands to use a common approach. The build-kernel one allows more control over what's being run, the question is whether it gives any advantage over simple -O2. Some experiments on that would be useful.

viktormalik avatar Sep 29 '23 11:09 viktormalik

It looks like it will probably not be possible to use -O2.

I changed build-kernel to use -O2 and to not use custom passes (run opt). For the regression (pytest) tests, 28 tests failed (29 for LLVM 12 and below). It would probably be necessary to go through the tests more thoroughly to be certain that the reason is not different.

In any case, by using -O2 it would probably be harder to identify exactly which function caused the semantic difference because the compiler can inline the functions (as I saw on EqBench benchmarks analysis). This could maybe cause better results but for the cost of not exactly knowing which function caused semantic difference -- I am not sure if the inlining is the reason for better results it would be necessary to analyse what is the cause of better results with -O2 (which is the case in EqBench benchmarks).

PLukas2018 avatar Oct 09 '23 05:10 PLukas2018

Thanks for the analysis! It's possible that it'd just require to add one or two specific passes for the tests to pass. It may also be the case that -O2 is too aggressive.

Also, you're probably right with the inlining. Since we intend to identify the function with differences as precisely as possible, we want to have inlining under control (which we probably wouldn't have with -O2).

There're several things that come to mind. For instance, running llvm-as < /dev/null | opt -O2 -disable-output -print-pipeline-passes gives you the complete list of passes run with -O2. We could take those (it's a long list, though) and either just remove inline and run everything else or to identify the passes that we're missing at the moment (to reproduce the EqBench results with -O2). Especially the latter would be quite time-consuming, though.

I'm getting inclined to using our custom set of passes both for build and build-kernel by default and allowing to specify -O2 for build if user wants to.

FWIW, let's keep this issue open to track the problem.

viktormalik avatar Oct 09 '23 06:10 viktormalik

@lenticularis39 any opinions?

viktormalik avatar Oct 09 '23 06:10 viktormalik

  • build-kernel uses -O1 -Xclang -disable-llvm-passes and then applies custom passes by running opt (see kernel_llvm_source_builder.py:412). It originally used -O0 but 91f72dd changed that b/c of LLVM 5.0. We don't support LLVM 5.0 anymore so that can be probably dropped.

It looks like it is not possible to get back to -O0 because then the passes (eg. mem2reg) do not work and the tests do not pass. I do not know what is the exact reason, maybe because with -O0 the LLVM IR does not contain tbaa metadata (information about types) or maybe there is a different reason.

PLukas2018 avatar Nov 21 '23 17:11 PLukas2018

I have started to work on the unification of program compilation in the build/build-kernel command. I have one question (@viktormalik , @lenticularis39 ), do we want to keep using RPython for compiling cc_wrapper into binary or could we switch to something else (eg. Cython) which would allow us to use python3. The reason why I am asking is that if we stick with RPython, then all shared parts of build/build-kernel commands would have to be RPython (python2) compatible.

PLukas2018 avatar Nov 21 '23 17:11 PLukas2018

It looks like it is not possible to get back to -O0 because then the passes (eg. mem2reg) do not work and the tests do not pass. I do not know what is the exact reason, maybe because with -O0 the LLVM IR does not contain tbaa metadata (information about types) or maybe there is a different reason.

Thanks for checking, let's keep the current version, then.

I have one question (@viktormalik , @lenticularis39 ), do we want to keep using RPython for compiling cc_wrapper into binary or could we switch to something else (eg. Cython) which would allow us to use python3. The reason why I am asking is that if we stick with RPython, then all shared parts of build/build-kernel commands would have to be RPython (python2) compatible.

If we could replace RPython with something Python3-compatible while achieving at least comparable performance, then let's do it. I've already fought with RPython several times (RPM build, Nix flakes), sometimes without success.

viktormalik avatar Nov 22 '23 06:11 viktormalik

Using Cython should be possible, it seems to work:

$ cython --embed diffkemp/building/cc_wrapper.py
$ gcc -I/usr/include/python3.12 diffkemp/building/cc_wrapper.c -lpython3.12 -o build/cc_wrapper-c
$ build/cc_wrapper-c
cc_wrapper: fatal error: missing environmental variable

I used RPython because I was familiar with how it works and that it will bring the performance I wanted. It's likely to be the same with Cython though.

lenticularis39 avatar Nov 22 '23 07:11 lenticularis39

Thank you both for your responses.

I have tried Cython, Nuitka and Mypyc so far I was unsuccessful in getting much better performance than Python has, so I am leaving it as it is for now. I think that maybe with Cython it would be possible, but it would be probably necessary to use C data types and C functions or maybe it would be easier to rewrite it completely eg. in C.

Here are the times I was able so far get with each "tool" for building a snapshot for musl v1.2.4:

Python3 RPython Cython Nuitka Mypyc
3:53.29 2:51.92 3:48.75 3:54.46 3:50.49

PLukas2018 avatar Dec 10 '23 19:12 PLukas2018

It looks like it is not possible to get back to -O0 because then the passes (eg. mem2reg) do not work and the tests do not pass. I do not know what is the exact reason, maybe because with -O0 the LLVM IR does not contain tbaa metadata (information about types) or maybe there is a different reason.

I tried the clang option -Xclang -disable-O0-optnone (which @ viktormalik discovered) with -O0. It looks like the passes then work, but the regression tests still fail (3/148, they are supposed to be equal but DiffKemp evaluates them as not-equal). For a program which I tried (not the one from the regression tests) the llvm is not much different, the llvm still does not include the tbaa metadata and there are a few other differences, so maybe the problem are the missing tbaa metada...

PLukas2018 avatar Dec 10 '23 20:12 PLukas2018

For a program which I tried (not the one from the regression tests) the llvm is not much different, the llvm still does not include the tbaa metadata and there are a few other differences, so maybe the problem are the missing tbaa metada...

I don't think that we rely on the tbaa metadata at all. Could you point me to the cases which are failing?

viktormalik avatar Dec 11 '23 10:12 viktormalik

I don't think that we rely on the tbaa metadata at all. Could you point me to the cases which are failing?

It does not have to be necessary problem with the metadata, there are some other differences. Here is the result of the tests:

tests/regression/functions_test.py:78: AssertionError
=========================== short test summary info ============================
FAILED tests/regression/functions_test.py::test_function_diff[rhel-82-83-patterns___put_page] - assert <Kind.NOT_EQUAL: 3> == <Kind.EQUAL: 1>
 +  where <Kind.NOT_EQUAL: 3> = <diffkemp.semdiff.result.Result object at 0x7ffff09688e0>.kind
 +  and   <Kind.EQUAL: 1> = <regression.task_spec.FunctionSpec object at 0x7ffff60fa380>.result
FAILED tests/regression/functions_test.py::test_function_diff[rhel-82-83-patterns_bio_endio] - assert <Kind.NOT_EQUAL: 3> == <Kind.EQUAL: 1>
 +  where <Kind.NOT_EQUAL: 3> = <diffkemp.semdiff.result.Result object at 0x7ffff0959270>.kind
 +  and   <Kind.EQUAL: 1> = <regression.task_spec.FunctionSpec object at 0x7ffff60fb760>.result
FAILED tests/regression/functions_test.py::test_function_diff[rhel-81-82-patterns_down_read] - assert <Kind.NOT_EQUAL: 3> == <Kind.EQUAL: 1>
 +  where <Kind.NOT_EQUAL: 3> = <diffkemp.semdiff.result.Result object at 0x7ffff097bb80>.kind
 +  and   <Kind.EQUAL: 1> = <regression.task_spec.FunctionSpec object at 0x7ffff0a8f010>.result
================== 3 failed, 145 passed in 887.36s (0:14:47) ===================

and here is a link to GitHub workflow run, btw it fails for other llvm/clang versions too (not only 9).

PLukas2018 avatar Dec 11 '23 10:12 PLukas2018

Closed by #311.

viktormalik avatar Jun 28 '24 11:06 viktormalik