Cycles: Cleanup, simplify and expand math/vectorization build flags #128342

Lukas Stockner · 2024-09-30T00:20:28+02:00

Lukas Stockner commented

2024-09-30 00:20:28 +02:00

The existing code to set vectorization and optimization build flags was a bit of a mess.
This PR cleans it up and makes some improvements:

There were bits of AVX code left, even though we only compile SSE4.2 and AVX2 kernels nowadays
Vectorization and optimization flags were mixed together for no reason
- This caused some downsides - e.g., enabling NATIVE_ONLY would skip all the optimization flags
The names were confusing/misleading - for example, CYCLES_KERNEL_FLAGS was also used for host code
- Optimizations are now directly appended to CMAKE_CXX_FLAGS instead of building CYCLES_KERNEL_FLAGS and then appending that at the end
- Vectorization is renamed from CYCLES_x_KERNEL_FLAGS to CYCLES_x_FLAGS
The math optimization flags were duplicated between Windows+Clang and GCC or Linux+Clang
- Since the only difference is a prefix, CMake lists can clean this up nicely
SSE4.2 was only enabled for host code on GCC/Clang, but we require it on all x86 platforms, so now it's assumed on all compilers that support it
There are some additional math optimizations that I think we can enable safely
- Specifically:
  - -ffp-contract=fast (enables FMA generation)
  - -freciprocal-math (enables x/y -> x*(1/y))
  - -fassociative-math (enables e.g. a*b + c*b -> (a+c)*b)
- These are used on Windows and HIP anyways, so our code can't expect exact IEEE semantics in any case
- The only difference between the new set and -ffast-math is that we don't use -ffinite-math-only since this causes issues with the BVH (see ce1f2e271d) and breaks e.g. isnan
- This causes a ~1.5% speedup in my very quick test, but might be higher for some more math-intensive cases

The existing code to set vectorization and optimization build flags was a bit of a mess. This PR cleans it up and makes some improvements: - There were bits of AVX code left, even though we only compile SSE4.2 and AVX2 kernels nowadays - Vectorization and optimization flags were mixed together for no reason - This caused some downsides - e.g., enabling `NATIVE_ONLY` would skip all the optimization flags - The names were confusing/misleading - for example, `CYCLES_KERNEL_FLAGS` was also used for host code - Optimizations are now directly appended to `CMAKE_CXX_FLAGS` instead of building `CYCLES_KERNEL_FLAGS` and then appending that at the end - Vectorization is renamed from `CYCLES_x_KERNEL_FLAGS` to `CYCLES_x_FLAGS` - The math optimization flags were duplicated between Windows+Clang and GCC or Linux+Clang - Since the only difference is a prefix, CMake lists can clean this up nicely - SSE4.2 was only enabled for host code on GCC/Clang, but we require it on all x86 platforms, so now it's assumed on all compilers that support it - There are some additional math optimizations that I think we can enable safely - Specifically: - `-ffp-contract=fast` (enables FMA generation) - `-freciprocal-math` (enables `x/y -> x*(1/y)`) - `-fassociative-math` (enables e.g. `a*b + c*b -> (a+c)*b`) - These are used on Windows and HIP anyways, so our code can't expect exact IEEE semantics in any case - The only difference between the new set and `-ffast-math` is that we don't use `-ffinite-math-only` since this causes issues with the BVH (see ce1f2e271d84f0bb7798c04cb9ca8459f12cee50) and breaks e.g. `isnan` - This causes a ~1.5% speedup in my very quick test, but might be higher for some more math-intensive cases

👀 1

Lukas Stockner added 7 commits 2024-09-30 00:20:38 +02:00

Remove obsolete AVX CMake logic 5779139070

Fix flag name f6bc696d38

Split vectorization and optimization flags 197a1e973e

Rename CYCLES_x_KERNEL_FLAGS to CYCLES_x_FLAGS 008477d0bb

Apply SSE4.2 minimum regardless of compiler 2aaa820475

Clean up and deduplicate math optimization flags 4e7687fa21

Add some more math optimizations

buildbot/vexp-code-patch-lint Build done.

Details

buildbot/vexp-code-patch-linux-x86_64 Build done.

Details

buildbot/vexp-code-patch-darwin-x86_64 Build done.

Details

buildbot/vexp-code-patch-windows-amd64 Build done.

Details

buildbot/vexp-code-patch-darwin-arm64 Build done.

Details

buildbot/vexp-code-patch-coordinator Build done.

Details

f1f32649b2

Lukas Stockner added this to the Render & Cycles project 2024-09-30 00:20:42 +02:00

Lukas Stockner commented

2024-09-30 00:20:49 +02:00

@blender-bot build

Lukas Stockner requested review from Sergey Sharybin 2024-09-30 00:21:01 +02:00

Lukas Stockner requested review from Weizhen Huang 2024-09-30 00:21:01 +02:00

Sergey Sharybin commented

2024-09-30 12:13:42 +02:00

Overall it seems nice, when you look at the individual commits.

I think we need to land this PR as a non-squished change, for the ease of reading the changes in Git log. But for this it would be nice to have a more proper commit messages (doesn't need to be anything super fancy, but follow the commit title guideline, and briefly explain what the commit is about -- can copy the corresponding lines from the PR description).

Overall it seems nice, when you look at the individual commits. I think we need to land this PR as a non-squished change, for the ease of reading the changes in Git log. But for this it would be nice to have a more proper commit messages (doesn't need to be anything super fancy, but follow the commit title guideline, and briefly explain what the commit is about -- can copy the corresponding lines from the PR description).

Lukas Stockner referenced this issue from a commit

2024-09-30 15:59:23 +02:00

Cleanup: Cycles: Remove obsolete AVX CMake logic

Lukas Stockner referenced this issue from a commit

2024-09-30 15:59:23 +02:00

Cleanup: Cycles: Fix flag name

Lukas Stockner referenced this issue from a commit

2024-09-30 15:59:23 +02:00

Cleanup: Cycles: Split vectorization and optimization CMake build flags

Lukas Stockner referenced this issue from a commit

2024-09-30 15:59:23 +02:00

Cleanup: Cycles: Rename CYCLES_x_KERNEL_FLAGS to CYCLES_x_FLAGS in CMake