Use properly-aligned float[4] as _mm_store_ps() output buffer
... not a QColorVector. Whilebb10cccda0e62d, 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. Amendsf944651e3d. Amendscccda0e62d. 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>
parent
62e91d0025
commit
f67f63c2f3
|
|
@ -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);
|
||||
|
|
|
|||
Loading…
Reference in New Issue