Closed Bug 848733 Opened 8 years ago Closed 8 years ago

IonMonkey: Don't eliminate callee phi when inlining natives

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla22
Tracking Status
firefox19 --- unaffected
firefox20 + fixed
firefox21 + fixed
firefox22 + fixed

People

(Reporter: jandem, Assigned: jandem)

References

Details

(Keywords: regression)

Attachments

(2 files)

Noticed this while looking at some benchmarks. The following testcase throws a "TypeError: round is not a function" on m-c:

var a = [1];
function f(x) {
    var round = Math.round;
    for (var i=0; i<20; i++) {
	a[0] = round(a[0]);
	if (x > 500)
	    a[0] = "a";
    }
}
for (var i=0; i<550; i++)
    f(i);
Attached patch PatchSplinter Review
Sets the Folded flag on the MIR instruction after inlining a native call.
Attachment #722180 - Flags: review?(bhackett1024)
Noticed this on a raytracer, so this breaks real-world code. Updating flags.
Attachment #722180 - Flags: review?(bhackett1024) → review+
https://hg.mozilla.org/mozilla-central/rev/814a0c94b215
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Comment on attachment 722180 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 818869
User impact if declined: Correctness issues, websites not working
Testing completed (on m-c, etc.): On m-c
Risk to taking this patch (and alternatives if risky): Very low
String or UUID changes made by this patch: None
Attachment #722180 - Flags: approval-mozilla-beta?
Attachment #722180 - Flags: approval-mozilla-aurora?
Keywords: regression
Comment on attachment 722180 [details] [diff] [review]
Patch

low risk uplift of a Fx20 regression which helps avoid broken websites.Approving for uplift
Attachment #722180 - Flags: approval-mozilla-beta?
Attachment #722180 - Flags: approval-mozilla-beta+
Attachment #722180 - Flags: approval-mozilla-aurora?
Attachment #722180 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/9cd73a76c112

This doesn't apply cleanly to beta. Please post a branch-specific patch.
Attached patch Patch for betaSplinter Review
Beta does not have callInfo.
Attachment #723398 - Flags: review?(bhackett1024)
Duplicate of this bug: 847430
Comment on attachment 723398 [details] [diff] [review]
Patch for beta

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

::: js/src/jit-test/tests/ion/bug848733.js
@@ +7,5 @@
> +	    a[0] = "a";
> +    }
> +}
> +for (var i=0; i<550; i++)
> +    f(i);

I don't think the new test is needed on beta.
Attachment #723398 - Flags: review?(bhackett1024) → review+
You need to log in before you can comment on or make changes to this bug.