Closed
Bug 1241224
Opened 8 years ago
Closed 8 years ago
Improve ranges at loop backedges
Categories
(Core :: JavaScript Engine: JIT, defect)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla46
Tracking | Status | |
---|---|---|
firefox47 | --- | fixed |
People
(Reporter: h4writer, Assigned: h4writer)
Details
Attachments
(1 file, 1 obsolete file)
952 bytes,
patch
|
nbp
:
review+
|
Details | Diff | Splinter Review |
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
Assignee | ||
Comment 1•8 years ago
|
||
Assignee: nobody → hv1989
Attachment #8710058 -
Flags: review?(nicolas.b.pierron)
Comment 2•8 years ago
|
||
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]. [1] https://dxr.mozilla.org/mozilla-central/source/js/src/jit/RangeAnalysis.cpp#616
Flags: needinfo?(hv1989)
Assignee | ||
Comment 3•8 years ago
|
||
> function test() { > for (var i=0; i<1000000000; i++) { } > } > > test(); https://dxr.mozilla.org/mozilla-central/source/js/src/jit/RangeAnalysis.cpp#616 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)
Assignee | ||
Comment 4•8 years ago
|
||
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 5•8 years ago
|
||
Comment on attachment 8710989 [details] [diff] [review] bug1241224-ranges-at-phi Review of attachment 8710989 [details] [diff] [review]: ----------------------------------------------------------------- Good catch!
Attachment #8710989 -
Flags: review?(nicolas.b.pierron) → review+
Comment 7•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/7c02451fb069
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox47:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in
before you can comment on or make changes to this bug.
Description
•