Changeset 219727 in webkit


Ignore:
Timestamp:
Jul 21, 2017, 9:01:01 AM (8 years ago)
Author:
Yusuke Suzuki
Message:

[FTL] Arguments elimination is suppressed by unreachable blocks
https://bugs.webkit.org/show_bug.cgi?id=174352

Reviewed by Filip Pizlo.

JSTests:

  • stress/arguments-elimination-force-exit.js: Added.

(shouldBe):
(strict):
(sloppy):

  • stress/arguments-elimination-throw.js: Added.

(shouldBe):
(shouldThrow):
(sloppy):
(isArguments):

Source/JavaScriptCore:

If we do not execute op_get_by_id, our value profiling tells us unpredictable and DFG emits ForceOSRExit.
The problem is that arguments elimination phase checks escaping even when ForceOSRExit preceeds.
Since GetById without information can escape arguments if it is specified, non-executed code including
op_get_by_id with arguments can escape arguments.

For example,

function test(flag)
{

if (flag) {

This is not executed, but emits GetById with arguments.
It prevents us from eliminating materialization.
return arguments.length;

}
return arguments.length;

}
noInline(test);
while (true)

test(false);

We do not perform CFA and dead-node clipping yet when performing arguments elimination phase.
So this GetById exists and escapes arguments.

To solve this problem, our arguments elimination phase checks preceding pseudo-terminal nodes.
If it is shown, following GetById does not escape arguments. Compared to performing AI, it is
lightweight. But it catches much of typical cases we failed to perform arguments elimination.

  • dfg/DFGArgumentsEliminationPhase.cpp:
  • dfg/DFGNode.h:

(JSC::DFG::Node::isPseudoTerminal):

  • dfg/DFGValidate.cpp:
Location:
trunk
Files:
2 added
5 edited

Legend:

Unmodified
Added
Removed
  • TabularUnified trunk/JSTests/ChangeLog

    r219498 r219727  
     12017-07-21  Yusuke Suzuki  <utatane.tea@gmail.com>
     2
     3        [FTL] Arguments elimination is suppressed by unreachable blocks
     4        https://bugs.webkit.org/show_bug.cgi?id=174352
     5
     6        Reviewed by Filip Pizlo.
     7
     8        * stress/arguments-elimination-force-exit.js: Added.
     9        (shouldBe):
     10        (strict):
     11        (sloppy):
     12        * stress/arguments-elimination-throw.js: Added.
     13        (shouldBe):
     14        (shouldThrow):
     15        (sloppy):
     16        (isArguments):
     17
    1182017-07-13  Mark Lam  <mark.lam@apple.com>
    219
  • TabularUnified trunk/Source/JavaScriptCore/ChangeLog

    r219702 r219727  
     12017-07-21  Yusuke Suzuki  <utatane.tea@gmail.com>
     2
     3        [FTL] Arguments elimination is suppressed by unreachable blocks
     4        https://bugs.webkit.org/show_bug.cgi?id=174352
     5
     6        Reviewed by Filip Pizlo.
     7
     8        If we do not execute `op_get_by_id`, our value profiling tells us unpredictable and DFG emits ForceOSRExit.
     9        The problem is that arguments elimination phase checks escaping even when ForceOSRExit preceeds.
     10        Since GetById without information can escape arguments if it is specified, non-executed code including
     11        op_get_by_id with arguments can escape arguments.
     12
     13        For example,
     14
     15            function test(flag)
     16            {
     17                if (flag) {
     18                    // This is not executed, but emits GetById with arguments.
     19                    // It prevents us from eliminating materialization.
     20                    return arguments.length;
     21                }
     22                return arguments.length;
     23            }
     24            noInline(test);
     25            while (true)
     26                test(false);
     27
     28        We do not perform CFA and dead-node clipping yet when performing arguments elimination phase.
     29        So this GetById exists and escapes arguments.
     30
     31        To solve this problem, our arguments elimination phase checks preceding pseudo-terminal nodes.
     32        If it is shown, following GetById does not escape arguments. Compared to performing AI, it is
     33        lightweight. But it catches much of typical cases we failed to perform arguments elimination.
     34
     35        * dfg/DFGArgumentsEliminationPhase.cpp:
     36        * dfg/DFGNode.h:
     37        (JSC::DFG::Node::isPseudoTerminal):
     38        * dfg/DFGValidate.cpp:
     39
    1402017-07-20  Chris Dumez  <cdumez@apple.com>
    241
  • TabularUnified trunk/Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp

    r217016 r219727  
    410410                    break;
    411411                }
     412                if (node->isPseudoTerminal())
     413                    break;
    412414            }
    413415        }
     
    11021104                    break;
    11031105                }
     1106                if (node->isPseudoTerminal())
     1107                    break;
    11041108            }
    11051109           
  • TabularUnified trunk/Source/JavaScriptCore/dfg/DFGNode.h

    r218794 r219727  
    13061306
    13071307        return false;
     1308    }
     1309
     1310    // As is described in DFGNodeType.h's ForceOSRExit, this is a pseudo-terminal.
     1311    // It means that execution should fall out of DFG at this point, but execution
     1312    // does continue in the basic block - just in a different compiler.
     1313    // FIXME: This is used for lightweight reachability decision. But this should
     1314    // be replaced with AI-based reachability ideally.
     1315    bool isPseudoTerminal()
     1316    {
     1317        switch (op()) {
     1318        case ForceOSRExit:
     1319        case CheckBadCell:
     1320        case Throw:
     1321        case ThrowStaticError:
     1322            return true;
     1323        default:
     1324            return false;
     1325        }
    13081326    }
    13091327
  • TabularUnified trunk/Source/JavaScriptCore/dfg/DFGValidate.cpp

    r217202 r219727  
    643643
    644644            bool didSeeExitOK = false;
     645            bool isOSRExited = false;
    645646           
    646647            for (auto* node : *block) {
     
    670671                    break;
    671672                }
     673
     674                if (isOSRExited)
     675                    continue;
    672676                switch (node->op()) {
    673677                case PhantomNewObject:
     
    739743                    break;
    740744                }
     745                isOSRExited |= node->isPseudoTerminal();
    741746            }
    742747        }
Note: See TracChangeset for help on using the changeset viewer.