NSPR still uses the MSVC -G5 flag

RESOLVED FIXED

Status

defect
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: wtc, Assigned: julien.pierre)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Reporter

Description

12 years ago
This bug does not affect products such as Mozilla clients
that use the WIN95 build configuration.

mozilla/nsprpub/configure.in has:

1540         if test "$OS_TARGET" = "WINNT"; then
1541             CFLAGS="$CFLAGS -GT"
1542             if test "$CPU_ARCH" = "x86"; then
1543                 CFLAGS="$CFLAGS -G5"
1544             fi
1545             LIBNSPR='$(dist_libdir)/libnspr$(MOD_MAJOR_VERSION).$(LIB_SUFFIX)'
1546             LIBPLC='$(dist_libdir)/libplc$(MOD_MAJOR_VERSION).$(LIB_SUFFIX)'
1547         else
1548             LIBNSPR='$(dist_libdir)/nspr$(MOD_MAJOR_VERSION).$(LIB_SUFFIX)'
1549             LIBPLC='$(dist_libdir)/plc$(MOD_MAJOR_VERSION).$(LIB_SUFFIX)'
1550         fi

I remember the /G5 flag was added at the request of
Netscape's server teams when we're using MSVC 4.2 in
1996 or 1997.  The relevant text from MSVC 6.0
documentation is reproduced below:
(http://msdn2.microsoft.com/en-us/library/aa235437(VS.60).aspx)

/GB   Blend     Optimizes the code created to favor the Pentium.
                It blends the optimizations for the 80386 (/G3),
                80486 (/G4), Pentium (/G5), and Pentium Pro (/G6)
                options. This option forces a value of 500 for
                the _M_IX86 preprocessor macro. This value is the
                default. Future versions of the compiler will issue
                a different value to reflect the dominant processor.

/G5   Pentium   Optimizes the code created to favor the Pentium.
                Use this option for programs meant only for the
                Pentium. This option forces a value of 500 for
                the _M_IX86 preprocessor macro.

Optimizing for Pentium is out of date.  MSVC 2005 doesn't support
the /G5 flag any more.  I suggest that we just remove the /G5 flag
because the default /GB flag in MSVC 6.0 seems better than /G5 to me,
and in future versions of MSVC the default /GB flag "will [...] reflect
the dominant processor."  It isn't worth our time to write code
that uses /G5 only for MSVC 4.2 and 5.0.
Does this need to be fixed in both configure and configure.in?
Assignee

Updated

12 years ago
Summary: NSPR still uses the MSVC /G5 flag → NSPR still uses the MSVC -G5 flag
Assignee

Comment 2

12 years ago
Wan-Teh,

Unfortunately, -GB is unsupported in VC2005 (VC8) also.
Should we just fix this by removing -G5 and not adding any -G(x) flag ?
Reporter

Comment 3

12 years ago
Julien, that's fine by me.
Assignee

Comment 4

12 years ago
Assignee: wtc → julien.pierre.boogz
Status: NEW → ASSIGNED
Attachment #280556 - Flags: review?
Assignee

Updated

12 years ago
Attachment #280556 - Flags: review? → review?(wtc)
Reporter

Comment 5

12 years ago
Comment on attachment 280556 [details] [diff] [review]
remove -G5 flag

r=wtc.  We need to regenerate the configure script using autoconf.
Julien, do you have the setup to do that?  If not, you can check
in this patch for now.  (The line numbers in configure for debugging
purposes will be off by 3.)
Attachment #280556 - Flags: review?(wtc) → review+
Assignee

Comment 6

12 years ago
Wan-Teh, thanks for the review.

I don't have the setup to regenerate the configure script. I checked in the patch to the trunk.

Checking in configure;
/cvsroot/mozilla/nsprpub/configure,v  <--  configure
new revision: 1.219; previous revision: 1.218
done
Checking in configure.in;
/cvsroot/mozilla/nsprpub/configure.in,v  <--  configure.in
new revision: 1.222; previous revision: 1.221
done
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.