I will accept this bug.  In the meantime, please use gcc3.

I'm half ready with a patch that masks out the clobbered registers not known to
the compiler with #ifdef __MMX__/#ifdef __SSE__. I'll post it here when I'm ready.

I don't know what you mean, exactly.  But please coordinate with me as I have
been working on the mmx, sse, and sse2 code with some other cleanup and
improvements.

Created attachment 46399 [details] [review]
Patch for GIMP 2.2.7 to compile with gcc 4.0

Newer gcc versions complain if you use MMX/SSE only registers in clobber lists
of inline asm statements (unless you use -mmmx/-msse/-msse2 which is out of the
question because it causes the compiler to generate MMX/SSE instructions from C
code which will not run on all CPUs).

This patch wraps MMX/SSE only registers in clobber lists inside #ifdef __MMX__
... #endif or #ifdef __SSE__ ... #endif respectively. Hiding these registers
from the compiler is okay in that case as it doesn't know or use these
registers for C code.

the patch works fine, thanks

While I understand the text you typed in your comment, I'm not following the
intended effect of this patch.  The asm clobber lists should be explicit about
what registers are clobbered.  If I've been negligent in specifying which
registers have been clobbered (which I have in several cases), that's one
problem which I prefer to fix by properly specifying the right list.

Secondly, I'm using gcc version 4.0.0 to test with and I haven't seen the
complaints you mention.  Could you include the actual output of the compiler
either here, or send it to me directly?

OK, maybe I can make myself clearer ;-).

The compiler now complains if you reference registers in clobber lists that it
wouldn't use by itself or (in other words) that it doesn't know about. When not
using -mmmx, -msse or -msse2 it doesn't know about or use the %mm* registers,
when not using -msse or -msse2 it doesn't know about or use the %xmm* registers
and in both cases complains if they are specified in the clobber lists. The fix
just omits these registers in clobber lists eventually, but it doesn't hurt
because the compiler doesn't use them either so it doesn't care if they are
clobbered or not.

AIUI, this change was made post gcc-4.0.0 and will likely be in later versions
of gcc-4.0.

I understand now, thanks.  I suspect I didn't understand because I couldn't
conceive of something this dumb.  After all, if one is using asm() constructs it
is because you're trying to outdo the compiler.  Now it presumes to utter
commentary on your attempts.

I cannot reproduce this behaviour.  It would be helpful to either a) attach or
send me the output from the compiler that you see, or b) tell me how to
reproduce it, or c) see if my latest updates to gimp-composite-sse2.c improve
(or otherwise change) this behaviour.

For starters here's the result of trying to compile
app/composite/gimp-composite-mmx.c:

nils@gibraltar:~/devel/cvs/dist/gimp/devel/gimp-2.2.7/app/composite> make
if gcc -DHAVE_CONFIG_H -I. -I. -I../.. -I../.. -I../../app
-I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include   -I/usr/include
-DG_LOG_DOMAIN=\"Gimp-Composite\" -D_REENTRANT -DGIMP_DISABLE_DEPRECATED
-DG_DISABLE_DEPRECATED -DGDK_PIXBUF_DISABLE_DEPRECATED -DGDK_DISABLE_DEPRECATED
-DGTK_DISABLE_DEPRECATED -DPANGO_DISABLE_DEPRECATED -DGDK_MULTIHEAD_SAFE
-DGTK_MULTIHEAD_SAFE  -O2 -g -pipe -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -m32
-march=i386 -mtune=pentium4 -fasynchronous-unwind-tables -Wall -MT
gimp-composite-mmx.o -MD -MP -MF ".deps/gimp-composite-mmx.Tpo" -c -o
gimp-composite-mmx.o gimp-composite-mmx.c; \
then mv -f ".deps/gimp-composite-mmx.Tpo" ".deps/gimp-composite-mmx.Po"; else rm
-f ".deps/gimp-composite-mmx.Tpo"; exit 1; fi
gimp-composite-mmx.c:835: warning: ‘mmx_op_overlay’ defined but not used
gimp-composite-mmx.c: In function ‘gimp_composite_addition_rgba8_rgba8_rgba8_mmx’:
gimp-composite-mmx.c:94: error: unknown register name ‘%mm0’ in ‘asm’
gimp-composite-mmx.c:101: error: unknown register name ‘%mm4’ in ‘asm’
gimp-composite-mmx.c:101: error: unknown register name ‘%mm3’ in ‘asm’
gimp-composite-mmx.c:101: error: unknown register name ‘%mm2’ in ‘asm’
gimp-composite-mmx.c:101: error: unknown register name ‘%mm1’ in ‘asm’
[... snipped for brevity ...]
gimp-composite-mmx.c:1279: error: unknown register name ‘%mm4’ in ‘asm’
gimp-composite-mmx.c:1279: error: unknown register name ‘%mm3’ in ‘asm’
gimp-composite-mmx.c:1279: error: unknown register name ‘%mm2’ in ‘asm’
gimp-composite-mmx.c:1279: error: unknown register name ‘%mm1’ in ‘asm’
make: *** [gimp-composite-mmx.o] Error 1

