Last Comment Bug 771285 - IonMonkey: don't treat JSOP_LABEL as a jump opcode during preliminary analysis
: IonMonkey: don't treat JSOP_LABEL as a jump opcode during preliminary analysis
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Other Branch
: x86 Mac OS X
: -- normal (vote)
: ---
Assigned To: general
:
:
Mentors:
Depends on:
Blocks: 771106
  Show dependency treegraph
 
Reported: 2012-07-05 13:01 PDT by Brian Hackett (:bhackett)
Modified: 2012-07-06 14:27 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (4.00 KB, patch)
2012-07-05 13:01 PDT, Brian Hackett (:bhackett)
dvander: review+
Details | Diff | Splinter Review

Description Brian Hackett (:bhackett) 2012-07-05 13:01:33 PDT
Created attachment 639441 [details] [diff] [review]
patch

JSOP_LABEL is a no-op annotation, but the analysis passes used by type inference (bytecode, lifetimes, SSA) treat it as a jump, degrading precision.  This bites on the fannkuch benchmark in bug 771106, where the extra edges cause some locals to be treated as possibly-undefined with a lot of stub calls resulting.  Fixing this takes time for --ion -n from 24s to 12s
Comment 1 David Anderson [:dvander] 2012-07-05 13:31:34 PDT
Comment on attachment 639441 [details] [diff] [review]
patch

Review of attachment 639441 [details] [diff] [review]:
-----------------------------------------------------------------

Sweet!
Comment 2 Brian Hackett (:bhackett) 2012-07-05 14:00:09 PDT
https://hg.mozilla.org/projects/ionmonkey/rev/171cc91d4c18
Comment 3 Alon Zakai (:azakai) 2012-07-06 10:07:48 PDT
(In reply to Brian Hackett (:bhackett) from comment #0)
> Fixing this takes time for --ion -n from 24s to 12s

I don't see a big change locally or on awfy, was the test on a modified version of fannkuch (maybe the hand-optimized one in bug 771106)?
Comment 4 Brian Hackett (:bhackett) 2012-07-06 13:36:28 PDT
Yes, this is fixing the hand optimized fannkuch benchmark.  The basic one has extra shifts that coerce the value the compiler thinks may be undefined into an int32, so we don't take stub calls.  Though pretty soon I'd like to work on getting emscripten to generate code closer to the hand optimized benchmark in bug 771106, as that is the pattern I'd like to optimize for base + index accesses.
Comment 5 Alon Zakai (:azakai) 2012-07-06 13:46:39 PDT
Ok, I see, thanks.

Regarding modifying emscripten to generate fewer << >> operations, there is a good chance we would want to do that in the C++ LLVM backend we are planning to write. The current compiler is written in JS and parses LLVM bitcode externally to LLVM, it does not perform well on 1M+ codebases, and also there are various optimization benefits from implementing a C++ backend. So for new optimizations we might want to focus on that as opposed to optimizing the current compiler. Unless there is a very simple solution.

Regarding the C++ backend, the plan is for Rafael and I to start very soon, and hopefully it will not take too long.
Comment 6 Brian Hackett (:bhackett) 2012-07-06 14:08:16 PDT
(In reply to Alon Zakai (:azakai) from comment #5)
> Ok, I see, thanks.
> 
> Regarding modifying emscripten to generate fewer << >> operations, there is
> a good chance we would want to do that in the C++ LLVM backend we are
> planning to write. The current compiler is written in JS and parses LLVM
> bitcode externally to LLVM, it does not perform well on 1M+ codebases, and
> also there are various optimization benefits from implementing a C++
> backend. So for new optimizations we might want to focus on that as opposed
> to optimizing the current compiler. Unless there is a very simple solution.
> 
> Regarding the C++ backend, the plan is for Rafael and I to start very soon,
> and hopefully it will not take too long.

Sounds good to me, can you loop me in on bugs/etc. for that new backend?  I'd like to help.  (My pre-Mozilla background is after all in C/C++ static analysis)
Comment 7 Alon Zakai (:azakai) 2012-07-06 14:27:36 PDT
Sure thing, very happy you are interested in this!

Note You need to log in before you can comment on or make changes to this bug.