237545 - Need to support objmodel=ibm for AIX Visual Age C'C++ compiler

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement)

Description

[Mary Hoetzel]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4.1) Gecko/20031008
Build Identifier: Mozilla/5.0 (X11; U; AIX ...; en-US; rv:1.4.1) 

We have a request to support the ibm compiler object model with the AIX version
of Mozilla. This means using -qobjmodel=ibm instead of -qobjmodel=compat (which
is the default and not explicitely set today).
The compiler object model determines the compiler's vtable structure. It
requires to add ibm objmodel specific xptcinvoke assembler code. 
We want to make the object model a build configuration with -qobjmodel=ibm being
the new default while still obtaining the possibility to compile with
-qobjmodel=compat.   

Reproducible: Always
Steps to Reproduce:
N/a - this is not a defect but a new feature

Comment 1

[Mary Hoetzel]

Attachment: Move to -qobjmodel=ibm VA compiler setting for Mozilla on AIX

This patch adds a new assembler file, xptcinvoke_asm_ppc_ibmobj_aix.s, for the
Visual Age ibm object model specific vtable access code. 

It changes configure.in to set the object model and define the AIX_OBJMODEL
variable:
- -qobjmodel=ibm is the new default
- -qobjmodel=compat is still available by specifying
CXXFLAGS="-qobjmodel=compat" in the .mozconfig file.

The change is AIX specific except that I had to move the code to check for
64-bit more to the top of configure.in so that the result is available when we
need to set the object model.
The object model change is for 32-bit only (at this point).

Comment 2

[Boris Zbarsky [:bzbarsky]]

brendan, who's the right person to review this?

Comment 3

[Brendan Eich [:brendan]]

Search me.  Cc'ing some likelies, reassigning to best-fitting component.

/be

Comment 4

[Jim Dunn]

Basically, IBM are the only people who are keeping tabs of the AIX port.
I figure between phil and myself we can hammer out the compiler issues.
If we just keep CLS in the loop, I don't see it as a big deal.

The big question is if everyone is ok with putting
    AIX_OBJMODEL = @AIX_OBJMODEL@
into configure.in and autoconf.mk

NOTE: I do like the fact that they don't break the other code...
and it is all determined by a compile time flag.

Hopefully I can do a couple of sample builds with this before
the end of the week and I will let you know.

Comment 5

[Mary Hoetzel]

Attachment: Enable -qobjmodel=ibm but keep -qobjmodel=compat the Mozilla default

This updated patch enables the usage of -qobjmodel=ibm as in the obsolete patch
but keeps -qobjmodel=compat as the default for Mozilla.

Comment 6

[Mary Hoetzel]

Comment on attachment 144225 [details] [diff] [review]
Enable -qobjmodel=ibm but keep -qobjmodel=compat the Mozilla default

cls: Could you please look over the build changes in this bug?

shaver: Are you ok with reviewing the assembler changes or should we find
another reviewer?

Comment 7

[Mike Shaver (:shaver emeritus)]

Comment on attachment 144225 [details] [diff] [review]
Enable -qobjmodel=ibm but keep -qobjmodel=compat the Mozilla default

In that they won't break other, existing builds, I'm fine with them.  If
there's someone else who knows the assembly well enough to comment on the
quality of it, that would obviously be good, but it's not necessary for
checking in a new port.

Comment 8

[Mary Hoetzel]

To explain the differences in the assembler code:

diff -u xptcinvoke_asm_ppc_aix.s xptcinvoke_asm_ppc_ibmobj_aix.s

@@ -104,14 +103,9 @@
                lwz     r3,160(r31)             # that
                lwz     r4,0(r3)                # get vTable from 'that'
                lwz     r5,164(r31)             # methodIndex
-               slwi    r5,r5,3                 # methodIndex * 8
-               addi    r5,r5,8                 # step over junk at start of
vTable !
+               slwi    r5,r5,2                 # methodIndex * 4
                lwzx    r11,r5,r4               # get function pointer
 