Compiling the files gimp-composite-sse.c and -sse2.c shows similar errors. To
reproduce it, you might have to checkout gcc from gcc-4_0-branch CVS.

I am curious about your build environment because your Makefile is including
several specific options which I don't have from a regularly created autogen.sh
Makefiles.  Importantly, you have the gcc option -mtune=pentium4 which the gcc
manual explicitly states:
...
       -mtune=cpu-type
           Tune to cpu-type everything applicable about the generated code,
           except for the ABI and the set of available instructions.  The
           choices for cpu-type are:
...

           i586, pentium
               Intel Pentium CPU with no MMX support.

Similarly, your Makefile specfies the -marchi386 option to gcc, which by
specifying -march=cpu-type implies -mtune=cpu-type.

Given all of this, I think that these errors are what one should expect when
using these gcc options and that it would be best to figure out why these gcc
options are set and not work around their effects with #ifdef 

Well, then again maybe not. My apologies. The option pentium4 is for the Intel
Pentium4 CPU with MMX, SSE and SSE2 instruction set support.  I quoted your
build environment incorrectly.

At this point, I'm still curious (and suspicious) about your build environment
and these options.  I don't see these errors when I use the gcc 4.0.0 release
and a regular autogen.sh set of Makefiles. Since you are using gcc from cvs, I
don't think it is prudent to #ifdef code to accomodate possibly transient
behaviour on a pre-release of gcc.

So, unless you have more authoritative information on how the next release of
gcc will handle this case, I'll take this bug on advisement.

I would like to know from Sebastien if the original description of this bug is
now address in Gimp cvs HEAD.

The build environment is nothing fancy, just (what will become) Fedora Core 4
and the standard optimization flags we have used in building RPM packages for
quite a while. I don't think that the change is transient as it came from the
(upstream) gcc stable branch.

Is there some documentation from the gcc folks on the rationale of this change?

same issue with the CVS.

Ubuntu uses "CVS 20050509, taken from the gcc-4_0-branch"

The patch from this bugzilla works fine, the current gimp package uses it

I can interview our gcc people about this matter.

At this point, I can confirm that while using a regular set of Makefiles
generated from autogen.sh, 'gcc (GCC) 4.1.0 20050515 (experimental)' from cvs
complains about the presence of the mmx registers in the clobber lists.

At this, point, however, I think it is still best to take this report under
advisement until we get a clearer idea of just what is going on with gcc.

Helvetix, what exactly do you mean when you say that the report is under
advisement? Is that correctly reflected by the current status of this report?

