diff options
author | Marc Mutz <marc.mutz@qt.io> | 2024-03-27 09:30:45 +0000 |
---|---|---|
committer | Marc Mutz <marc.mutz@qt.io> | 2024-04-04 12:24:19 +0100 |
commit | f67f63c2f3dc56f72590d8eaadb8e824bf8133bc (patch) | |
tree | b82bf37c8d4be7c3b2686db3abe2c2bc3ff250d7 /src/gui/painting | |
parent | 62e91d0025d4a039df0375190a6b93f98e2e7c35 (diff) |
Use properly-aligned float[4] as _mm_store_ps() output buffer
... not a QColorVector.
While cccda0e62d3d70f09654bbd6681a3e79c9814c8d, successfully worked
around the problem that QColorVector does not fulfill the
_mm_store_ps() function's alignment requirement (by using the
unaligned version, _mm_storeu_ps()), it failed to address the problem
that the code writes into a QColorVector object's member variable
(float, a 32-bit storage location) when the function is asking for a
128-bit storage:
/// Stores a 128-bit vector of [4 x float] into an aligned memory
/// location.
///
/// \headerfile <x86intrin.h>
///
/// This intrinsic corresponds to the <c> VMOVAPS / MOVAPS </c> instruction.
///
/// \param __p
/// A pointer to a 128-bit memory location. The address of the memory
/// location has to be 16-byte aligned.
/// \param __a
/// A 128-bit vector of [4 x float] containing the values to be stored.
The treatment of a struct { float x, y, z, w; } as a float[4] is not
allowed by C++, and even if it was, we'd have to pass the address of
the struct object, not that of its first member variable.
It seems like Coverity has recently learned about this kind of thing,
at least there are tons of such new issues listed in the March scan
and while I can't find a report about this particular instance in the
current build, that probably just means that the Coverity build simply
doesn't see this code path due to the #ifdef'ery.
To fix, replace the QColorVector with an over-aligned float[4].
Because we're over-aligning the float[4], we can go back to the
original _mm_store_ps(), which is, presumably, faster then the -u
variant. We don't lose any expressiveness here, either, because the old
code never used any of the member functions of the QColorVector object
used as a store target.
Amends f944651e3db01a73b10212926a7b1c7aad5eb83e.
Amends cccda0e62d3d70f09654bbd6681a3e79c9814c8d.
Change-Id: Ia0f8202bf4266b8b19b3de897a897de58b7a6d94
Reviewed-by: Giuseppe D'Angelo <giuseppe.dangelo@kdab.com>
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Diffstat (limited to 'src/gui/painting')
-rw-r--r-- | src/gui/painting/qcolormatrix_p.h | 10 |
1 files changed, 5 insertions, 5 deletions
diff --git a/src/gui/painting/qcolormatrix_p.h b/src/gui/painting/qcolormatrix_p.h index de6a1dddef..e586e0dadc 100644 --- a/src/gui/painting/qcolormatrix_p.h +++ b/src/gui/painting/qcolormatrix_p.h @@ -103,11 +103,11 @@ public: #else v = _mm_or_ps(_mm_and_ps(cmpgt, est), _mm_andnot_ps(cmpgt, kapmul)); #endif - QColorVector out; - _mm_storeu_ps(&out.x, v); - const float L = 116.f * out.y - 16.f; - const float a = 500.f * (out.x - out.y); - const float b = 200.f * (out.y - out.z); + alignas(16) float out[4]; + _mm_store_ps(out, v); + const float L = 116.f * out[1] - 16.f; + const float a = 500.f * (out[0] - out[1]); + const float b = 200.f * (out[1] - out[2]); #else float xr = x * (1.f / ref.x); float yr = y * (1.f / ref.y); |