Closed
Bug 1782771
Opened 2 years ago
Closed 2 years ago
Fold some MMinMax operations in String.prototype.{substring, slice, substr}
Categories
(Core :: JavaScript Engine: JIT, enhancement)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
105 Branch
| Tracking | Status | |
|---|---|---|
| firefox105 | --- | fixed |
People
(Reporter: anba, Assigned: anba)
References
Details
Attachments
(6 files)
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review | |
|
48 bytes,
text/x-phabricator-request
|
Details | Review |
No description provided.
|
Assignee | |
Comment 1•2 years ago
|
||
The change removes a MMinMax in s.substring(0, 3):
10 constant 0x0
11 constant 0x3
31 minmax constant10:Int32 stringlength28:Int32
32 minmax constant11:Int32 stringlength28:Int32
33 compare minmax31:Int32 minmax32:Int32 Lt
34 test compare33:Bool block9 block10
37 phi minmax32:Int32 minmax31:Int32
38 phi minmax31:Int32 minmax32:Int32
39 sub phi38:Int32 phi37:Int32 [int32]
40 substr tostring27:String phi37:Int32 sub39:Int32
After:
10 constant 0x0
11 constant 0x3
31 minmax constant11:Int32 stringlength28:Int32
32 compare constant10:Int32 minmax31:Int32 Lt
33 test compare32:Bool block9 block10
36 phi minmax31:Int32 constant10:Int32
37 phi constant10:Int32 minmax31:Int32
38 sub phi37:Int32 phi36:Int32 [int32]
39 substr tostring27:String phi36:Int32 sub38:Int32
More improvements for s.slice(0, 3):
10 constant 0x0
11 constant 0x3
32 minmax constant10:Int32 stringlength28:Int32
35 minmax constant11:Int32 stringlength28:Int32
37 sub minmax35:Int32 minmax32:Int32 [int32]
38 minmax sub37:Int32 constant10:Int32
39 substr tostring27:String minmax32:Int32 minmax38:Int32
After:
10 constant 0x0
11 constant 0x3
34 minmax constant11:Int32 stringlength28:Int32
36 minmax minmax34:Int32 constant10:Int32
37 substr tostring27:String constant10:Int32 minmax36:Int32
|
|
Assignee | |
Comment 2•2 years ago
|
||
Improves s.slice(0, 3) from:
10 constant 0x0
11 constant 0x3
34 minmax constant11:Int32 stringlength28:Int32
36 minmax minmax34:Int32 constant10:Int32
37 substr tostring27:String constant10:Int32 minmax36:Int32
to:
10 constant 0x0
11 constant 0x3
34 minmax constant11:Int32 stringlength28:Int32
36 substr tostring27:String constant10:Int32 minmax34:Int32
Depends on D153489
|
|
Assignee | |
Comment 3•2 years ago
|
||
Improves s.substring(0, 3) from:
10 constant 0x0
11 constant 0x3
31 minmax constant11:Int32 stringlength28:Int32
32 compare constant10:Int32 minmax31:Int32 Lt
33 test compare32:Bool block9 block10
36 phi minmax31:Int32 constant10:Int32
37 phi constant10:Int32 minmax31:Int32
38 sub phi37:Int32 phi36:Int32 [int32]
39 substr tostring27:String phi36:Int32 sub38:Int32
to:
10 constant 0x0
11 constant 0x3
31 minmax constant11:Int32 stringlength28:Int32
32 substr tostring27:String constant10:Int32 minmax31:Int32
Depends on D153490
|
|
Assignee | |
Comment 4•2 years ago
|
||
Improves s.substr(0, 3) from:
10 constant 0x0
11 constant 0x3
32 minmax constant11:Int32 stringlength28:Int32
33 compare minmax32:Int32 constant10:Int32 Le
34 test compare33:Bool block10 block11
35 constant string 3df1f0729800
37 substr tostring27:String constant10:Int32 minmax32:Int32
39 phi constant35:String substr37:String
to:
10 constant 0x0
11 constant 0x3
32 minmax constant11:Int32 stringlength28:Int32
33 substr tostring27:String constant10:Int32 minmax32:Int32
Depends on D153491
|
|
Assignee | |
Comment 5•2 years ago
|
||
Improves s.substring(1) from:
10 constant 0x1
29 minmax constant10:Int32 stringlength27:Int32
30 minmax minmax29:Int32 stringlength27:Int32
31 minmax minmax29:Int32 stringlength27:Int32
32 sub minmax31:Int32 minmax30:Int32 [int32]
33 substr tostring26:String minmax30:Int32 sub32:Int32
to:
10 constant 0x1
29 minmax constant10:Int32 stringlength27:Int32
30 sub stringlength27:Int32 minmax29:Int32 [int32]
31 substr tostring26:String minmax29:Int32 sub30:Int32
Depends on D153492
Pushed by andre.bargull@gmail.com: https://hg.mozilla.org/integration/autoland/rev/939921da7183 Part 1: Handle string length when folding MMinMax. r=jandem https://hg.mozilla.org/integration/autoland/rev/f1dd237d41ef Part 2: Fold min(x, min(y, z)) to min(c, z) with compile-time constant c=min(x, y). r=jandem https://hg.mozilla.org/integration/autoland/rev/9b75089ec05e Part 3: Use Math.min/max in String.prototype.substring. r=jandem https://hg.mozilla.org/integration/autoland/rev/7dfac88ea495 Part 4: Avoid phi nodes in String.prototype.substr. r=jandem https://hg.mozilla.org/integration/autoland/rev/52d2623e6704 Part 5: Fold min(x, min(x, y)) and max(x, max(x, y)). r=jandem
|
||
Comment 7•2 years ago
|
||
Backed out 5 changesets (bug 1782771) for causing assertion failures in js/src/builtin/String.cpp
Backout link: https://hg.mozilla.org/integration/autoland/rev/7e7bca63bd09fe0d2f246d09293b2fb42b275569
Flags: needinfo?(andrebargull)
|
|
Assignee | |
Comment 8•2 years ago
|
||
Yep, part 4 is incorrect. The case which triggered the assertion isn't covered in test262, so I started writing a test262 test for it. While doing that I found out that the spec is incorrect -> https://github.com/tc39/ecma262/pull/2844. :-)
Flags: needinfo?(andrebargull)
|
|
Assignee | |
Comment 9•2 years ago
|
||
This matches how SubstringKernel works when it calls NewDependentString,
and is also needed to avoid a differential testing issue in
"heap-analysis/byteSize-of-string.js".
Drive-by change:
Remove curly braces which are no longer useful to structure the code, now that
clang-format moved everything into a single line.
|
||
Comment 10•2 years ago
|
||
Pushed by andre.bargull@gmail.com: https://hg.mozilla.org/integration/autoland/rev/fe8deaa22397 Part 1: Handle string length when folding MMinMax. r=jandem https://hg.mozilla.org/integration/autoland/rev/7dc875ba64bb Part 2: Fold min(x, min(y, z)) to min(c, z) with compile-time constant c=min(x, y). r=jandem https://hg.mozilla.org/integration/autoland/rev/f13ddc37f83f Part 3: Use Math.min/max in String.prototype.substring. r=jandem https://hg.mozilla.org/integration/autoland/rev/8b53f7a9dea2 Part 4: Avoid phi nodes in String.prototype.substr. r=jandem https://hg.mozilla.org/integration/autoland/rev/bb791e77a175 Part 5: Fold min(x, min(x, y)) and max(x, max(x, y)). r=jandem https://hg.mozilla.org/integration/autoland/rev/adb23405ab19 Part 6: Return input when requesting substring from 0 to string length. r=jandem
|
||
Comment 11•2 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/fe8deaa22397
https://hg.mozilla.org/mozilla-central/rev/7dc875ba64bb
https://hg.mozilla.org/mozilla-central/rev/f13ddc37f83f
https://hg.mozilla.org/mozilla-central/rev/8b53f7a9dea2
https://hg.mozilla.org/mozilla-central/rev/bb791e77a175
https://hg.mozilla.org/mozilla-central/rev/adb23405ab19
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 105 Branch
You need to log in
before you can comment on or make changes to this bug.
Description
•