IonMonkey: Check for disabled script before inlining.

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: nbp, Assigned: nbp)

Tracking

(Blocks: 1 bug, {perf})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
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.
Attachment #610716 - Flags: review?(sstangl)
(Assignee)

Comment 1

5 years ago
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.
Attachment #610716 - Flags: review?(sstangl)
(Assignee)

Comment 2

5 years ago
Created attachment 610724 [details] [diff] [review]
Do not inline uncompilable functions

Fix previously buggy patch.
Attachment #610716 - Attachment is obsolete: true
Attachment #610724 - Flags: review?(sstangl)
(Assignee)

Updated

5 years ago
Assignee: general → nicolas.b.pierron
Status: NEW → ASSIGNED
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.
Attachment #610724 - Flags: review?(sstangl) → review+
drive-by nit: canCompile should be something like "canIonCompile"
(Assignee)

Comment 5

5 years ago
https://hg.mozilla.org/projects/ionmonkey/rev/5824c381df95
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.