Inline polymorphic [[Construct]] calls to native functions again

RESOLVED FIXED in Firefox 66

Status

()

enhancement
RESOLVED FIXED
4 months ago
4 months ago

People

(Reporter: anba, Assigned: anba)

Tracking

Trunk
mozilla66
Points:
---

Firefox Tracking Flags

(firefox66 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

4 months ago

Inlining polymorphic [[Construct]] calls to inline functions broke in bug 1472132, because the NewTarget of the copied CallInfo wasn't adjusted to hold the currently tested function.

Test case:

function f() {
    new Int32Array(8).slice(0);
    var ta = new Uint32Array(8);
    var r = 0
    var t = dateNow()
    for (var i = 0; i < 5000000; ++i){
        r += ta.slice(0).length;
    }
    return [dateNow() - t, r];
}
for (var i = 0; i < 5; ++i) print(f());

Before patch:

  • First iteration: ~740ms
  • Subsequent iterations: ~2600ms

With patch:

  • All iterations: ~740ms
Assignee

Comment 1

4 months ago

Modifies the NewTarget of the cloned CallInfo to match the current function, similar to [1]. This ensures we don't reject inlining native functions at [2].

[1] https://searchfox.org/mozilla-central/rev/c43240cef5829b8a2dec118faff8a5e1fec6ae1b/js/src/jit/IonBuilder.cpp#4522-4525
[2] https://searchfox.org/mozilla-central/rev/c43240cef5829b8a2dec118faff8a5e1fec6ae1b/js/src/jit/MCallOptimize.cpp#71-76

Attachment #9036284 - Flags: review?(jdemooij)
Comment on attachment 9036284 [details] [diff] [review]
bug1519795.patch

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

Great catch, thanks!
Attachment #9036284 - Flags: review?(jdemooij) → review+

Comment 4

4 months ago

Pushed by csabou@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1d0d0fed3608
Adjust CallInfo::newTarget to allow inlining polymorphic construct-calls of native functions. r=jandem

Keywords: checkin-needed

Comment 5

4 months ago
bugherder
Status: ASSIGNED → RESOLVED
Last Resolved: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
You need to log in before you can comment on or make changes to this bug.