Enzyme icon indicating copy to clipboard operation
Enzyme copied to clipboard

Update benchmarks

Open ZuseZ4 opened this issue 1 year ago • 6 comments

Splitting this out of the rust-benchmarks to make it easier to upstream rust stuff on it's own.

closes #1414

ZuseZ4 avatar Aug 09 '24 23:08 ZuseZ4

@wsmoses


CMake Error at /usr/lib/llvm-16/lib/cmake/llvm/AddLLVM.cmake:965 (add_executable):
  Target "enzyme-tblgen" links to target "zstd::libzstd_shared" but the
  target was not found.  Perhaps a find_package() call is missing for an
  IMPORTED target, or an ALIAS target is missing?
Call Stack (most recent call first):
  /usr/lib/llvm-16/lib/cmake/llvm/TableGen.cmake:146 (add_llvm_executable)
  tools/enzyme-tblgen/CMakeLists.txt:9 (add_tablegen)

and

➜  Enzyme git:(update-benchmarks) rg find_package . 
./enzyme/CMakeLists.txt
69:find_package(LLVM REQUIRED CONFIG)
188:    find_package(Clang REQUIRED CONFIG PATHS ${Clang_DIR} NO_DEFAULT_PATH)
200:    find_package(MLIR REQUIRED CONFIG PATHS ${MLIR_DIR} NO_DEFAULT_PATH)

./enzyme/test/test_find_package/CMakeLists.txt
5:find_package(Enzyme REQUIRED CONFIG NO_DEFAULT_PATH)

./diff.txt
13:@@ -71,14 +72,14 @@ find_package(LLVM REQUIRED CONFIG)

./enzyme/test/CMakeLists.txt
16:add_custom_target(test-cmake COMMAND rm -rf ${CMAKE_CURRENT_BINARY_DIR}/test_cmake && mkdir ${CMAKE_CURRENT_BINARY_DIR}/test_cmake && cd ${CMAKE_CURRENT_BINARY_DIR}/test_cmake &&  ${CMAKE_COMMAND} -S ${CMAKE_CURRENT_BINARY_DIR}/test_cmake ${CMAKE_CURRENT_SOURCE_DIR}/test_find_package -DEnzyme_DIR=${CMAKE_BINARY_DIR} -DCMAKE_C_COMPILER=$<TARGET_FILE:clang> && ${CMAKE_COMMAND} --build ${CMAKE_CURRENT_BINARY_DIR}/test_cmake DEPENDS ClangEnzymeFlags LLDEnzymeFlags)
18:add_custom_target(test-cmake COMMAND rm -rf ${CMAKE_CURRENT_BINARY_DIR}/test_cmake && mkdir ${CMAKE_CURRENT_BINARY_DIR}/test_cmake && cd ${CMAKE_CURRENT_BINARY_DIR}/test_cmake &&  ${CMAKE_COMMAND} -S ${CMAKE_CURRENT_BINARY_DIR}/test_cmake ${CMAKE_CURRENT_SOURCE_DIR}/test_find_package -DEnzyme_DIR=${CMAKE_BINARY_DIR} -DCMAKE_C_COMPILER=$<TARGET_FILE:clang> && ${CMAKE_COMMAND} --build ${CMAKE_CURRENT_BINARY_DIR}/test_cmake DEPENDS ClangEnzymeFlags)
➜  Enzyme git:(update-benchmarks) rg zstd
enzyme/WORKSPACE
39:    name = "llvm_zstd",
40:    build_file = "@llvm-raw//utils/bazel/third_party_build:zstd.BUILD",
42:    strip_prefix = "zstd-1.5.2",
44:        "https://github.com/facebook/zstd/releases/download/v1.5.2/zstd-1.5.2.tar.gz"

I won't look further into such cmake issues, but if you're ok with merging it despite those (or fixing them yourself), I can fix the unsupported flag combinations. As you can see it's already broken in the other PR where I just enable the benchmarks they are already broken, so fixing at least some things is IMHO a strict improvement.

ZuseZ4 avatar Sep 05 '24 05:09 ZuseZ4

Looks like an issue in setup for the script, can you copy whatever config from other CI ensures zlib is installed?

wsmoses avatar Sep 05 '24 18:09 wsmoses

The current status is that no benchmark CI is working: https://github.com/EnzymeAD/Enzyme/actions/runs/10607142537/job/29399074787?pr=2056 With this PR at least 4 Benchmarks are run under llvm-16, so I'd recommend to merge it. Happy to disable the still broken benchmarks, so someone else can pick them up once they have time.

ZuseZ4 avatar Sep 09 '24 02:09 ZuseZ4

lmk how to disable the benchmarks I haven't touched and which are thus still broken and this is good to merge and should even pass CI for 15 and 16.

ZuseZ4 avatar Sep 16 '24 01:09 ZuseZ4

It .. doesn't, can you check again, or point out where exactly it uses ClangEnzyme? :D This should be cosmetic and just makes sure that these tests here at least build again. It still uses LLVM-Enzyme for opt. I sometimes define LOADCLANG="%loadClangEnzyme", because the RustBench will use this for optimal C++ perf, but it's not used in this PR. I can remove it from this PR and reintroduce it in the next PR that includes Rust benches if you want?

I assume that merging Rustbenches will require more discussions than I have time/energy before the deadline, so this is just a fix-mvp to reduce the diff between what I use for benching and what enzyme has.

ZuseZ4 avatar Sep 18 '24 01:09 ZuseZ4

Ah it looks like it was just in a second makefile (I thought it was a sed script which modified this in place): https://github.com/EnzymeAD/Enzyme/blob/87ea5eb39315262930af8c5f01874921482484fb/enzyme/benchmarks/ReverseMode/ba/Makefile.makeafter#L13

Yeah this one should be modified too or ideally merged into the same makefile

wsmoses avatar Sep 18 '24 02:09 wsmoses

I rebased this branch and will let CI run.

jedbrown avatar Oct 30 '24 00:10 jedbrown

I see the Bazel job is failing on main too. It's when building LLVM so must be unrelated to this PR.

jedbrown avatar Oct 30 '24 02:10 jedbrown