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

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P2)

Description

[Igor Bukanov]

+++ 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.

Comment 1

[(not currently active) Ted Mielczarek]

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?

Comment 2

[Igor Bukanov]

(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.

Comment 3

[(not currently active) Ted Mielczarek]

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

[Mike Schroepfer]

Yes - but we don't have PGO on the mac..

Comment 5

[Igor Bukanov]

(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

[Robert Sayre]

(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.

Comment 7

[Brendan Eich [:brendan]]

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

Comment 8

[(not currently active) Ted Mielczarek]

Attachment: 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.

Comment 9

[Igor Bukanov]

(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

[Benjamin Smedberg]

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

Comment 11

[(not currently active) Ted Mielczarek]

Attachment: with target variable override

I have seen that before, I just completely forgot about it. Thanks!

Comment 12

[Robert Sayre]

Checking in js/src/Makefile.in;
/cvsroot/mozilla/js/src/Makefile.in,v  <--  Makefile.in
new revision: 3.121; previous revision: 3.120
done