Gcc-3.4.0 / PIC fix

Mike_Frysinger · May 3, 2004, 11:25am

when building with -fPIC and gcc-3.4.0, a few files fail to compile

src/audio/SDL_mixer_MMX.c
this one is easy to fix … find patch attached

src/video/SDL_yuv_mmx.c
this one claims ebx gets clobbered yet the asm saves/restores it ? removing
it from the clobber list ‘fixes’ the problem since in reality it isn’t
clobbered …

src/cpuinfo/SDL_cpuinfo.c
perhaps remove ebx from the clobber list and push/pop it in the same manner
as SDL_yuv_mmx.c ?
-mike
-------------- next part --------------
A non-text attachment was scrubbed…
Name: gcc-3.4-pic.patch
Type: text/x-diff
Size: 1682 bytes
Desc: not available
URL: http://lists.libsdl.org/pipermail/sdl-libsdl.org/attachments/20040503/2fcc7af0/attachment.patch

Stephane_Marchesin · May 3, 2004, 12:14pm

Mike Frysinger wrote:

when building with -fPIC and gcc-3.4.0, a few files fail to compile

Hmmm, right, SDL sets it. I’ll upgrade to gcc 3.4 and fix it.

Stephane

Stephane_Marchesin · May 3, 2004, 12:17pm

Mike Frysinger wrote:

when building with -fPIC and gcc-3.4.0, a few files fail to compile

Why do you need the -fPIC option on x86 architectures ?

Stephane

Stephane_Marchesin · May 3, 2004, 1:11pm

Mike Frysinger wrote:

when building with -fPIC and gcc-3.4.0, a few files fail to compile

src/audio/SDL_mixer_MMX.c
this one is easy to fix … find patch attached

Your patch is fine, but I added some x86_64 support that I had lying in
my SDL tree (and it makes the code shorter and cleaner anyway).

src/video/SDL_yuv_mmx.c
this one claims ebx gets clobbered yet the asm saves/restores it ? removing
it from the clobber list ‘fixes’ the problem since in reality it isn’t
clobbered …

I don’t like push/pops too much, but I see no other way. Using push/pop
means you have to be sure you can touch the stack which is not always
the case (for example with inlined functions).

src/cpuinfo/SDL_cpuinfo.c
perhaps remove ebx from the clobber list and push/pop it in the same manner
as SDL_yuv_mmx.c ?

We can simply remove ebx from the clobber list, since it’s already saved
and restored.

A modified patch is attached, tested with gcc 3.3.1 and 2.96. Could you
report if this works with 3.4.0 please (it seems it’s not yet packaged
by my distro) ?

Stephane

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed…
Name: gcc-3.4-pic2.patch
URL: http://lists.libsdl.org/pipermail/sdl-libsdl.org/attachments/20040503/8ccdc1b3/attachment.asc

Mike_Frysinger · May 3, 2004, 1:42pm

mmm are we confusing SDL_cpuinfo.c and SDL_yuv_mmx.c ?
the yuv_mmx code pushes/pops ebx so that bit can just trim the edx register
from the clobber list, but the cpuinfo code doesnt … it just calls cpuid
which clobbers e[abcd]x … and like you say, there doesnt seem to be any way
around this other than using push/pops

after rebuilding 1.2.7, SDL apps get angry with the MMX??? stuff:
/usr/lib/libSDL.so: undefined reference to _MMX_UbluRGB' /usr/lib/libSDL.so: undefined reference to_MMX_Vred5x5’
/usr/lib/libSDL.so: undefined reference to _MMX_VgrnRGB' /usr/lib/libSDL.so: undefined reference to_MMX_grn565’
the patch below fixes up the asm code in SDL_yuv_mmx.c … basically changes
_MMX_VredRGB to %[_MMX_VredRGB] and adds [_MMX_VredRGB] “m” (*_MMX_VredRGB)
definitions at the end … havent had a chance to test it with gcc-3.3 though

also, i forgot to mention in my original e-mail the original bug report/patch:
bug: http://bugs.gentoo.org/show_bug.cgi?id=48947
patch: http://bugs.gentoo.org/attachment.cgi?id=30017
-mikeOn Monday 03 May 2004 04:08 pm, Stephane Marchesin wrote:

src/video/SDL_yuv_mmx.c
this one claims ebx gets clobbered yet the asm saves/restores it ?
removing it from the clobber list ‘fixes’ the problem since in reality it
isn’t clobbered …

I don’t like push/pops too much, but I see no other way. Using push/pop
means you have to be sure you can touch the stack which is not always
the case (for example with inlined functions).

src/cpuinfo/SDL_cpuinfo.c
perhaps remove ebx from the clobber list and push/pop it in the same
manner as SDL_yuv_mmx.c ?

We can simply remove ebx from the clobber list, since it’s already saved
and restored.

Stephane_Marchesin · May 3, 2004, 2:09pm

Mike Frysinger wrote:

src/video/SDL_yuv_mmx.c
this one claims ebx gets clobbered yet the asm saves/restores it ?
removing it from the clobber list ‘fixes’ the problem since in reality it
isn’t clobbered …

I don’t like push/pops too much, but I see no other way. Using push/pop
means you have to be sure you can touch the stack which is not always
the case (for example with inlined functions).

src/cpuinfo/SDL_cpuinfo.c
perhaps remove ebx from the clobber list and push/pop it in the same
manner as SDL_yuv_mmx.c ?

We can simply remove ebx from the clobber list, since it’s already saved
and restored.

mmm are we confusing SDL_cpuinfo.c and SDL_yuv_mmx.c ?

Hmmm… well let’s be clear :

no push/pops in the cpuinfo part (ebx is saved in edi)
the yuv code is touchy wrt the register allocator (that varies greatly
among gcc versions), which tends to use all registers and thus we can’t
spare ebx, so we can’t avoid the push/pop

