This is an archive of the discontinued LLVM Phabricator instance.

[libc][math] Improved ExhaustiveTest performance.
ClosedPublic

Authored by orex on Jul 1 2022, 7:44 AM.

Details

Summary

Previous implementation splits value ranges around threads. Because of
very different performance of testing functions over different ranges,
CPU utilization were poor. Current implementation split test range
over small pieces and threads take the pieces when they finish with
previous. Therefore the CPU load is constant during testing.

Diff Detail

Event Timeline

orex created this revision.Jul 1 2022, 7:44 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJul 1 2022, 7:44 AM
orex requested review of this revision.Jul 1 2022, 7:44 AM
lntue added inline comments.Jul 1 2022, 8:11 AM
libc/test/src/math/exhaustive/exhaustive_test.cpp
42–44

This might be overflow when stop < increment, since both are unsigned?

54

It is actually convenient to display the tested range whether it passed or not, so that we can save time by only running the failed ranges again while developing. Also if you build the message separately before passing all at once to std::cout, you will not need this lock, since a single << for std::cout is guaranteed to be atomic.

libc/test/src/math/exhaustive/exhaustive_test.h
22

Probably this should be configurable from the caller side. Few possibilities:

  1. add increment to the last parameter of test_full_range with default value.
  2. move rounding mode to a template parameter of test_full_range and add increment to the last parameter of test_full_range with default value.
  3. add increment to as a template parameter of test_full_range with default value.
  4. add both rounding mode and increment to template parameters of test_full_range with default value for increment.

Among these, I think option 2 will make the caller sides look the best. What do you think?

orex added inline comments.Jul 1 2022, 8:23 AM
libc/test/src/math/exhaustive/exhaustive_test.cpp
42–44

Agree. Thank you.

54

That will be a lot of ranges, which is passed. I don't think it will be good to display them. Also you always have a values, which is not passed from testing macros, so you can use them to check problems.

lntue added inline comments.Jul 1 2022, 8:35 AM
libc/test/src/math/exhaustive/exhaustive_test.cpp
54

Or just display the failed ranges? So that you can redirect the outputs to a file and find the exact / smallest ranges to skip all the passed one? Actually with your default option, 2^12 ranges is not too bad for output to a file.

orex updated this revision to Diff 441707.Jul 1 2022, 9:06 AM

Fix some reviewer comments.

orex added inline comments.Jul 1 2022, 9:23 AM
libc/test/src/math/exhaustive/exhaustive_test.cpp
54

Good proposition. Done.

libc/test/src/math/exhaustive/exhaustive_test.h
22

I can hardly imagine, that developers in general should take care about this. It looks like a technical thing. Anyhow I understand you point. I propose another solution. Make it virtual function, which default value in base class. If you would like to change the parameter, you can overload the function together with check function, which you should overload. What do you think?

About the template parameter of rounding. It is a good idea, but probably, we can think about to do all rounding and ranges with one command? Also, it is good to combine everything in one function like exec?
And you pass ranges and rounding as overload functions in check implementation class?

So, in derivative class you overload array of rounding, array of ranges and check function, of course.

But, let's do as you always said several small steps.)))))

lntue accepted this revision.Jul 1 2022, 9:29 AM
This revision is now accepted and ready to land.Jul 1 2022, 9:29 AM
This revision was automatically updated to reflect the committed changes.