valgrind for lit tests

[leissa]

We should definitely enable valgrind for lit tests. It's just too easy with C++ to make some stupid error. Currently, I'm investigating some bugs that wouldn't have slipped through with valgrind enabled.

[fodinabor]

In theory does lit provide a --vg parameter that can be used to say, please run the tests inside with valgrind.
That also works on my machine (probably won't on Windows..?) but I'm not sure how often we want to use that as the performance is obviously awful... it also runs clang or whatever else we specify to run in the RUN lines with valgrind (although thorin clearly dominated the testing time..).
Timings of running the lit tests on my 16 thread Ryzen 7 5850u:

Config	--vg	No --vg
Debug	20min	22s
Release	200s	17s

So I'd provide either a CMake option to add the parameter or just provide a check-valgrind target or so?
Not sure if we want to run that in the CI though... maybe in the Release build?
What are your thoughts?

[leissa]

args. 20 minutes is really bad.

Maybe we can disable running clang and what not from being run inside valgrind by somehow passing and fiddling around with the --trace-children and --trace-children-skip options?

Another idea that just came to my mind is to either explicitly include valgrind within our RUN: %thorin command or - even better - have a another script that looks for RUN: %thorin commands and runs just these commands with valgrind interdependently from the lit stuff.

[fodinabor]

given that we're down to 200s total, when building Thorin in Release, I would claim valgriding clang does not hurt too much.

I assume we could still build our own solution to just valgrind thorin: since we set the value of %thorin in the lit config ourselves, we could also have some config option that replaces that with valgrind ... /path/to/thorin instead of just /path/to/thorin.
That obviously won't change much for that in debug thorin is the dominating factor.

[leissa]

since we set the value of %thorin in the lit config ourselves, we could also have some config option that replaces that with valgrind ... /path/to/thorin instead of just /path/to/thorin.

I really like this approach.

That obviously won't change much for that in debug thorin is the dominating factor.

I don't really get why this takes so long. make check takes 21s on my old workstation here. Now using valgrind on Thorin alone would probably have an impact of an order of magnitude or so but not 2 orders of magnitudes???

[fodinabor]

Release with %thorin set to valgrind /path/to/thorin takes 75s.
Debug 16m. (haven't rerun and averaged these numbers obvs..)

[leissa]

So my vote would be to enable valgrind for release builds in GitHub CI and by enabling it simply using valgraind /path/to/thorin.

My guess is that this dynamic loading of plugins is really, really slow with valgrind.