Last Comment Bug 740622 - IonMonkey: Check for disabled script before inlining.
: IonMonkey: Check for disabled script before inlining.
Status: RESOLVED FIXED
: perf
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: x86_64 Linux
: -- normal (vote)
: ---
Assigned To: Nicolas B. Pierron [:nbp]
:
Mentors:
Depends on:
Blocks: IonSpeed
  Show dependency treegraph
 
Reported: 2012-03-29 15:11 PDT by Nicolas B. Pierron [:nbp]
Modified: 2012-03-29 18:08 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
[part 0] Check if inline candidates are disabled or not. (1.80 KB, patch)
2012-03-29 15:11 PDT, Nicolas B. Pierron [:nbp]
no flags Details | Diff | Review
Do not inline uncompilable functions (1.89 KB, patch)
2012-03-29 15:30 PDT, Nicolas B. Pierron [:nbp]
sstangl: review+
Details | Diff | Review

Description Nicolas B. Pierron [:nbp] 2012-03-29 15:11:49 PDT
Created attachment 610716 [details] [diff] [review]
[part 0] Check if inline candidates are disabled or not.

https://bugzilla.mozilla.org/show_bug.cgi?id=739901#c3

The test case of bug739901, fail to recompile because the potential inlinable function has an instruction which cause failure of the compilation.  When we recompile and try to inline the other script, we end-up not being able to compile the current one and flag the current one as disabled.

./iongraph /tmp/ion.json
 function 0 (_build/bug739901.js:9): abort during SSA construction.
 function 1 (_build/bug739901.js:5): success; 12 passes.
 function 2 (_build/bug739901.js:1): abort during SSA construction.
 function 3 (_build/bug739901.js:5): abort during SSA construction.

The attached patch fix this issue by checking if the function is flagged as disabled or not, which happens when the function is hot enough to be compiled individually, before we even try to inline it.  So, at the recompilation time, we will just recompile without the recompile check.

The problem is that this patch cause 7% failures all over the test-suite, and these failures have to be fixed before.
Comment 1 Nicolas B. Pierron [:nbp] 2012-03-29 15:16:49 PDT
Comment on attachment 610716 [details] [diff] [review]
[part 0] Check if inline candidates are disabled or not.

The check is made too early, before checking if we can reach the script.
I will update it soon.
Comment 2 Nicolas B. Pierron [:nbp] 2012-03-29 15:30:12 PDT
Created attachment 610724 [details] [diff] [review]
Do not inline uncompilable functions

Fix previously buggy patch.
Comment 3 Sean Stangl [:sstangl] 2012-03-29 16:26:17 PDT
Comment on attachment 610724 [details] [diff] [review]
Do not inline uncompilable functions

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

It looks like this only hits in earley-boyer, but so long as ION_DISABLED_SCRIPT exists, this should too.
Comment 4 David Anderson [:dvander] 2012-03-29 17:17:59 PDT
drive-by nit: canCompile should be something like "canIonCompile"
Comment 5 Nicolas B. Pierron [:nbp] 2012-03-29 18:08:00 PDT
https://hg.mozilla.org/projects/ionmonkey/rev/5824c381df95

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