142594 - Linux/PPC g++-3 ABI support for XPCOM

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Franz.Sirl-kernel]

Hi, the appended patch implements g++-3 ABI awareness for Linux/PPC.

It passes the TestXPTCInvoke test and mozilla-1.0-rc2 runs fine here, compiled
with gcc-3.1pre.

Franz.

PS. The patch also contains 2 new tests for TestXPTCInvoke, which proved to be
useful.

Comment 1

[Franz.Sirl-kernel]

Attachment: gcc3 support for Linux/PPC

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 82504 [details] [diff] [review]
gcc3 support for Linux/PPC

>+GXXABIVERSION	:= $(shell $(CXX) -x c++ -dM -E - </dev/null \
>+			   | grep -w __GXX_ABI_VERSION \
>+			   | sed -e 's/.*__/__/' \
>+			   | sed -e 's/N\W\{1,\}/N=/')
>+

An autoconf test should already be setting HAVE_GCC3_ABI correctly.  See
bug 71627.

>+#if __GXX_ABI_VERSION - 0 < 100 /* G++ pre-V3 ABI */
> 	addi	r4,r4,2				# skip first two vtable entries
>+#endif

I think the preferred way to ifdef this (and the way gcc uses in its
own tests) is 
#if !defined(__GXX_ABI_VERSION) || __GXX_ABI_VERSION < 100 /* not G++ V3 ABI */
which is at least clearer about what happens when the macro
isn't defined.	(Does it become zero by default?)

I'm surprised this is being run through the preprocessor.  My testing had
shown that it wasn't, I think, which led me to work on attachment 59277 [details] [diff] [review]
on bug 71627.

>+#if __GXX_ABI_VERSION - 0 < 100

Same comment about the way to |#ifdef| this.

>+// Use assembler directives to get the names right...

Neat.  So the stuff I cooked up in bug 86446 probably isn't needed.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

> I'm surprised this is being run through the preprocessor.  My testing had
> shown that it wasn't, I think, which led me to work on attachment 59277 [details] [diff] [review]
> on bug 71627.

Oh, I see.  You changed AS in the makefile.  Perhaps you could do that based on
|#ifdef HAVE_GCC3_ABI|, and just -DHAVE_GCC3_ABI instead?

Comment 4

[Franz.Sirl-kernel]

Well, I preferred to test the same flag in assembly and C++, that's why I
mirrored __GXX_ABI_VERSION to the assembly.

On the testing style of the preprocessor macro, it is used in the GCC sources as
well, even without the '- 0' I usually use to make things a bit clearer to the
reader. In an #if construct, an undefined macro evaluates to 0.

Comment 5

[Doug Turner (:dougt)]

dbaron, your in the know.  over to you.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

->shaver

Comment 7

[Franz.Sirl-kernel]

Attachment: gcc3 support for Linux/PPC (reworked)

OK, now that I added a patch to GCC (is in gcc3.2 and later) which makes
__GXX_ABI_VERSION available to all languages, the Makefile.in hunk to mirror
this macro to assembly preprocessing is no longer necessary.
Another cosmetic change since the last patch is the change from "#" to "//"
comments now that the assembly is preprocessed.

It would be nice if this patch could make it into the 1.1 branch.

Tested with gcc-3.1.1 and gcc-3.2.

Comment 8

[Jack Howarth]

I can confirm that the latest version of this patch, submitted 
today by Franz Sirl, can build a working mozilla snapshot. On 
debian ppc sid, using Matthias Klose's gcc 3.2-0pre4 packages 
(gcc version 3.2 20020809 (Debian prerelease), I can build the
mozilla trunk from 17 Jul 2002 (which is the newest snapshot package
debian has) with no problems. The resulting mozilla runs fine.
Please try to make sure that Franz's patch makes it into mozilla 1.1.
Thanks.

Comment 9

[Mark Guertin]

I can also confirm these patches are just what the doctor ordered for Gentoo   PPC.  Mozilla builds and functions very well with these patches, and they are   now part of our portage tree.  It appears as though they didn't make the cut  for 1.1 release, but wouldbe very nice the get them into CVS ASAP as without them, ppc linux with gcc 3.2 is dead in the water :(  

Comment 10

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 96560 [details] [diff] [review]
gcc3 support for Linux/PPC (reworked)

sr=shaver.  Someone should check this in.

Comment 11

[Doug Turner (:dougt)]

Checking in Makefile.in;
/cvsroot/mozilla/xpcom/reflect/xptcall/src/md/unix/Makefile.in,v  <--  Makefile.in
new revision: 1.67; previous revision: 1.66
done
Checking in xptcinvoke_asm_ppc_linux.s;
/cvsroot/mozilla/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_asm_ppc_linux.s,v
 <--  xptcinvoke_asm_ppc_linux.s
new revision: 1.5; previous revision: 1.4
done
Checking in xptcstubs_asm_ppc_linux.s;
/cvsroot/mozilla/xpcom/reflect/xptcall/src/md/unix/xptcstubs_asm_ppc_linux.s,v 
<--  xptcstubs_asm_ppc_linux.s
new revision: 1.5; previous revision: 1.4
done
Checking in xptcstubs_ppc_linux.cpp;
/cvsroot/mozilla/xpcom/reflect/xptcall/src/md/unix/xptcstubs_ppc_linux.cpp,v 
<--  xptcstubs_ppc_linux.cpp
new revision: 1.6; previous revision: 1.5
done

Fix checked into trunk.

Comment 12

[Doug Turner (:dougt)]

does this need to be pushed onto the 1.0 branch?

Comment 13

[Marc T.]

mozilla/xpcom/reflect/xptcall/src/md/unix/Makefile.in is creating a buggy Makefile

Comment 14

[Doug Turner (:dougt)]

what is the error?

Comment 15

[Marc T.]

It seems that my tree was corrupted when it tried to merge the patches
The errors has been resolved after I deleted the faulty files and got a fresh copy off cvs
apologies for any inconveniences caused!

Comment 16

[Franz.Sirl-kernel]

OK, now that this has been resolved, it would be nice if someone could add it to
the 1.0 branch too.
Thanks.

Comment 17

[Doug Turner (:dougt)]

I thought I had sent mail to drivers; I screwed up. I sent the mail.  As soon as
it is marked approved, I will land this patch.