[PATCH] D119547: [libc][bazel] Add tests to the bazel build

Guillaume Chatelet via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 10 09:04:12 PST 2022


gchatelet added inline comments.


================
Comment at: utils/bazel/llvm-project-overlay/libc/test/src/math/libc_math_test_rules.bzl:36
+        additional_deps = additional_deps,
+        size = "enormous",  # Some tests are emulated and take very long.
+    )
----------------
gchatelet wrote:
> GMNGeoffrey wrote:
> > gchatelet wrote:
> > > GMNGeoffrey wrote:
> > > > Oh no 😨 How much does this extend CI runtime? Also, how many tests are like this? Instead of hardcoding "enormous" for every test, could you just pass that argument for the few that are in fact enormous? Separately, I think it is a better fit to use the `timeout` argument here `timeout = "eternal"`. Test size is ill-defined, but I think by any reasonable definition this test isn't an enormous one. Those usually involve spinning up whole servers and such. I think it's a small test that just happens to take a very long time.
> > > It is not //that// bad. Here are the top tests sorted by runtime:
> > > ```
> > > @llvm-project//libc/test/src/math:hypot_test                             PASSED in 44.1s
> > > @llvm-project//libc/test/src/math:hypotf_test                            PASSED in 31.4s
> > > @llvm-project//libc/test/src/math:remquol_test                           PASSED in 19.7s
> > > @llvm-project//libc/test/src/math:sincosf_test                           PASSED in 12.8s
> > > @llvm-project//libc/test/src/math:sqrtf_test                             PASSED in 6.3s
> > > @llvm-project//libc/test/src/math:sqrt_test                              PASSED in 6.0s
> > > @llvm-project//libc/test/src/math:sqrtl_test                             PASSED in 4.8s
> > > @llvm-project//libc/test/src/math:sinf_test                              PASSED in 3.8s
> > > ...
> > > ```
> > > 
> > > @lntue can we add the size tag only on the few bad ones?
> > Thos are all within the timeouts for "short", which is implied by "small". It doesn't seem like any of these should be tagged "enormous"
> So I've run more tests on different archs and build modes.
> - for local machine `x86_64` test time is < 45s in opt and < 80s in fastbuild.
> - for local machine `aarch64` test time is < 45s in optand < 87s in fastbuild.
> - for Google `aarch64` test time is < 100s in opt and < 160s in fastbuild.
> - for Google `qemued aarch64` test time is < 30m in opt and  timing out after 1h in fastbuild.
> 
> So indeed these are //enormous// but only when run through an emulator. Let's mark the bigger tests wit the correct `size` and handle the timeouts internally.
Actually I'm just removing the size attribute as the default is fine for all tests here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119547/new/

https://reviews.llvm.org/D119547



More information about the llvm-commits mailing list