Page MenuHomePhabricator

Add benchmarks for 128 bit division and create a directory for all builtins benchmarks
ClosedPublic

Authored by danlark on Jul 2 2020, 2:30 AM.

Details

Reviewers
courbet
MaskRay
Summary

Event Timeline

danlark created this revision.Jul 2 2020, 2:30 AM
danlark updated this revision to Diff 275025.Jul 2 2020, 2:31 AM

Remove unnecessary header

Harbormaster completed remote builds in B62637: Diff 275025.
courbet added inline comments.Jul 2 2020, 2:57 AM
MicroBenchmarks/Builtins/Int128/main.cpp
17

Why is the default mt19937 not fine ? (please add a comment)

26

Could we make H a provate typedef inside the class ? (It's not used outside, and I don;t see any specialization)

39

H looks unused.

41

Can you add some doc ?

62

Use kSampleSize instead of values.size() for more accurate results (the DoNotOptimize call above will likely force the compiler to recompute the size at every iteration).

Also it would be better to make kSampleSize a power of two and do: i = (i+1)%kSampleSize to avoid the branch altogether.

67

why not __uint128_t ?

danlark updated this revision to Diff 275040.Jul 2 2020, 3:26 AM
danlark marked 7 inline comments as done.

Fix comments

MicroBenchmarks/Builtins/Int128/main.cpp
17

You are right, does not matter

courbet accepted this revision.Jul 7 2020, 2:41 AM
This revision is now accepted and ready to land.Jul 7 2020, 2:41 AM

Could you submit it for me? I don't have commit rights for now. danilak@google.com

MaskRay added inline comments.Jul 7 2020, 9:39 AM
MicroBenchmarks/Builtins/Int128/main.cpp
42

What if a=b=1? max(a,b) < max(2,min(a,b))

MaskRay added inline comments.Jul 7 2020, 9:43 AM
MicroBenchmarks/Builtins/Int128/CMakeLists.txt
2

You can set 14 because LLVM requires 14 now..
MSVC friendly alternative:

set(CMAKE_CXX_STANDARD 14)
set(CMAKE_CXX_STANDARD_REQUIRED TRUE)
danlark updated this revision to Diff 276120.Jul 7 2020, 10:14 AM
danlark marked 3 inline comments as done.

Fix comments

danlark added inline comments.Jul 7 2020, 10:14 AM
MicroBenchmarks/Builtins/Int128/main.cpp
42

The probability of that is 2^{-254} (a <= 1 and b <= 1) and it does not matter at that point, overall updated to match the comment

MaskRay accepted this revision.Jul 7 2020, 5:28 PM

Thanks!