support for custom options for jsinterp.c when compiling the browser

RESOLVED FIXED

Status

()

Core
JavaScript Engine
P2
enhancement
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: Igor Bukanov, Assigned: ted)

Tracking

Trunk
Points:
---
Bug Flags:
blocking1.9 +
in-testsuite -
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

10 years ago
+++ This bug was initially created as a clone of Bug #419969 +++

It would be nice to be able to specify customized compilation flags when compiling js_Interpret for a browser build. It will allows to apply specific optimizations to the interpreter loop without affecting the rest of code.

Updated

10 years ago
Assignee: general → ted.mielczarek
Flags: blocking1.9+
Priority: -- → P2
(Assignee)

Comment 1

10 years ago
This is hard given the way our build system works. Is this really necessary? Can we just get most of the benefit here by using PGO or other compiler optimizations?
(Reporter)

Comment 2

10 years ago
(In reply to comment #1)
> This is hard given the way our build system works. Is this really necessary?
> Can we just get most of the benefit here by using PGO or other compiler
> optimizations?

What I have observed with GCC 4.1 is that passing -O3 when compiling just jsinterp.c while keeping the rest of files at -Os benefited SunSpider more then compiling the whole SpiderMonkey with -Os. I suspect that this is due to extra code bloat and cache penalties that -O3 produces.

As such the title of the bug is somewhat misleading: what is really necessary is the ability to hardcode a different set of options in Makefile.in for jsinterp.c in comparison with the rest of SpiderMonkey. I.e. with GCC it can be -O3, but with MSVC it can be something else. There is no need to support a configure option.
(Assignee)

Comment 3

10 years ago
Right, but specifying compiler options for one source file is not supported in our build system, so it would be pretty hacky. Have you tried doing a PGO build and comparing performance? It's pretty easy:
http://developer.mozilla.org/en/docs/Building_with_Profile-Guided_Optimization

Comment 4

10 years ago
Yes - but we don't have PGO on the mac..
(Reporter)

Comment 5

10 years ago
(In reply to comment #3)
> Right, but specifying compiler options for one source file is not supported in
> our build system, so it would be pretty hacky. Have you tried doing a PGO build
> and comparing performance?

I will try but given that PGO affects the branch prediction and not the general optimization strategy that the compiler uses, I suspect that custom options for jsinterp.c is an orthogonal feature from performance gains point of view.

Comment 6

10 years ago
(In reply to comment #5)
> (In reply to comment #3)
> > Right, but specifying compiler options for one source file is not supported in
> > our build system, so it would be pretty hacky. 

We have a need for speed. :)

> I suspect that custom options for
> jsinterp.c is an orthogonal feature from performance gains point of view.

I agree.

Our build system is based on make (GNU make, but doesn't matter here). We know this cuz it is slow. :-/ But with make one can always write an explicit rule for one .o file, to customize how it is built, and let the inference rule take care of the rest. I don't see why this has to be a hack in the bad sense of the word.

/be
(Assignee)

Comment 8

10 years ago
Created attachment 308849 [details] [diff] [review]
allow custom optimization flags for jsinterp.c

Ok, this isn't so bad. I ifdef'ed the rule out so that on platforms where we don't care to define special optimization options, we'll just use the default rules. I've only defined it here for GCC, where we're using -O3. Should be easy enough to do a follow on patch to change this elsewhere if someone wants to do the testing and find out what we want there.
Attachment #308849 - Flags: review?(benjamin)
(Reporter)

Comment 9

10 years ago
(In reply to comment #8)
> Created an attachment (id=308849) [details]
> allow custom optimization flags for jsinterp.c
> 
> Ok, this isn't so bad. I ifdef'ed the rule out so that on platforms where we
> don't care to define special optimization options, we'll just use the default
> rules. I've only defined it here for GCC, where we're using -O3. Should be easy
> enough to do a follow on patch to change this elsewhere if someone wants to do
> the testing and find out what we want there.

I have checked in the js shell changes from bug 420904 but I have not changed the options itself. This is to make sure that the shell does not deviate from the browser. When this will be ready, then js/src/config.mk should also also be updated with new options. 

Comment 10

10 years ago
Comment on attachment 308849 [details] [diff] [review]
allow custom optimization flags for jsinterp.c

I don't think we need to copy the whole rule: use a target-specific variable definition:

jsinterp.$(OBJ_SUFFIX): MODULE_OPTIMIZE_FLAGS=$(INTERP_OPTIMIZER)

This is less fragile and less likely to get out of sync with the main config/rules.mk

see http://www.gnu.org/software/automake/manual/make/Target_002dspecific.html#Target_002dspecific
Attachment #308849 - Flags: review?(benjamin) → review-
(Assignee)

Comment 11

10 years ago
Created attachment 309121 [details] [diff] [review]
with target variable override

I have seen that before, I just completely forgot about it. Thanks!
Attachment #308849 - Attachment is obsolete: true
Attachment #309121 - Flags: review?(benjamin)

Updated

10 years ago
Attachment #309121 - Flags: review?(benjamin) → review+
(Assignee)

Updated

10 years ago
Keywords: checkin-needed

Comment 12

10 years ago
Checking in js/src/Makefile.in;
/cvsroot/mozilla/js/src/Makefile.in,v  <--  Makefile.in
new revision: 3.121; previous revision: 3.120
done
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED

Updated

10 years ago
Flags: in-testsuite-
Flags: in-litmus-
You need to log in before you can comment on or make changes to this bug.