Closed Bug 1241224 Opened 4 years ago Closed 4 years ago

Improve ranges at loop backedges


(Core :: JavaScript Engine: JIT, defect)

Not set



Tracking Status
firefox47 --- fixed


(Reporter: h4writer, Assigned: h4writer)



(1 file, 1 obsolete file)

A long time ago I saw that we have some non-specific ranges on MPhi's in a loop. Now I finally found out why. For memory optimization we don't compute the Range of a MPhi if one of ranges of the operands is unknown.

When still doing this for phi's that have a backedge we get:

I[-2147483648 {#22}, 2147483647 {[loop] +999999999}] : Int32

Instead of the less precise:

F[? {#22}, ? {[loop] +999999999}] (U NaN U -Infinity U Infinity U -0) : Int32
Attached patch Patch (obsolete) — Splinter Review
Assignee: nobody → hv1989
Attachment #8710058 - Flags: review?(nicolas.b.pierron)
I am not sure to see where is the issue, would you have a test case where this matter?

From what I understand, the MPhi still has an Int32 type, and even if we do not attach a Range information, the type is still used to filter out ranges which cannot be represented, see Range::Range(const MDefinition*) [1].

Flags: needinfo?(hv1989)
> function test() {
>   for (var i=0; i<1000000000; i++) { }
> }
> test();
Is the reason why this is not needed in most cases. But for an MPhi that has a backedge this makes a difference. That is why I see the types mentioned in comment 0.
Flags: needinfo?(hv1989)
Like you said on irc, we are using the phi->range() somewhere. Here for the symbolic range analysis. We need a 'range()' that we can alter during the analysis, but we just take an empty range, instead of the output range. That is the reason we see the less precise range.
Attachment #8710058 - Attachment is obsolete: true
Attachment #8710058 - Flags: review?(nicolas.b.pierron)
Attachment #8710989 - Flags: review?(nicolas.b.pierron)
Comment on attachment 8710989 [details] [diff] [review]

Review of attachment 8710989 [details] [diff] [review]:

Good catch!
Attachment #8710989 - Flags: review?(nicolas.b.pierron) → review+
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.