Closed
Bug 464127
Opened 16 years ago
Closed 16 years ago
Don't use -Os with Intel C/C++ compilers
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: jimb, Assigned: jimb)
Details
Attachments
(1 file, 2 obsolete files)
4.84 KB,
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
Intel recommends against the use of -Os, and using it seems to produce incorrect code in many recent versions of Intel's compilers. js/src/Makefile.in tries to use -Os only with GCC, but it only tests INTEL_CC, ignoring INTEL_CXX.
Assignee | ||
Comment 1•16 years ago
|
||
Intel recommends against the use of -Os, and using it seems to produce incorrect code in many recent versions of Intel's compilers. js/src/Makefile.in tries to use -Os only with GCC, but it only tests INTEL_CC, ignoring INTEL_CXX. (The try server likes this patch.)
Attachment #347484 -
Flags: review?(ted.mielczarek)
Updated•16 years ago
|
Attachment #347484 -
Flags: review?(ted.mielczarek) → review-
Comment 2•16 years ago
|
||
Comment on attachment 347484 [details] [diff] [review] Bug 464127 - Don't use -Os with Intel C/C++ compilers *-*linux*) - if test "$GNU_CC"; then + # Note: both GNU_CC and INTEL_CC are set when using Intel's C compiler. + # Similarly for GNU_CXX and INTEL_CXX. + if test "$INTEL_CC" || test "$INTEL_CXX"; then This is in the Linux block. I didn't think anyone was using ICC on Linux. -ifdef GNU_CC -ifdef INTEL_CC +# The $(value X) construct lets us test X's unexpanded value, just as +# 'ifdef' does. +ifneq (,$(value GNU_CC)$(value GNU_CXX)) +ifneq (,$(value INTEL_CC)$(value INTEL_CXX)) Since SpiderMonkey is all-C++, you should only have to test INTEL_CXX now.
Assignee | ||
Comment 3•16 years ago
|
||
(In reply to comment #2) > (From update of attachment 347484 [details] [diff] [review]) > *-*linux*) > - if test "$GNU_CC"; then > + # Note: both GNU_CC and INTEL_CC are set when using Intel's C compiler. > + # Similarly for GNU_CXX and INTEL_CXX. > + if test "$INTEL_CC" || test "$INTEL_CXX"; then > > This is in the Linux block. I didn't think anyone was using ICC on Linux. ICC does support Linux; shouldn't we try to make it work? > -ifdef GNU_CC > -ifdef INTEL_CC > +# The $(value X) construct lets us test X's unexpanded value, just as > +# 'ifdef' does. > +ifneq (,$(value GNU_CC)$(value GNU_CXX)) > +ifneq (,$(value INTEL_CC)$(value INTEL_CXX)) > > Since SpiderMonkey is all-C++, you should only have to test INTEL_CXX now. Okay, sure.
Assignee | ||
Comment 4•16 years ago
|
||
Intel recommends against the use of -Os, and using it seems to produce incorrect code in many recent versions of Intel's compilers. js/src/Makefile.in tries to use -Os only with G++, but it tests INTEL_CC, not INTEL_CXX --- even though almost all the sources are C++. Forbid the use of mixed Intel/GNU compilers, and check INTEL_CXX.
Attachment #347484 -
Attachment is obsolete: true
Attachment #347628 -
Flags: review?(ted.mielczarek)
Comment 5•16 years ago
|
||
Comment on attachment 347628 [details] [diff] [review] Bug 464127 - Don't use -Os with Intel C/C++ compilers +dnl You can't mix and match C and C++ compilers. Certainly the +dnl Mozilla Makefiles don't allow us to establish different +dnl MOZ_OPTIMIZE_FLAGS and MOZ_DEBUG_FLAGS for the C and C++ +dnl compilers. I don't know that I agree with this. For example, we may not be able to build NSPR with icc, but we could build js/src with icpc, so we might wind up using gcc and icpc together. Regardless, does SpiderMonkey have any straight C files left that matter?
Assignee | ||
Comment 6•16 years ago
|
||
Our current build system means that mixing compilers requires us to use least-common-denominator compiler flags. Our tree still has significant stretches of C code, so this doesn't seem useful to me. That aside, js/src is, in fact, almost entirely C++ (and could be made completely so without too much effort), so I'll go along with you that C++ is all we care about here.
Assignee | ||
Comment 7•16 years ago
|
||
Intel recommends against the use of -Os, and using it seems to produce incorrect code in many recent versions of Intel's compilers. js/src/Makefile.in tries to use -Os only with G++, but it tests INTEL_CC, not INTEL_CXX --- even though almost all the sources are C++. Check INTEL_CXX instead.
Attachment #347628 -
Attachment is obsolete: true
Attachment #347798 -
Flags: review?(ted.mielczarek)
Attachment #347628 -
Flags: review?(ted.mielczarek)
Assignee | ||
Updated•16 years ago
|
Assignee: general → jim
Status: NEW → ASSIGNED
Updated•16 years ago
|
Attachment #347798 -
Flags: review?(ted.mielczarek) → review+
Assignee | ||
Comment 8•16 years ago
|
||
Landed in tracemonkey hg as changeset 19028cd2bad2.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Updated•16 years ago
|
Flags: in-testsuite-
Flags: in-litmus-
You need to log in
before you can comment on or make changes to this bug.
Description
•