Title: [218728] trunk
- Revision
- 218728
- Author
- [email protected]
- Date
- 2017-06-22 16:34:05 -0700 (Thu, 22 Jun 2017)
Log Message
ValueRep(DoubleRep(@v)) can not simply convert to @v
https://bugs.webkit.org/show_bug.cgi?id=173687
<rdar://problem/32855563>
Reviewed by Mark Lam.
JSTests:
* stress/dont-strength-reduce-valuerep-of-doublerep.js: Added.
(i.catch):
Source/_javascript_Core:
Consider this IR:
block#x
p: Phi() // int32 and double flows into this phi from various control flow
d: DoubleRep(@p)
some uses of @d here
v: ValueRep(DoubleRepUse:@d)
a: NewArrayWithSize(Int32:@v)
some more nodes here ...
Because the flow of ValueRep(DoubleRep(@p)) will not produce an Int32,
AI proves that the Int32 check will fail. Constant folding phase removes
all nodes after @a and inserts an Unreachable after the NewArrayWithSize node.
The IR then looks like this:
block#x
p: Phi() // int32 and double flows into this phi from various control flow
d: DoubleRep(@p)
some uses of @d here
v: ValueRep(DoubleRepUse:@d)
a: NewArrayWithSize(Int32:@v)
Unreachable
However, there was a strength reduction rule that tries eliminate redundant
conversions. It used to convert the program to:
block#x
p: Phi() // int32 and double flows into this phi from various control flow
d: DoubleRep(@p)
some uses of @d here
a: NewArrayWithSize(Int32:@p)
Unreachable
However, at runtime, @p will actually be an Int32, so @a will not OSR exit,
and we'll crash. This patch removes this strength reduction rule since it
does not maintain what would have happened if we executed the program before
the rule.
This rule is also wrong for other types of programs (I'm not sure we'd
actually emit this code, but if such IR were generated, we would previously
optimize it incorrectly):
@a: Constant(JSTrue)
@b: DoubleRep(@a)
@c: ValueRep(@b)
@d: use(@c)
However, the strength reduction rule would've transformed this into:
@a: Constant(JSTrue)
@d: use(@a)
And this would be wrong because node @c before the transformation would
have produced the JSValue jsNumber(1.0).
This patch was neutral in the benchmark run I did.
* dfg/DFGStrengthReductionPhase.cpp:
(JSC::DFG::StrengthReductionPhase::handleNode):
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (218727 => 218728)
--- trunk/JSTests/ChangeLog 2017-06-22 23:32:29 UTC (rev 218727)
+++ trunk/JSTests/ChangeLog 2017-06-22 23:34:05 UTC (rev 218728)
@@ -1,3 +1,14 @@
+2017-06-22 Saam Barati <[email protected]>
+
+ ValueRep(DoubleRep(@v)) can not simply convert to @v
+ https://bugs.webkit.org/show_bug.cgi?id=173687
+ <rdar://problem/32855563>
+
+ Reviewed by Mark Lam.
+
+ * stress/dont-strength-reduce-valuerep-of-doublerep.js: Added.
+ (i.catch):
+
2017-06-22 Yusuke Suzuki <[email protected]>
[JSC] Object.values should be implemented in C++
Added: trunk/JSTests/stress/dont-strength-reduce-valuerep-of-doublerep.js (0 => 218728)
--- trunk/JSTests/stress/dont-strength-reduce-valuerep-of-doublerep.js (rev 0)
+++ trunk/JSTests/stress/dont-strength-reduce-valuerep-of-doublerep.js 2017-06-22 23:34:05 UTC (rev 218728)
@@ -0,0 +1,15 @@
+let a2 = [];
+let thingy = {length: 2**55, __proto__: []};
+let func = (x) => x;
+
+noInline(Array.prototype.map);
+
+// This test should not crash.
+for (let i = 0; i < 100000; ++i) {
+ try {
+ if (i > 0 && (i % 1000) === 0)
+ thingy.map(func)
+ a2.map(func);
+ } catch(e) {
+ }
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (218727 => 218728)
--- trunk/Source/_javascript_Core/ChangeLog 2017-06-22 23:32:29 UTC (rev 218727)
+++ trunk/Source/_javascript_Core/ChangeLog 2017-06-22 23:34:05 UTC (rev 218728)
@@ -1,3 +1,67 @@
+2017-06-22 Saam Barati <[email protected]>
+
+ ValueRep(DoubleRep(@v)) can not simply convert to @v
+ https://bugs.webkit.org/show_bug.cgi?id=173687
+ <rdar://problem/32855563>
+
+ Reviewed by Mark Lam.
+
+ Consider this IR:
+ block#x
+ p: Phi() // int32 and double flows into this phi from various control flow
+ d: DoubleRep(@p)
+ some uses of @d here
+ v: ValueRep(DoubleRepUse:@d)
+ a: NewArrayWithSize(Int32:@v)
+ some more nodes here ...
+
+ Because the flow of ValueRep(DoubleRep(@p)) will not produce an Int32,
+ AI proves that the Int32 check will fail. Constant folding phase removes
+ all nodes after @a and inserts an Unreachable after the NewArrayWithSize node.
+
+ The IR then looks like this:
+ block#x
+ p: Phi() // int32 and double flows into this phi from various control flow
+ d: DoubleRep(@p)
+ some uses of @d here
+ v: ValueRep(DoubleRepUse:@d)
+ a: NewArrayWithSize(Int32:@v)
+ Unreachable
+
+ However, there was a strength reduction rule that tries eliminate redundant
+ conversions. It used to convert the program to:
+ block#x
+ p: Phi() // int32 and double flows into this phi from various control flow
+ d: DoubleRep(@p)
+ some uses of @d here
+ a: NewArrayWithSize(Int32:@p)
+ Unreachable
+
+ However, at runtime, @p will actually be an Int32, so @a will not OSR exit,
+ and we'll crash. This patch removes this strength reduction rule since it
+ does not maintain what would have happened if we executed the program before
+ the rule.
+
+ This rule is also wrong for other types of programs (I'm not sure we'd
+ actually emit this code, but if such IR were generated, we would previously
+ optimize it incorrectly):
+ @a: Constant(JSTrue)
+ @b: DoubleRep(@a)
+ @c: ValueRep(@b)
+ @d: use(@c)
+
+ However, the strength reduction rule would've transformed this into:
+ @a: Constant(JSTrue)
+ @d: use(@a)
+
+ And this would be wrong because node @c before the transformation would
+ have produced the JSValue jsNumber(1.0).
+
+ This patch was neutral in the benchmark run I did.
+
+ * dfg/DFGStrengthReductionPhase.cpp:
+ (JSC::DFG::StrengthReductionPhase::handleNode):
+
2017-06-22 JF Bastien <[email protected]>
ARM64: doubled executable memory limit from 32MiB to 64MiB
Modified: trunk/Source/_javascript_Core/dfg/DFGStrengthReductionPhase.cpp (218727 => 218728)
--- trunk/Source/_javascript_Core/dfg/DFGStrengthReductionPhase.cpp 2017-06-22 23:32:29 UTC (rev 218727)
+++ trunk/Source/_javascript_Core/dfg/DFGStrengthReductionPhase.cpp 2017-06-22 23:34:05 UTC (rev 218728)
@@ -215,11 +215,8 @@
break;
case ValueRep:
- case Int52Rep:
- case DoubleRep: {
- // This short-circuits circuitous conversions, like ValueRep(DoubleRep(value)) or
- // even more complicated things. Like, it can handle a beast like
- // ValueRep(DoubleRep(Int52Rep(value))).
+ case Int52Rep: {
+ // This short-circuits circuitous conversions, like ValueRep(Int52Rep(value)).
// The only speculation that we would do beyond validating that we have a type that
// can be represented a certain way is an Int32 check that would appear on Int52Rep
@@ -258,7 +255,6 @@
hadInt32Check = true;
continue;
- case DoubleRep:
case ValueRep:
continue;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
