Title: [232134] trunk/Source/_javascript_Core
- Revision
- 232134
- Author
- [email protected]
- Date
- 2018-05-23 16:04:58 -0700 (Wed, 23 May 2018)
Log Message
InPlaceAbstractState should filter variables at the tail from a GetLocal by their flush format
https://bugs.webkit.org/show_bug.cgi?id=185923
Reviewed by Saam Barati.
Previously, we could confuse AI by overly broadening a type. This happens when a block in a
loop has a local mutated following a GetLocal but never SetLocaled to the stack. For example,
Block 1:
@1: GetLocal(loc42, FlushedInt32);
@2: PutStructure(Check: Cell: @1);
@3: Jump(Block 1);
Would cause us to claim that loc42 could be either an int32 or a some cell. However,
the type of an local cannot change without writing to it.
This fixes a crash in destructuring-rest-element.js
* dfg/DFGInPlaceAbstractState.cpp:
(JSC::DFG::InPlaceAbstractState::endBasicBlock):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (232133 => 232134)
--- trunk/Source/_javascript_Core/ChangeLog 2018-05-23 22:55:28 UTC (rev 232133)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-05-23 23:04:58 UTC (rev 232134)
@@ -1,3 +1,26 @@
+2018-05-23 Keith Miller <[email protected]>
+
+ InPlaceAbstractState should filter variables at the tail from a GetLocal by their flush format
+ https://bugs.webkit.org/show_bug.cgi?id=185923
+
+ Reviewed by Saam Barati.
+
+ Previously, we could confuse AI by overly broadening a type. This happens when a block in a
+ loop has a local mutated following a GetLocal but never SetLocaled to the stack. For example,
+
+ Block 1:
+ @1: GetLocal(loc42, FlushedInt32);
+ @2: PutStructure(Check: Cell: @1);
+ @3: Jump(Block 1);
+
+ Would cause us to claim that loc42 could be either an int32 or a some cell. However,
+ the type of an local cannot change without writing to it.
+
+ This fixes a crash in destructuring-rest-element.js
+
+ * dfg/DFGInPlaceAbstractState.cpp:
+ (JSC::DFG::InPlaceAbstractState::endBasicBlock):
+
2018-05-23 Filip Pizlo <[email protected]>
Speed up JetStream/base64
Modified: trunk/Source/_javascript_Core/dfg/DFGInPlaceAbstractState.cpp (232133 => 232134)
--- trunk/Source/_javascript_Core/dfg/DFGInPlaceAbstractState.cpp 2018-05-23 22:55:28 UTC (rev 232133)
+++ trunk/Source/_javascript_Core/dfg/DFGInPlaceAbstractState.cpp 2018-05-23 23:04:58 UTC (rev 232134)
@@ -236,21 +236,39 @@
case Phi:
case SetArgument:
case PhantomLocal:
- case Flush:
+ case Flush: {
// The block transfers the value from head to tail.
destination = variableAt(index);
break;
+ }
- case GetLocal:
+ case GetLocal: {
// The block refines the value with additional speculations.
destination = forNode(node);
+
+ // We need to make sure that we don't broaden the type beyond what the flush
+ // format says it will be. The value may claim to have changed abstract state
+ // but it's type cannot change without a store. For example:
+ //
+ // Block #1:
+ // 0: GetLocal(loc42, FlushFormatInt32)
+ // 1: PutStructure(Check: Cell: @0, ArrayStructure)
+ // ...
+ // 2: Branch(T: #1, F: #2)
+ //
+ // In this case the AbstractState of @0 will say it's an SpecArray but the only
+ // reason that would have happened is because we would have exited the cell check.
+
+ FlushFormat flushFormat = node->variableAccessData()->flushFormat();
+ destination.filter(typeFilterFor(flushFormat));
break;
-
- case SetLocal:
+ }
+ case SetLocal: {
// The block sets the variable, and potentially refines it, both
// before and after setting it.
destination = forNode(node->child1());
break;
+ }
default:
RELEASE_ASSERT_NOT_REACHED();
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