I saw a checkin in the HEAD branch that I assume is related to this report.
Unfortunately the associated ChangeLog entry doesn't mention a bug number and it
is also not mentioned here, so I can only guess :(

I would very much appreciate if all commits to CVS that somehow relate to an
existing bug report would refer to that report _and_ that the relevant ChangeLog
entry is immidiately added as a comment to the bug report.

Mea culpa.  This report was distorted by the tangental problems with gcc, and
the resolution of the original problem was left out.

I think this bug should be closed as FIXED.

Do you think this fix should be backported to the 2.2 branch? We might be doing
a 2.2.8 release at some point...

If we expect people to be compiling with gcc 3.0, then yes.

Helvetix, if you consider this to be worth being backported, then please do that.

I am confused by this bug report.  In trying to build cvs using Fedora Core 4, I
get the error messages shown in comment #9.  Is this problem supposed to have
been fixed, or not?  If this is a "tangential" problem, shall I open a new bug
report about it?

This bug is closed, but the tangental problem is still an open issue because the
compiler that Fedora Core uses is inconsistent with previous versions of the
compiler.

I've been waiting to hear back from Nils Philippsen (nphilipp@redhat.com) for
other clues.

To summarise, the newer compiler will not recognise the names for the mmx/sse
registers unless you specify the -mmmx and/or -msse, -msse2 (and perhaps other
combinations of -mtune and -march) compiler flags.  Unfortunately, if you
specify these flags, you also will cause the compiler to generate other machine
code that uses these instructions, from other places in the gimp, for older
computers that don't have these instructions.

This is wacky enough behaviour to suspect that it's unintentional.  The compiler
that I've seen cause this problem is "gcc (GCC) 4.1.0 20050515 (experimental)".
 Since it is obviously "experimental" I claimed that we should wait to see what
the gcc folks actually have to say about this behaviour before trying to fix it
with giant #ifdef'd sections of code just to work around this.

Okay; I have created bug #308142 to follow up on this issue.

Amend that last comment (#25) to be bug #308412

So, Nils, did you talk with the gcc people yet?

Just FYI, the GIMP debian package (gimp 2.2.8-5) seems to ship with this fix now.

We should really try hard to get a fix released soon. Are we still waiting from
response from Nils?

It seems so (sorry for not getting back on this earlier). I just confirmed that
this change is in gcc 4.0.1 that was released beginning of July.

For those building Gimp from source, would it be safe to add the "-mmmx" flag to
CFLAGS provided our CPU supports MMX instructions?  I'm trying to build either
Gimp 2.2.8 or 2.3.2 and can't since I have gcc-4.0.1 installed.

Thanks!

Peace...

Why don't you just try it? It works for me.

These issues have been addressed now, the solution is to use -mmmx and -msse
only when compiling the bits that contain the assembly.

Manish, IMO that isn't the right approach (see comment #24) -- using -mmmx,
-msse, -msse2 causes the compiler to generate MMX and SSE assembly from C code
(in addition to knowing about the MMX/SSE only registers). The attached patch
will list the MMX/SSE only registers as clobbered only if the compiler would use
them for its own purposes, i.e. if -mmmx, -msse or -msse2 is specified on the
command line.

Right, so the solution I committed only uses those CFLAGS on
gimp-composite-{mmx,sse,sse2}.c, and not on any other files. Since those files
contain code that is only used if the appropriate cpu features are detected, it
doesn't matter if the compiler decides to emit mmx or sse from C code too.

This is much better than cluttering the code with #ifdefs, and your patch did
the wrong thing on pre gcc4 anyway.

Care to elaborate what went wrong with pre gcc4? Mind, what the patch does is
what the compiler developers told me was the right thing to do.

I've not actually tested it, but from past bug reports, pre gcc4 compilers were
sensitive to the contents of the clobber list, and since those compilers won't
define __MMX__ or __SSE__ out of the blue either, they'd be masked out, which
could lead to potential problems.

Regardless, the #ifdefs are horribly ugly and make the code harder to read. The
compiler developers should revisit their recommendation. ;)

I don't think it would lead to problems because on the one hand __MMX__/__SSE__
only get defined if -mmmx/-msse is used, but the compiler will use those
registers only if -mmmx/-msse is used as well. If not, the compiler couldn't
care less about what a program does to these registers in inline assembly ;-).

That's the theory. In practice, it seems to have mattered for some versions.

*** Bug 315484 has been marked as a duplicate of this bug. ***