Skip to content

Commit

Permalink
refactor(simd.h): use default for ctrs and assignment where applicable (
Browse files Browse the repository at this point in the history
#4187)

simd.h improvements and refactorization for consistency:

* Use `= default` for all the uninitialized constructors, copy
constructors, and assignment from the same type.
* Fix the `operator=` to be more standard by returning a `vec&` instead
of `const vec&`, which apparently is unusual.
* Test that these all result in C++ understanding that these classes are
trivially copyable.

There is a bit of awkwardness where for the vint types, we have to
define the copy constructors for the Intel icc compiler. It just seems
to do the wrong thing for `=default` and fails lots of our tests. I
don't understand why, and I'm not inclined to investigate too deeply,
since it's a discontinued compiler we support only for legacy reasons,
so it doesn't bother me too much that we special-case it. One reason I
think it's a compiler bug is that it has no problem with the floating
point vector types. It's confused about something with the simd ints.

Disable SIMD acceleration of fast_exp2 when using MSVS prior to 2022.
It seems to sporadically generate bad code in this function and I
can't seem to restructure the function in a way that consistently is
correct. Doesn't seem to happen with newer MSVS, nor on any other
platform or compiler. I'm assuming this is a compiler bug and it's not
worth pursuing for an old version of one compiler, for this one
function that isn't really used all that often or in
performance-critical paths.

Some extra slosh in the simd fast_exp test.

Rewrite OIIO_CHECK_SIMD_EQUAL_THRESH to print the differences and use
more digits of precision when it fails.

---------

Signed-off-by: Larry Gritz <[email protected]>
  • Loading branch information
lgritz authored Mar 16, 2024
1 parent e41ac03 commit 68666db
Show file tree
Hide file tree
Showing 6 changed files with 132 additions and 126 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -547,15 +547,15 @@ jobs:
generator: "Visual Studio 16 2019"
openexr_ver: v3.2.2
python_ver: 3.7
simd: sse4.2
# simd: sse4.2
# - desc: windows-2022
# runner: windows-2022
# vsver: 2022
# generator: "Visual Studio 17 2022"
# openexr_ver: main
# openexr_ver: v3.2.2
# # v3.1.4
# python_ver: 3.7
# simd: sse4.2
# # simd: AVX2
runs-on: ${{ matrix.runner }}
env:
PYTHON_VERSION: ${{matrix.python_ver}}
Expand Down
2 changes: 1 addition & 1 deletion src/cmake/compiler.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,8 @@ endif ()
#
set (USE_SIMD "" CACHE STRING "Use SIMD directives (0, sse2, sse3, ssse3, sse4.1, sse4.2, avx, avx2, avx512f, f16c, aes)")
set (SIMD_COMPILE_FLAGS "")
message (STATUS "Compiling with SIMD level ${USE_SIMD}")
if (NOT USE_SIMD STREQUAL "")
message (STATUS "Compiling with SIMD level ${USE_SIMD}")
if (USE_SIMD STREQUAL "0")
set (SIMD_COMPILE_FLAGS ${SIMD_COMPILE_FLAGS} "-DOIIO_NO_SIMD=1")
else ()
Expand Down
2 changes: 1 addition & 1 deletion src/include/OpenImageIO/fmath.h
Original file line number Diff line number Diff line change
Expand Up @@ -1852,7 +1852,7 @@ template<typename T>
OIIO_FORCEINLINE OIIO_HOSTDEVICE T fast_exp2 (const T& xval) {
using namespace simd;
typedef typename T::vint_t intN;
#if OIIO_SIMD_SSE
#if OIIO_SIMD_SSE && !OIIO_MSVS_BEFORE_2022
// See float specialization for explanations
T x = clamp (xval, T(-126.0f), T(126.0f));
intN m (x); x -= T(m);
Expand Down
Loading

0 comments on commit 68666db

Please sign in to comment.