-               addi    r5,r5,4                 # We need to manually adjust the
'that' pointer, this is CF
RONT based
-               lwzx    r5,r4,r5                # offset = r4(vtable) +
r5(methodIndex offset) - 4 
-               add     r3,r5,r3                # adjust 'that' r3 = r3 + r5
-               
                lwz     r4,28(sp)
                lwz     r5,32(sp)
                lwz     r6,36(sp)


The CFRONT based compat object model vtable has 8 bytes junk at the beginning
which are ignored, and a 4 byte offset adjustment for each function pointer
which must be made. 
The thunking based ibm object model vtable does not require these adjustments.   

Comment 9

[cls]

Comment on attachment 144225 [details] [diff] [review]
Enable -qobjmodel=ibm but keep -qobjmodel=compat the Mozilla default

+	 if test ! "$HAVE_64BIT_OS"; then
+	     # Compiling with Visual Age C++ object model compat is the
+	     # default. To compile with object model ibm, add 
+	     # CXXFLAGS="$CXXFLAGS -qobjmodel=ibm" to .mozconfig.
+	     if test `echo "$CXXFLAGS" | grep -c 'qobjmodel=ibm' 2>/dev/null`
-ne 0; then
+		CXXFLAGS="$CXXFLAGS -qobjmodel=ibm"
+		AIX_OBJMODEL=ibm
+	     fi
+	 fi

You're adding -qobjmodel=ibm to CXXFLAGS after checking for qobjmodel=ibm in
CXXFLAGS.  That seems unnecessary.  In fact, I'd suggest setting
AIX_OBJMODEL=ibm in mozconfig and then modifying CXXFLAGS in configure.in if
necessary.

Comment 10

[Mary Hoetzel]

Attachment: Enable -qobjmodel=ibm with AIX_OBJMODEL=ibm in .mozconfig

This patch expects AIX_OBJMODEL=ibm to be set in .mozconfig in order to compile
with object model ibm and adds -qobjmodel=ibm to CXXFLAGS in this case.
If AIX_OBJMODEL is not set to ibm, configure.in will set AIX_OBJMODEL=compat.

Comment 11

[cls]

Comment on attachment 144273 [details] [diff] [review]
Enable -qobjmodel=ibm with AIX_OBJMODEL=ibm in .mozconfig

r=cls

Comment 12

[Mary Hoetzel]

Comment on attachment 144273 [details] [diff] [review]
Enable -qobjmodel=ibm with AIX_OBJMODEL=ibm in .mozconfig

This is an AIX specific change which we would like to get into 1.7.

Comment 13

[chris hofmann]

Comment on attachment 144273 [details] [diff] [review]
Enable -qobjmodel=ibm with AIX_OBJMODEL=ibm in .mozconfig

a=chofmann for 1.7

Comment 14

[Philip K. Warren]

Fix checked in.

Checking in configure.in;
/cvsroot/mozilla/configure.in,v  <--  configure.in
new revision: 1.1327; previous revision: 1.1326
done
Checking in config/autoconf.mk.in;
/cvsroot/mozilla/config/autoconf.mk.in,v  <--  autoconf.mk.in
new revision: 3.309; previous revision: 3.308
done
Checking in xpcom/reflect/xptcall/src/md/unix/Makefile.in;
/cvsroot/mozilla/xpcom/reflect/xptcall/src/md/unix/Makefile.in,v  <--  Makefile.in
new revision: 1.75; previous revision: 1.74
done
RCS file:
/cvsroot/mozilla/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_asm_ppc_ibmobj_aix.s,v
done
Checking in xpcom/reflect/xptcall/src/md/unix/xptcinvoke_asm_ppc_ibmobj_aix.s;
/cvsroot/mozilla/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_asm_ppc_ibmobj_aix.s,v
 <--  xptcinvoke_asm_ppc_ibmobj_aix.s
initial revision: 1.1
done