[PATCH] D52030: [clangd] Test benchmark if we can build it

Kirill Bobyrev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Sep 13 07:36:37 PDT 2018


kbobyrev added a subscriber: rnk.
kbobyrev added a comment.

In https://reviews.llvm.org/D52030#1233349, @sammccall wrote:

> In https://reviews.llvm.org/D52030#1233323, @kbobyrev wrote:
>
> > Looks good, thanks!
> >
> > While we're here: I'm wondering whether we should also introduce very basic test which would just run `clangd-indexer` since it doesn't depend on benchmarks and would be run on all buildbots.
>
>
> Isn't that this test?


Yes, but this test checks both. As mentioned, buildbots don't build benchmarks by default and hence this testcase is not actually run anywhere at the moment.

> 
> 
>> Also, I was thinking whether we should ask some buildbot owners to enable `LLVM_BUILD_BENCHMARKS`, but it probably makes sense when we have more benchmarks (ideally, across different LLVM parts).
> 
> Hmm, I thought that was already the case. Isn't the benchmark stuff only not-built on the windows bots?

`LLVM_BUILD_BENCHMARKS` (which adds benchmarks to the list of default targets) is `OFF` on all platforms by default (although `LLVM_INCLUDE_BENCHMARKS` which produces build targets in the first place is enabled on Windows now, thanks to @rnk; it seems that the stage 2 bug we encountered was fixed). Hence, the benchmark library itself is built on all platforms (same as gtest library), but assuming that buildbots run `ninja`/`make` this only compiles default targets (which exclude `IndexBenchmark` due to `LLVM_BUILD_BENCHMARKS` being `OFF` by default). Hence, we should either ask buildbot owners to set `LLVM_BUILD_BENCHMARKS` to `ON` or don't exclude benchmarks from the list of default targets (which might result in some noise due to the full build time increase and is probably not what we want to do.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52030





More information about the cfe-commits mailing list