Skip to content
  • fpizlo@apple.com's avatar
    DFG PhantomArguments shouldn't rely on a dead Phi graph · 14721941
    fpizlo@apple.com authored
    https://bugs.webkit.org/show_bug.cgi?id=126218
    
    Source/JavaScriptCore: 
    
    Reviewed by Oliver Hunt.
            
    This change dramatically rationalizes our handling of PhantomArguments (i.e.
    speculative elision of arguments object allocation).
            
    It's now the case that if we decide that we can elide arguments allocation, we just
    turn the arguments-creating node into a PhantomArguments and mark all locals that
    it's stored to as being arguments aliases. Being an arguments alias and being a
    PhantomArguments means basically the same thing: in DFG execution you have the empty
    value, on OSR exit an arguments object is allocated in your place, and all operations
    that use the value now just refer directly to the actual arguments in the call frame
    header (or the arguments we know that we passed to the call, in case of inlining).
            
    This means that we no longer have arguments simplification creating a dead Phi graph
    that then has to be interpreted by the OSR exit logic. That sort of never made any
    sense.
            
    This means that PhantomArguments now has a clear story in SSA: basically SSA just
    gets rid of the "locals" but everything else is the same.
            
    Finally, this means that we can more easily get rid of forward exiting. As I was
    working on the code to get rid of forward exiting, I realized that I'd have to
    carefully preserve the special meanings of MovHint and SetLocal in the case of
    PhantomArguments. It was really bizarre: even the semantics of MovHint were tied to
    our specific treatment of PhantomArguments. After this change this is no longer the
    case.
            
    One of the really cool things about this change is that arguments reification now
    just becomes a special kind of FlushFormat. This further unifies things: it means
    that a MovHint(PhantomArguments) and a SetLocal(PhantomArguments) both have the same
    meaning, since both of them dictate that the way we recover the local on exit is by
    reifying arguments. Previously, the SetLocal(PhantomArguments) case needed some
    special handling to accomplish this.
            
    A downside of this approach is that we will now emit code to store the empty value
    into aliased arguments variables, and we will even emit code to load that empty value
    as well. As far as I can tell this doesn't cost anything, since PhantomArguments are
    most profitable in cases where it allows us to simplify control flow and kill the
    arguments locals entirely. Of course, this isn't an issue in SSA form since SSA form
    also eliminates the locals.
    
    * dfg/DFGArgumentsSimplificationPhase.cpp:
    (JSC::DFG::ArgumentsSimplificationPhase::run):
    (JSC::DFG::ArgumentsSimplificationPhase::detypeArgumentsReferencingPhantomChild):
    * dfg/DFGFlushFormat.cpp:
    (WTF::printInternal):
    * dfg/DFGFlushFormat.h:
    (JSC::DFG::resultFor):
    (JSC::DFG::useKindFor):
    (JSC::DFG::dataFormatFor):
    * dfg/DFGSpeculativeJIT.cpp:
    (JSC::DFG::SpeculativeJIT::compileCurrentBlock):
    * dfg/DFGSpeculativeJIT32_64.cpp:
    (JSC::DFG::SpeculativeJIT::compile):
    * dfg/DFGSpeculativeJIT64.cpp:
    (JSC::DFG::SpeculativeJIT::compile):
    * dfg/DFGValueSource.h:
    (JSC::DFG::ValueSource::ValueSource):
    (JSC::DFG::ValueSource::forFlushFormat):
    * dfg/DFGVariableAccessData.h:
    (JSC::DFG::VariableAccessData::flushFormat):
    * ftl/FTLLowerDFGToLLVM.cpp:
    (JSC::FTL::LowerDFGToLLVM::buildExitArguments):
    
    LayoutTests: 
    
    Reviewed by Oliver Hunt.
            
    Added a test for an obvious case that I don't think we had coverage for in
    microbenchmarks. Of course, this case was already covered by more complex tests.
    
    * js/regress/inline-arguments-aliased-access-expected.txt: Added.
    * js/regress/inline-arguments-aliased-access.html: Added.
    * js/regress/script-tests/inline-arguments-aliased-access.js: Added.
    (foo):
    (bar):
    
    
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@161072 268f45cc-cd09-0410-ab3c-d52691b4dbfc
    14721941