Title: [187513] trunk/Source/_javascript_Core
- Revision
- 187513
- Author
- [email protected]
- Date
- 2015-07-28 14:23:06 -0700 (Tue, 28 Jul 2015)
Log Message
DFG::ArgumentsEliminationPhase has a redundant check for inserting CheckInBounds when converting GetByVal to GetStack in the inline non-varargs case
https://bugs.webkit.org/show_bug.cgi?id=147373
Reviewed by Mark Lam.
The code was doing a check for "index >= inlineCallFrame->arguments.size() - 1" in code where
safeToGetStack is true and we aren't in varargs context, but in a non-varargs context,
safeToGetStack can only be true if "index < inlineCallFrame->arguments.size() - 1".
When converting a GetByVal to GetStack, there are three possibilities:
1) Impossible to convert. This can happen if the GetByVal is out-of-bounds of the things we
know to have stored to the stack. For example, if we inline a function that does
"arguments[42]" at a call that passes no arguments.
2) Possible to convert, but we cannot prove statically that the GetByVal was in bounds. This
can happen for "arguments[42]" with no inline call frame (since we don't know statically
how many arguments we will be passed) or in a varargs call frame.
3) Possible to convert, and we know statically that the GetByVal is in bounds. This can
happen for "arguments[42]" if we have an inline call frame, and it's not a varargs call
frame, and we know that the caller passed 42 or more arguments.
The way the phase handles this is it first determines that we're not in case (1). This is
called safeToGetStack. safeToGetStack is true if we have case (2) or (3). For inline call
frames that have no varargs, this means that safeToGetStack is true exactly when the GetByVal
is in-bounds (i.e. case (3)).
But the phase was again doing a check for whether the index is in-bounds for non-varargs
inline call frames even when safeToGetStack was true. That check is redundant and should be
eliminated, since it makes the code confusing.
* dfg/DFGArgumentsEliminationPhase.cpp:
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (187512 => 187513)
--- trunk/Source/_javascript_Core/ChangeLog 2015-07-28 21:19:28 UTC (rev 187512)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-07-28 21:23:06 UTC (rev 187513)
@@ -1,5 +1,41 @@
2015-07-28 Filip Pizlo <[email protected]>
+ DFG::ArgumentsEliminationPhase has a redundant check for inserting CheckInBounds when converting GetByVal to GetStack in the inline non-varargs case
+ https://bugs.webkit.org/show_bug.cgi?id=147373
+
+ Reviewed by Mark Lam.
+
+ The code was doing a check for "index >= inlineCallFrame->arguments.size() - 1" in code where
+ safeToGetStack is true and we aren't in varargs context, but in a non-varargs context,
+ safeToGetStack can only be true if "index < inlineCallFrame->arguments.size() - 1".
+
+ When converting a GetByVal to GetStack, there are three possibilities:
+
+ 1) Impossible to convert. This can happen if the GetByVal is out-of-bounds of the things we
+ know to have stored to the stack. For example, if we inline a function that does
+ "arguments[42]" at a call that passes no arguments.
+
+ 2) Possible to convert, but we cannot prove statically that the GetByVal was in bounds. This
+ can happen for "arguments[42]" with no inline call frame (since we don't know statically
+ how many arguments we will be passed) or in a varargs call frame.
+
+ 3) Possible to convert, and we know statically that the GetByVal is in bounds. This can
+ happen for "arguments[42]" if we have an inline call frame, and it's not a varargs call
+ frame, and we know that the caller passed 42 or more arguments.
+
+ The way the phase handles this is it first determines that we're not in case (1). This is
+ called safeToGetStack. safeToGetStack is true if we have case (2) or (3). For inline call
+ frames that have no varargs, this means that safeToGetStack is true exactly when the GetByVal
+ is in-bounds (i.e. case (3)).
+
+ But the phase was again doing a check for whether the index is in-bounds for non-varargs
+ inline call frames even when safeToGetStack was true. That check is redundant and should be
+ eliminated, since it makes the code confusing.
+
+ * dfg/DFGArgumentsEliminationPhase.cpp:
+
+2015-07-28 Filip Pizlo <[email protected]>
+
DFG::PutStackSinkingPhase should be more aggressive about its "no GetStack until put" rule
https://bugs.webkit.org/show_bug.cgi?id=147371
Modified: trunk/Source/_javascript_Core/dfg/DFGArgumentsEliminationPhase.cpp (187512 => 187513)
--- trunk/Source/_javascript_Core/dfg/DFGArgumentsEliminationPhase.cpp 2015-07-28 21:19:28 UTC (rev 187512)
+++ trunk/Source/_javascript_Core/dfg/DFGArgumentsEliminationPhase.cpp 2015-07-28 21:23:06 UTC (rev 187513)
@@ -452,8 +452,7 @@
arg += inlineCallFrame->stackOffset;
data = "" FlushedJSValue);
- if (!inlineCallFrame || inlineCallFrame->isVarargs()
- || index >= inlineCallFrame->arguments.size() - 1) {
+ if (!inlineCallFrame || inlineCallFrame->isVarargs()) {
insertionSet.insertNode(
nodeIndex, SpecNone, CheckInBounds, node->origin,
node->child2(), Edge(getArrayLength(candidate), Int32Use));
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