the yuv_mmx code pushes/pops ebx so that bit can just trim the edx register
from the clobber list, but the cpuinfo code doesnt … it just calls cpuid
which clobbers e[abcd]x … and like you say, there doesnt seem to be any way
around this other than using push/pops

In cpuinfo, ebx is saved in edi, so everything is fine.

after rebuilding 1.2.7, SDL apps get angry with the MMX??? stuff:
/usr/lib/libSDL.so: undefined reference to _MMX_UbluRGB' /usr/lib/libSDL.so: undefined reference to_MMX_Vred5x5’
/usr/lib/libSDL.so: undefined reference to _MMX_VgrnRGB' /usr/lib/libSDL.so: undefined reference to_MMX_grn565’
the patch below fixes up the asm code in SDL_yuv_mmx.c … basically changes
_MMX_VredRGB to %[_MMX_VredRGB] and adds [_MMX_VredRGB] “m” (*_MMX_VredRGB)
definitions at the end … havent had a chance to test it with gcc-3.3 though

Not sure that’ll work on older gcc versions.
I’ll look at it and send another patch (but probably not today, as I
need to test this on different gcc versions). Hopefully this will be the
last

Stephane>On Monday 03 May 2004 04:08 pm, Stephane Marchesin wrote:

Mike_Gorchak · May 5, 2004, 4:53am

Hello, Mike!

MF> when building with -fPIC and gcc-3.4.0, a few files fail to compile

Hm, and what about hermes sublibrary in SDL ? It’s also damnely broken as
for PIC/shared. I’ve wrote about this a quite time ago, but got noone
respond.

Here my mail:------------------------------------------------------------------

==========================================================================

Newsgroup: gmane.comp.lib.sdl
From: “Mike Gorchak” <@Mike_Gorchak>
Date: Mon, 5 Apr 2004 08:39:29 +0300
To: All
Subj: Oops ! Really big problem with hermes sublibrary of SDL under QNX6
==========================================================================

Hello, All!

Got a big problem yesterday, trying to enable MMX accelerated code under
QNX6. Hermes library isn’t a valid PIC code, so it’s usage in shared library
objects can cause continuous segmentation faults in different places
(including at initialization stage). Of course it depends on OS
architecture.

For example file mmxp2_32.asm: it contains the following line:

movq mm6, qword [mmx32_rgb888_mask]

which is wrong. According to NASM manual (check this section:
http://ivs.cs.uni-magdeburg.de/bs/lehre/sose99/bs1/nasm/nasmdoc8.html#sectio
n-8.2) it must be:

…
call .get_GOT
.get_GOT: pop ebx
add ebx,GLOBAL_OFFSET_TABLE+$$-.get_GOT wrt …gotpc
…

otherwise it will fail ! QNX6 is very sensetive to this problems with PIC
code. One mistake and you’ll get SEGFAULT. Only static library works in this
case.

Maybe better to rewrite all this code via ‘asm’ includes in C modules ? Any
suggestions ?

And three days later:

Sorry, but anyone have any comments about described problems above ? Does
anyone have the similar problems ? Btw, I’ve found that ebx register was
destroyed in some of hermes’s assembly routines, which is not allowed in the
shared object modules, which also can cause many problems, while using it.

With best regards, Mike Gorchak. E-mail: @Mike_Gorchak

Stephane_Marchesin · May 5, 2004, 5:45am

Hello, Mike!

MF> when building with -fPIC and gcc-3.4.0, a few files fail to compile

Hm, and what about hermes sublibrary in SDL ? It’s also damnely broken as
for PIC/shared. I’ve wrote about this a quite time ago, but got noone
respond.

That’s right. All this code would need a refactor, but I’m not sure of what kind it should be. There are basically three choices :

fix the nasm code : the advantage is that the work to be done is minimal, but I’m not sure we will be able to keep this code in the long run, and nasm seems to have issues on exotic platforms.
port the code to use gcc inline asm : the advantage of using gcc inline assembly is that we could have the same code work on x86_64, but breaks compatibility with visual C++ (that won’t prevent people from compiling SDL under visual C++, but will disable this mmx code).
port the code to use gas assembly : the advantage is that we avoid the whole register input/output/clobber thing and use standard calling conventions instead. Breaks compatibility with visual C++ too. Not sure this will lead to code that works on x86_64.

I’m willing to do one of these it if no one else volunteers, but I’d like to have some views (I mean - different views than mine) first.

Stephane

Mike_Frysinger · May 10, 2004, 6:18am

Hmmm… well let’s be clear :

no push/pops in the cpuinfo part (ebx is saved in edi)

yeah, i’m a tool, i missed that

Not sure that’ll work on older gcc versions.
I’ll look at it and send another patch (but probably not today, as I
need to test this on different gcc versions). Hopefully this will be the
last

here’s a combined patch (yours and the one i mentioned earlier) that i tested
with gcc-3.4.0 and gcc-3.3.3
-mike
-------------- next part --------------
A non-text attachment was scrubbed…
Name: gcc-3.4-pic2.patch
Type: text/x-diff
Size: 8196 bytes
Desc: not available
URL: http://lists.libsdl.org/pipermail/sdl-libsdl.org/attachments/20040510/af1093a1/attachment.patchOn Monday 03 May 2004 05:08 pm, Stephane Marchesin wrote:

slouken · May 16, 2004, 5:20pm

here’s a combined patch (yours and the one i mentioned earlier) that i tested
with gcc-3.4.0 and gcc-3.3.3

Thanks, I’ve applied this to CVS. Let me know if there are any problems!

See ya,
-Sam Lantinga, Software Engineer, Blizzard Entertainment