Bug 147101 – Unexpected ABI diffference between armv7 and armv7s in WebCore::GraphicsLayerCA::~GraphicsLayerCA()

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[David Kilzer (:ddkilzer)]

Unexpected ABI diffference between armv7 and armv7s in WebCore::GraphicsLayerCA::~GraphicsLayerCA() results in two different symbols:

__ZThn568_N7WebCore15GraphicsLayerCAD0Ev
__ZThn576_N7WebCore15GraphicsLayerCAD0Ev

This is due to the following code in Source/WebCore/platform/graphics/transforms/TransformationMatrix.h:

#if CPU(APPLE_ARMV7S) || defined(TRANSFORMATION_MATRIX_USE_X86_64_SSE2)
#if COMPILER(MSVC)
    __declspec(align(16)) typedef double Matrix4[4][4];
#else
    typedef double Matrix4[4][4] __attribute__((aligned (16)));
#endif
#else
    typedef double Matrix4[4][4];
#endif

We should simply use the same alignment on armv7 to fix this.

[David Kilzer (:ddkilzer)]

<rdar://problem/21867029>

[David Kilzer (:ddkilzer)]

Created attachment 257078 [details]
Patch v1

[Alex Christensen]

Comment on attachment 257078 [details]
Patch v1

TransformationMatrix.cpp has a corresponding #elif CPU(APPLE_ARMV7S) which should be changed with this.

[David Kilzer (:ddkilzer)]

(In reply to comment #3)
> Comment on attachment 257078 [details]
> Patch v1
> 
> TransformationMatrix.cpp has a corresponding #elif CPU(APPLE_ARMV7S) which
> should be changed with this.

The assembly in TransformationMatrix.cpp in the #elif CPU(APPLE_ARMV7S) may use instructions that aren't available on non-armv7s platforms.  In that case, we fall back to the C code in the #else clause.

Ben Poulain needs to review this change.  Moving back to r?.

[Michael Saboff]

Comment on attachment 257078 [details]
Patch v1

r=me

[Benjamin Poulain]

+1 from me too.

The ARMV7S in TransformationMatrix.cpp has to stay.

[Alex Christensen]

Comment on attachment 257078 [details]
Patch v1

ok, makes sense.  I just thought it may have been overlooked.

[WebKit Commit Bot]

Comment on attachment 257078 [details]
Patch v1

Clearing flags on attachment: 257078

Committed r187034: <http://trac.webkit.org/changeset/187034>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[David Kilzer (:ddkilzer)]

(In reply to comment #7)
> Comment on attachment 257078 [details]
> Patch v1
> 
> ok, makes sense.  I just thought it may have been overlooked.

It was a good question!  I had overlooked it until you mentioned it, but it turns out we shouldn't change it as noted above.