Skip to content

Update google benchmark to v1.9.1 #222

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 27, 2025

Conversation

androm3da
Copy link
Member

Updated MicroBenchmarks/lib/benchmark/ using update_benchmark.sh with the change below:

diff --git a/MicroBenchmarks/libs/update_benchmark.sh b/MicroBenchmarks/libs/update_benchmark.sh
index b4caa3c9..c1f3e84f 100755
--- a/MicroBenchmarks/libs/update_benchmark.sh
+++ b/MicroBenchmarks/libs/update_benchmark.sh
@@ -9,8 +9,8 @@ echo
 read -p "Press a key to continue, or Ctrl+C to cancel"

 rm -rf benchmark
-git clone https://github.com/google/benchmark.git
-git clone https://github.com/google/googletest.git benchmark/googletest
+git clone --branch=v1.9.1 https://github.com/google/benchmark.git
+git clone --branch=v1.16.0 https://github.com/google/googletest.git benchmark/googletest
 rm -rf benchmark/googletest/.git*
 rm -rf benchmark/.git*

@androm3da androm3da requested a review from mtrofin March 18, 2025 04:36
@androm3da androm3da self-assigned this Mar 18, 2025
@mtrofin
Copy link
Member

mtrofin commented Mar 18, 2025

Assuming that all patches in LLVM made in the interim were also patched in google/benchmark, LGTM

@androm3da
Copy link
Member Author

Assuming that all patches in LLVM made in the interim were also patched in google/benchmark, LGTM

Oh, gee - I didn't consider that. Is there a way I can check that?

@MatzeB
Copy link
Contributor

MatzeB commented Mar 18, 2025

Should be easy to check the git log just for this directory: https://github.com/llvm/llvm-test-suite/commits/main/MicroBenchmarks/libs/benchmark

@androm3da
Copy link
Member Author

androm3da commented Mar 19, 2025

Should be easy to check the git log just for this directory: https://github.com/llvm/llvm-test-suite/commits/main/MicroBenchmarks/libs/benchmark

So, below are the commits which appear in that log since b1854df - Update benchmarks (3 years ago) <Mircea Trofin>.

  • AFAICT changes from MicroBenchmarks/libs/benchmark/test/benchmark_setup_teardown_test.cc that came from Fix Solaris compilation are still there and whatever changes happened to this file since c4c6c75 are expected from upstream. cc @rorth
  • I'm assuming the apply/revert pair Guard visibility variables on AIX can be ignored.
  • [test-suite] Add C-SKY Support in benchmark is present.
  • Fix MicroBenchmark build on Linux with clang 18.1.8 is missing, but I suppose I could re-apply this change? Or should it land upstream first?

Updated MicroBenchmarks/lib/benchmark/ using update_benchmark.sh with the
change below:

```
diff --git a/MicroBenchmarks/libs/update_benchmark.sh b/MicroBenchmarks/libs/update_benchmark.sh
index b4caa3c..c1f3e84f 100755
--- a/MicroBenchmarks/libs/update_benchmark.sh
+++ b/MicroBenchmarks/libs/update_benchmark.sh
@@ -9,8 +9,8 @@ echo
 read -p "Press a key to continue, or Ctrl+C to cancel"

 rm -rf benchmark
-git clone https://github.com/google/benchmark.git
-git clone https://github.com/google/googletest.git benchmark/googletest
+git clone --branch=v1.9.1 https://github.com/google/benchmark.git
+git clone --branch=v1.16.0 https://github.com/google/googletest.git benchmark/googletest
 rm -rf benchmark/googletest/.git*
 rm -rf benchmark/.git*
```
@androm3da androm3da force-pushed the bcain/new_goog_benchmark branch from 6c4a505 to 419e7da Compare March 21, 2025 00:45
@androm3da
Copy link
Member Author

@mtrofin @MatzeB I suppose I should re-apply 3ed6810 on top of this update?

@mtrofin
Copy link
Member

mtrofin commented Mar 26, 2025

@mtrofin @MatzeB I suppose I should re-apply 3ed6810 on top of this update?

I suppose so. FYI @omjavaid - could you please make sure patches to the internal benchmark are paired with patches to its original (google/benchmark) repo? (also @jroelofs as fyi, as was reviewer of the above) - Thanks!

@DavidSpickett
Copy link
Contributor

FYI, seems unlikely that you wanted to tag @omjavaid there, perhaps someone else beginning with o ?

MicroBenchmarks/libs/benchmark/test/options_test.cc fails to build
on Linux/AArch64 with following error:
error: variable 'actual_iterations' set but not used

This patch adds benchmark::DoNotOptimize(actual_iterations); to
to function BM_explicit_iteration_count in options_test.cc

-Wall and -Werror were being used to compile and I am surprised that
this was not caught by any of the buildbots.
Some versions of clang compile this all fine with -Wall -Werror.
@androm3da
Copy link
Member Author

FYI, seems unlikely that you wanted to tag @omjavaid there, perhaps someone else beginning with o ?

AFAICT that is the author of 3ed6810 - so, no - that's probably the right person tagged.

@androm3da androm3da requested a review from mtrofin March 27, 2025 04:08
@androm3da androm3da merged commit 97d90b5 into llvm:main Mar 27, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants