Some patches for SDL_memset4 and SDL_memcpy4

Vittorio_Giovara · July 20, 2013, 10:42am

Hi all,

long time ago some code was posted here to improve the default
implementation fo SDL_memset4 for arm (
http://sdl.5483.n7.nabble.com/ARM-optimised-memset-td29728.html).
I originally ported that code for the ancient GAS present in Xcode and then
lost track of it when converting it to inline assembly.

When I found that code a few days ago and tried inlining it again but
failed, So I looked if there were alternatives and found this nice code
http://lists.uclibc.org/pipermail/uclibc/2003-September/027817.html that
works well with SDL. I have a patch ready (attached) but I think I have to
contact the original author and ask if he’s ok with zlib before inclusion.
Just as backup, I’ve included the original patch that still needs to be
inlined… Could anyone take a look at it maybe and see what’s wrong when
inlining?

Then I noticed that also memcpy4 could be optimized, and added it even
thought SDL doesn’t use it internally – maybe I placed the assembly in the
wrong place, if so tell me and I’ll move it.

What also puzzled me a bit is that for SDL_memcpy there is a check for
using __builtin_memcpy… so what is really preventing us for applying the
same check for the other functions? I’ve added a third patch that adds this
check for other functions too…

Can anyone test and review all these patches?
Also, besides alignment, what is preventing using the __builtin equivalent
functions for memset?

Cheers,
Vittorio
-------------- next part --------------
A non-text attachment was scrubbed…
Name: 1-check_for_some___builtin___functions.patch
Type: application/octet-stream
Size: 1217 bytes
Desc: not available
URL: http://lists.libsdl.org/pipermail/sdl-libsdl.org/attachments/20130720/a1612e28/attachment.obj
-------------- next part --------------
A non-text attachment was scrubbed…
Name: 2-ARM_optimized_SDL_memset4.patch
Type: application/octet-stream
Size: 3000 bytes
Desc: not available
URL: http://lists.libsdl.org/pipermail/sdl-libsdl.org/attachments/20130720/a1612e28/attachment-0001.obj
-------------- next part --------------
A non-text attachment was scrubbed…
Name: 3-ARM_optimized_SDL_memcpy4.patch
Type: application/octet-stream
Size: 2548 bytes
Desc: not available
URL: http://lists.libsdl.org/pipermail/sdl-libsdl.org/attachments/20130720/a1612e28/attachment-0002.obj
-------------- next part --------------
A non-text attachment was scrubbed…
Name: sdl-framebuffer.patch
Type: application/octet-stream
Size: 4077 bytes
Desc: not available
URL: http://lists.libsdl.org/pipermail/sdl-libsdl.org/attachments/20130720/a1612e28/attachment-0003.obj

Vittorio_Giovara · July 25, 2013, 8:59pm

Hi all,

Can anyone test and review all these patches?
Also, besides alignment, what is preventing using the __builtin equivalent
functions for memset?

Cheers,
Vittorio

I’ve attached the patches here for better tracking

github.com/libsdl-org/SDL

arm optimized memset4 and memcpy4

opened 11:00PM - 10 Feb 21 UTC

closed 03:55PM - 18 Feb 21 UTC

SDLBugzilla

enhancement waiting

# This bug report was migrated from our old Bugzilla tracker. These attachments… are available in the static archive: * [checking GNUCC for __builtin function (1-check_for_some___builtin___functions.patch, text/plain, 2013-07-25 16:55:20 +0000, 1218 bytes)](https://bugzilla.libsdl.org/attachment.cgi?id=1255) * [arm optimized SDL_memset4 (2-ARM_optimized_SDL_memset4.patch, text/plain, 2013-07-25 16:55:52 +0000, 3001 bytes)](https://bugzilla.libsdl.org/attachment.cgi?id=1256) * [arm optimized SDL_memcpy4 (3-ARM_optimized_SDL_memcpy4.patch, text/plain, 2013-07-25 16:56:16 +0000, 2549 bytes)](https://bugzilla.libsdl.org/attachment.cgi?id=1257) * [original non-inlined memset4 version (sdl-framebuffer.patch, text/plain, 2013-07-25 16:57:19 +0000, 4078 bytes)](https://bugzilla.libsdl.org/attachment.cgi?id=1258) **Reported in version:** HG 2.0 **Reported for operating system, platform:** Other, ARM # Comments on the original bug report: On 2013-07-25 16:53:43 +0000, Vittorio Giovara wrote: > Long time ago some code was posted here to improve the default implementation fo SDL_memset4 for arm (http://sdl.5483.n7.nabble.com/ARM-optimised-memset-td29728.html). > I originally ported that code for the ancient GAS present in Xcode and then lost track of it when converting it to inline assembly. > > When I found that code a few days ago and tried inlining it again but failed, So I looked if there were alternatives and found this nice code http://lists.uclibc.org/pipermail/uclibc/2003-September/027817.html that works well with SDL. I have a patch ready (attached) - there is no explicit license attached so I think the original author should be contacted about this use (and maybe improve it?). > > Just as backup, I've included the original patch that still needs to be inlined if anyone would prefer that... > > Then I noticed that also memcpy4 could be optimized, and added it even thought SDL doesn't use it internally -- maybe I placed the assembly in the wrong place, if so tell me and I'll move it. > > > What also puzzled me a bit is that for SDL_memcpy there is a check for using __builtin_memcpy... so what is really preventing us for applying the same check for the other functions? I've added a third patch that adds this check for other functions too... > > Besides alignment, what is preventing using the __builtin equivalent functions for memset? On 2013-07-25 16:55:20 +0000, Vittorio Giovara wrote: > Created attachment 1255 > checking GNUCC for __builtin function On 2013-07-25 16:55:52 +0000, Vittorio Giovara wrote: > Created attachment 1256 > arm optimized SDL_memset4 On 2013-07-25 16:56:16 +0000, Vittorio Giovara wrote: > Created attachment 1257 > arm optimized SDL_memcpy4 On 2013-07-25 16:57:19 +0000, Vittorio Giovara wrote: > Created attachment 1258 > original non-inlined memset4 version On 2013-07-27 04:16:01 +0000, Sam Lantinga wrote: > Thanks for the patches. A few comments: > > * Do you know what version of gcc provides each of those builtins? I believe gcc 2.95 doesn't, for example, which is I think still used for the Haiku build. > * I didn't check your assembly, but your SDL_memset4() is a 32-bit set, not an 8-bit set, correct? > * SDL_memcpy4() has been removed, since it isn't faster than modern memcpy functions.

I’d be nice if anyone could shed some light on those __builtin functions
too

Cheers,
VittorioOn Sat, Jul 20, 2013 at 12:42 PM, Vittorio Giovara < @Vittorio_Giovara> wrote:

AndreRenaud · July 25, 2013, 10:21pm

Hi Vittorio,

Can anyone test and review all these patches?
Also, besides alignment, what is preventing using the __builtin equivalent
functions for memset?

I sent through those original patches. I don’t really have a comment
on the ones you’ve supplied, however you might be interested in a
similar bit of work that’s being done in the musl C library. We’re
trying to get ARM optimised versions of mem* functions in there
(currently just memcpy, but hopefully memset will be next). The
discussion/code for this can be found at
musl - Re: Thinking about release.

Regards,
Andre

Vittorio_Giovara · July 29, 2013, 2:25pm

Hi Andre,On Fri, Jul 26, 2013 at 12:21 AM, Andre Renaud wrote:

I sent through those original patches. I don’t really have a comment
on the ones you’ve supplied, however you might be interested in a
similar bit of work that’s being done in the musl C library. We’re
trying to get ARM optimised versions of mem* functions in there
(currently just memcpy, but hopefully memset will be next). The
discussion/code for this can be found at
musl - Re: Thinking about release.

I remember your submission, and you current code looks very nice. Your
experience with asm beats mine by all means, so would you consider
submitting a patch to bring your optmized memset to inline assembly for
SDL2? I don’t mind dropping mine.

If I remember correctly you mentioned a 5 time speed increase, so I’m sure
many iOS/Android developers would look forward to your contribution.

Cheers,
Vittorio