summaryrefslogtreecommitdiffstats
path: root/src/gui/painting
diff options
context:
space:
mode:
authorMarc Mutz <marc.mutz@qt.io>2024-03-27 09:30:45 +0000
committerMarc Mutz <marc.mutz@qt.io>2024-04-04 12:24:19 +0100
commitf67f63c2f3dc56f72590d8eaadb8e824bf8133bc (patch)
treeb82bf37c8d4be7c3b2686db3abe2c2bc3ff250d7 /src/gui/painting
parent62e91d0025d4a039df0375190a6b93f98e2e7c35 (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.h10
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);