Bug 56247 – Alignement problem (list of CPU types requiring alignment is too short)

• Status: UNCONFIRMED
• Component: WebCore Misc.

[Martin Husemann]

Created attachment 85584 [details]
Possible extension of the cpu type list

WebCore/platform/text/StringHash.h and WebCore/platform/text/AtomicString.cpp have a hard coded list of cpu types requiring alignment. This list is too short.

Actually it would probably be preferable to create the inverse list of cpu types where unaligned 32bit access is both allowed AND faster than single byte access - I would guess this to be pretty short.

[Martin Husemann]

Created attachment 85585 [details]
same change for a copy&pasted location

[Alexey Proskuryakov]

Thank you for tackling this.

Please follow the steps in <http://www.webkit.org/coding/contributing.html> precisely to have this landed. Specifically,

- The patch must be marked for review. That makes EWS bots build the patch on several platforms to ensure that it doesn't break the build.
- Please add a properly formatted ChangeLog entry. If you don't write it, who will? Note that prepare-ChangeLog script helps with preparing those.
- Please use svn-create-patch script to make a diff against trunk. A patch that doesn't apply to tip of tree cleanly cannot be landed via commit queue, which automatically verifies that a patch doesn't break the build or regression tests, and generally saves a lot of effort.
- Please land one patch per bugzilla bug. That makes it easier to land via commit queue.

I think that both changes you propose can be made in one patch, there is no need to split them. Your patch is made against some unspecified branch - any fixes need to go to trunk first, and be backported to branches that need it later.

Where does the list of processors that require alignment come from? I'm particularly unsure if we want to include PowerPC.

[Martin Husemann]

Thanks for the hints - I'll follow the procedure.

The list of CPUs I made up out of thin air - I always associate "alignement required" with RISC in general, but you are right, ppc is the oddball risc platform and does allow it. So remove ppc and ppc64, add alpha and hppa.

I wonder if it is worth to split the loop into three parts, head and tail doing slow unaligned compares, but the main part doing 32bit compares.

This could be used on all cpus, and should be faster if the main loop dominates the length, even on x86/amd64 cpus.

[Alexey Proskuryakov]

I'm curious why we are not just using memcmp. Maybe svn history has the answer.

[Alexey Proskuryakov]

Note that alignment problems on SPARC specifically were discussed in bug 19775. Several factors contributed to difficult history of that bug, but there might be useful information in comments. I'm not suggesting closing this one as a duplicate.

[Martin Husemann]

I'll provide a patch in the requested format against trunk; I don't care for which of the two bugs, nor closing this one as a duplicate.

For the record: I've been told that only some powerpc cpus do implement the unaligned access.