[libc-commits] [PATCH] D117028: [libc] Add multithreading support for exhaustive testing and MPFRUtils.

Tue Ly via Phabricator via libc-commits libc-commits at lists.llvm.org
Wed Jan 12 22:56:21 PST 2022

lntue marked an inline comment as done.
lntue added inline comments.

Comment at: libc/test/src/math/exhaustive/exhaustive_test.h:26
+  // Pointer to the testing method to be used by `test_full_range`.
+  void (LlvmLibcExhaustiveTest::*check)(T, T);
sivachandra wrote:
> Why is using a method pointer over an overridable virtual method with an additional rounding mode arg better? FWIW, method pointers are like virtual functions.
See the comment below in logf_test.cpp.

Comment at: libc/test/src/math/exhaustive/logf_test.cpp:26
+  template <mpfr::RoundingMode Rounding>
+  void check(uint32_t start, uint32_t stop) {
+    int old_rounding = fegetround();
sivachandra wrote:
> lntue wrote:
> > sivachandra wrote:
> > > It seems to me that this function need not be a template? As in, the rounding mode can be an arg? You will need to add a rounding mode arg to `testFullRange` also I think.
> > I don't think rounding mode information should be included in test_full_range, as their sole purpose is to break down the input range and run it in parallel.  So in order to move test_full_range implementation to .cpp file, the testing function has to have a fixed signature.  That leave template parameter as the cleanest way to pass rounding mode to each testing function.
> I am not sure what "including" rounding mode information in `test_full_range` would me. But, with the virtual function setup, it will be like this:
> ```
> test_full_range(..., RoundingMode rounding_mode) {
>   check(..., rounding_mode);
> }
> ```
> And it will avoid all the function pointers in the tests below. Also, the template arg is not deducible so there is not benefit to making it a template.
> An additional note:  Tests which don't need rounding mode will just ignore that arg - so there has to be a default.
It's exactly passing rounding_mode directly to test_full_range that I didn't like and preferred method pointer over virtual function.  Since if other tests need different parameters to execute, similar to rounding_mode, then they will have to pass to test_full_range, and all other overridden functions would have to be updated to carry more unused parameters. 

Without template parameters (since its implementations is not in the header) and std::function, I found method pointers is the next easiest way to allow each test to add their own required parameters individually without affecting other tests.

Nonetheless, I updated to use virtual function in here, and we can change to method pointers later if more tests need different parameters.

  rG LLVM Github Monorepo



More information about the libc-commits mailing list