[Webkit-unassigned] [Bug 73182] Need SSE optimization for functions vfmul and vadd

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=73182





--- Comment #14 from xingnan.wang at intel.com  2011-11-29 22:22:49 PST ---
(In reply to comment #10)
> (From update of attachment 116909 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=116909&action=review
> 
> > Source/WebCore/platform/audio/VectorMath.cpp:76
> > +        // If the sourceP address does not 16-byte align, the first several frames(at most three) should be processed seperately.
> 
> Missing space before (.
> 
> > Source/WebCore/platform/audio/VectorMath.cpp:77
> > +        while ((((size_t)sourceP & 0x0F) != 0) && n) {
> 
> Please use static_cast instead of C-style cast.
> 
> > Source/WebCore/platform/audio/VectorMath.cpp:87
> > +        __m128 *pSource, *pDest;
> 
> We prefer one declaration per line and the * goes next to the type.
> 
> > Source/WebCore/platform/audio/VectorMath.cpp:91
> > +            pSource = reinterpret_cast<__m128*>((float*)sourceP);
> 
> Please use static_cast instead of C-style cast.
> 
> > Source/WebCore/platform/audio/VectorMath.cpp:93
> > +            // There is less than 1% performance enhancement when pulling the if statement out of the loop, so leave it here.
> 
> It is not clear why you wouldn't want the 1% performance gain here.
> 
> > Source/WebCore/platform/audio/VectorMath.cpp:94
> > +            if (((size_t)destP & 0x0F) != 0) {
> 
> Please use static_cast instead of C-style cast.
> 
> > Source/WebCore/platform/audio/VectorMath.cpp:136
> > +        while ((((size_t)source1P & 0x0F) != 0) && n) {
> 
> Please use static_cast instead of C-style cast.
> 
> > Source/WebCore/platform/audio/VectorMath.cpp:147
> > +        __m128 * pSource1, *pSource2, *pDest;
> > +        __m128 source2, dest;
> 
> We prefer one declaration per line and the * goes next to the type.
> 
> > Source/WebCore/platform/audio/VectorMath.cpp:150
> > +        bool source2Aligned = !((size_t)source2P & 0x0F);
> > +        bool destAligned = !((size_t)destP & 0x0F);
> 
> Please use static_cast instead of C-style cast.
> 
> > Source/WebCore/platform/audio/VectorMath.cpp:155
> > +                pSource1 = reinterpret_cast<__m128*>((float*)source1P);
> > +                pSource2 = reinterpret_cast<__m128*>((float*)source2P);
> 
> Please use static_cast instead of C-style cast.
> 
> > Source/WebCore/platform/audio/VectorMath.cpp:167
> > +                pSource1 = reinterpret_cast<__m128*>((float*)source1P);
> > +                pSource2 = reinterpret_cast<__m128*>((float*)source2P);
> 
> Please use static_cast instead of C-style cast.
> 
> > Source/WebCore/platform/audio/VectorMath.cpp:178
> > +                pSource1 = reinterpret_cast<__m128*>((float*)source1P);
> 
> Please use static_cast instead of C-style cast.
> 
> > Source/WebCore/platform/audio/VectorMath.cpp:188
> > +        } else if (!source2Aligned && !destAligned) // both source2 and dest not aligned 
> > +        {
> 
> { should go on the previous line.
> 
> > Source/WebCore/platform/audio/VectorMath.cpp:191
> > +                source2 = _mm_loadu_ps(source2P);
> 
> Please use static_cast instead of C-style cast.

Hi Sam, 
    Thanks to your review and we update the patch by your comments.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.