Closed Bug 1072911 Opened 6 years ago Closed 6 years ago

Scalar Replacement: Add MIRType to MPhi & M*State definitions

Categories

(Core :: JavaScript Engine: JIT, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla35

People

(Reporter: nbp, Assigned: nbp)

References

Details

Attachments

(1 file)

See tests cases from bug 1069307 comment 1.
Comment on attachment 8495235 [details] [diff] [review]
Scalar Replacement: Add missing MIRTypes.

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

Note: I added the last 2 test cases in the patch as the first test case do an infinite loop and highlight the same assertion.
Comment on attachment 8495235 [details] [diff] [review]
Scalar Replacement: Add missing MIRTypes.

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

::: js/src/jit/ScalarReplacement.cpp
@@ +310,5 @@
>          // nodes.
>          succState = BlockState::Copy(alloc_, state_);
>          size_t numPreds = succ->numPredecessors();
>          for (size_t slot = 0; slot < state_->numSlots(); slot++) {
> +            MPhi *phi = MPhi::New(alloc_, MIRType_Value);

No need for this change. MPhi has a default resultType of MIRType_Value

@@ +764,5 @@
>          // nodes.
>          succState = BlockState::Copy(alloc_, state_);
>          size_t numPreds = succ->numPredecessors();
>          for (size_t index = 0; index < state_->numElements(); index++) {
> +            MPhi *phi = MPhi::New(alloc_, MIRType_Value);

No need for this change. MPhi has a default resultType of MIRType_Value
Attachment #8495235 - Flags: review?(hv1989) → review+
there appears to be a regression (19672 -> 17651) on Octane-Splay on machine 11 on AWFY. 
The regression range is this from AWFY : http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=846faaa68219&tochange=808aa539aad1
Flags: needinfo?(nicolas.b.pierron)
Hannes, I do not think this can cause any performance impact as the only modifications are on unused objects/arrays.  This issue is caused by the fix in Bug 1007213, and would be resolved by the optimization from Bug 1072188.
Flags: needinfo?(nicolas.b.pierron)
(In reply to Nicolas B. Pierron [:nbp] from comment #6)
> Hannes, I do not think this can cause any performance impact as the only
> modifications are on unused objects/arrays.  This issue is caused by the fix
> in Bug 1007213, and would be resolved by the optimization from Bug 1072188.

Yeah that sound likely. Thanks. I'll keep an eye on bug 1072188. Let's hope it can land soonish so this regression is gone.
https://hg.mozilla.org/mozilla-central/rev/11a4a935eb7d
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Blocks: 1074833
No longer blocks: 1074833
You need to log in before you can comment on or make changes to this bug.