refactor(simd.h): use default for ctrs and assignment where applicable

[lgritz]

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.

[ThiagoIze]

With the exception of icc code, which will go away, this looks much nicer :)

There's some commented out code which I wasn't sure if you want to keep. I'm approving in case you're happy with that being present.

[lgritz]

We're still getting a suspicious CI failure, just on Windows. I don't want to merge until I understand why that's happening. Not at all sure if it's a latent bug that this change exposes (like how #4183 is an old bug I discovered in the process of this PR) or a compiler problem.

I will continue to investigate.

[lgritz]

This is very frustrating. I laboriously reproduced the whole thing on godbolt.org in the hopes of both reproducing there and seeing exactly which code is being incorrectly generated. But unfortunately on that site Windows will compile but not execute (some kind of security issue).

[ThiagoIze]

I wonder if maybe this is because windows is using SSE2 and the other CI machines are SSE4 or newer?

    HW features enabled at build: sse2

If you build locally on your non-windows machine with just SSE2, does it trigger the same fast_exp unit test fail?

[lgritz]

As far as I can tell, it's fine except on Windows. Thanks for the suggestion -- it's a little tricky, I think that the compiler is rigged to know that all Intel Macs supporting this OS are SSE4+, because asking for sse2 still seems to get me sse4.B ut I'm trying to rig it to be "sse2 only" on my Mac to verify what happens.

I'm also experimenting with an instrumented version of fast_exp2 with prints at every step, sending it to CI to see what happens.

[lgritz]

Confirmed: forcing sse2 only on Intel Mac still works fine. I think this is a MSVS thing in particular.

Curiously, that very same fast_exp2 function has the following comment in the middle of it:

// Original code was:
//     return bitcast_to_float (bitcast_to_int(r) + (m << 23));
// This was producing the wrong results when called via `sRGB_to_linear()`
// and `linear_to_sRGB()` on some Windows MSVC builds.
// We found that it was fixed by using a temporary intN, as below.
// Presumed to be an optimizer bug in MSVC versions 16.11.x
// and earlier.
// See PR #3804

#3804

So we have apparently identified trouble in this spot before as well. Maybe this is just more of the same, and the changes I made in this PR have shifted an existing problem around a little and made it symptomatic again.

[lgritz]

This thing is really squirrely. Putting print statements makes it disappear or move around to entirely different spots or get different results. Changing from SSE to AVX2 makes things happen in different places, different symptoms. The errors are all with the 2019 compiler. Can't reproduce with MSVS 2022 compiler. I'll tinker a bit longer but I'm real close to just disabling the SIMD for the one or two functions where it seems to be having trouble (only for older MSVS).

[lgritz]

Disabling simd fast_exp2 for older versions of MSVS seems to avoid the problem. I don't feel good about sinking any more time into figuring out why an obsolete compiler mis-handles this one function that isn't especially performance critical.

[ThiagoIze]

Looks good to me. I agree it's not worth the effort for this old compiler.

[ThiagoIze]

I liked that scalar and simd were required to have identical results, but this would at least solve #4064 without needing to actually make both code paths equal. I'll leave it up to you for which approach you want to take.

[lgritz]

At some point in my chasing this down, I made this change. Ultimately, disabling the SIMD path for the old MSVS versions in this function took care of the problem, so this change isn't strictly needed to close out this PR. So I'm not totally averse to backing that out, but I'm inclined to keep it since for whatever reason, fast_exp has seemed especially problematic for us historically.

I think my rationale here was that the family of fast_* functions in fmath.h is already advertised as being approximate, deviating slightly from IEEE full precision and not necessarily being bit identical on all platforms (CPU + compiler + build flags). Even on the same platform, you could get ever so slight differences in scalar vs simd results depending on if one used fused mad (fp contraction) but the other didn't, or other minor compiler rearrangements of the underlying instructions.