Closed Bug 420906 Opened 12 years ago Closed 12 years ago

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

Categories

(Core :: JavaScript Engine, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED

People

(Reporter: igor, Assigned: ted)

References

Details

Attachments

(1 file, 1 obsolete file)

+++ 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.
Assignee: general → ted.mielczarek
Flags: blocking1.9+
Priority: -- → P2
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?
(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.
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
Yes - but we don't have PGO on the mac..
(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.
(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
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)
(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 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-
I have seen that before, I just completely forgot about it. Thanks!
Attachment #308849 - Attachment is obsolete: true
Attachment #309121 - Flags: review?(benjamin)
Attachment #309121 - Flags: review?(benjamin) → review+
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
Closed: 12 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Flags: in-testsuite-
Flags: in-litmus-
You need to log in before you can comment on or make changes to this bug.