[PATCH] D32272: [XRay] Add Google Benchmark library + initial XRay benchmarks

[Dean Michael Berris via Phabricator via llvm-commits]

dberris added a comment.

In https://reviews.llvm.org/D32272#739271, @kristof.beyls wrote:

> Could you explain the rationale for adding libs/benchmarks-1.1.0?


I mentioned this a while back in the mailing list (http://lists.llvm.org/pipermail/llvm-dev/2016-September/104392.html) and it was thought that adding google benchmarks to test-suite seemed like it would have been fine. We've written the micro-benchmark to use a benchmarking library that was widely available and easy to use (and familiar to me). There seemed to be very little opposition to adding them to the test-suite at the time -- based on the discussion in the thread.

> Also, since that seems to have a different license, you probably need to add that directory to the top-level LICENSE.TXT file as one of the sub-directories with "additional or alternate copyrights,licenses, and/or restrictions".

Good point, I agree 100%.



================
Comment at: MultiSource/Benchmarks/XRay/CMakeLists.txt:6-7
+
+set(PROG retref-bench-O0)
+list(APPEND CPPFLAGS -O0)
+llvm_multisource()
----------------
kristof.beyls wrote:
> It looks weird to me that this CMakeFile explicitly set different targets for -O0, -O1, -O2, -O3. I don't think other programs in the test-suite do this.
> Do you have a good reason to do it like this?
> It seems to go slightly against the general philosophy of making the test-suite an as-normal-as-possible cmake project; where you would set optimization level during cmake configuration; and if you want to collect data for multiple optimization levels build the test-suite multiple times with different cmake configurations.
> @MatzeB : do you have thoughts on this?
Now that you mention it, this does seem weird. This is mostly a faithful reproduction of my attempts at doing this with a shell script. @eizan has graciously volunteered to port this into something that can be part of the LLVM test suite.

If it would be more acceptable to just have a single target, then that's something we ought to change this to.


https://reviews.llvm.org/D32272