[MERGE #5702 @yullin-ms] Argument length and argument constant optimization

yullin-ms · yullin-ms · commit 4b72289bf944 · 2018-09-20T14:36:11.000-07:00
Merge pull request #5702 from yullin-ms:argLenAndConstOpt Optimize LdLen_A argument length and LdElemI_A for the argument[constants]
diff --git a/lib/Backend/BackwardPass.cpp b/lib/Backend/BackwardPass.cpp
@@ -7842,10 +7842,12 @@ BackwardPass::ProcessInlineeEnd(IR::Instr* instr)
     }
     if (this->tag == Js::BackwardPhase)
     {
-        if (!GlobOpt::DoInlineArgsOpt(instr->m_func))
+        // Commenting out to allow for argument length and argument[constant] optimization
+        // Will revisit in phase two
+        /*if (!GlobOpt::DoInlineArgsOpt(instr->m_func))
         {
             return;
-        }
+        }*/
 
         // This adds a use for function sym as part of InlineeStart & all the syms referenced by the args.
         // It ensure they do not get cleared from the copy prop sym map.
diff --git a/lib/Backend/Func.cpp b/lib/Backend/Func.cpp
@@ -58,6 +58,7 @@ Func::Func(JitArenaAllocator *alloc, JITTimeWorkItem * workItem,
     m_bailoutReturnValueSym(nullptr),
     m_hasBailedOutSym(nullptr),
     m_inlineeFrameStartSym(nullptr),
+    inlineeStart(nullptr),
     m_regsUsed(0),
     m_fg(nullptr),
     m_labelCount(0),
@@ -92,6 +93,7 @@ Func::Func(JitArenaAllocator *alloc, JITTimeWorkItem * workItem,
     hasInlinee(false),
     thisOrParentInlinerHasArguments(false),
     hasStackArgs(false),
+    hasArgLenAndConstOpt(false),
     hasImplicitParamLoad(false),
     hasThrow(false),
     hasNonSimpleParams(false),
@@ -301,8 +303,10 @@ Func::Codegen(JitArenaAllocator *alloc, JITTimeWorkItem * workItem,
     Js::ScriptContextProfiler *const codeGenProfiler, const bool isBackgroundJIT)
 {
     bool rejit;
+    int rejitCounter = 0;
     do
     {
+        Assert(rejitCounter < 25);
         Func func(alloc, workItem, threadContextInfo,
             scriptContextInfo, outputData, epInfo, runtimeInfo,
             polymorphicInlineCacheInfo, codeGenAllocators, 
@@ -334,6 +338,8 @@ Func::Codegen(JitArenaAllocator *alloc, JITTimeWorkItem * workItem,
             case RejitReason::DisableStackArgOpt:
                 outputData->disableStackArgOpt = TRUE;
                 break;
+            case RejitReason::DisableStackArgLenAndConstOpt:
+                break;
             case RejitReason::DisableSwitchOptExpectingInteger:
             case RejitReason::DisableSwitchOptExpectingString:
                 outputData->disableSwitchOpt = TRUE;
@@ -360,6 +366,7 @@ Func::Codegen(JitArenaAllocator *alloc, JITTimeWorkItem * workItem,
             }
 
             rejit = true;
+            rejitCounter++;
         }
         // Either the entry point has a reference to the number now, or we failed to code gen and we
         // don't need to numbers, we can flush the completed page now.
diff --git a/lib/Backend/Func.h b/lib/Backend/Func.h
@@ -517,6 +517,17 @@ static const unsigned __int64 c_debugFillPattern8 = 0xcececececececece;
         m_inlineeFrameStartSym = sym;
     }
 
+    void SetInlineeStart(IR::Instr *inlineeStartInstr)
+    {
+        Assert(inlineeStart == nullptr);
+        inlineeStart = inlineeStartInstr;
+    }
+
+    IR::Instr* GetInlineeStart()
+    {
+        return inlineeStart;
+    }
+
     IR::SymOpnd *GetInlineeArgCountSlotOpnd()
     {
         return GetInlineeOpndAtOffset(Js::Constants::InlineeMetaArgIndex_Argc * MachPtr);
@@ -721,6 +732,7 @@ static const unsigned __int64 c_debugFillPattern8 = 0xcececececececece;
     bool                hasBailout: 1;
     bool                hasBailoutInEHRegion : 1;
     bool                hasStackArgs: 1;
+    bool                hasArgLenAndConstOpt : 1;
     bool                hasImplicitParamLoad : 1; // True if there is a load of CallInfo, FunctionObject
     bool                hasThrow : 1;
     bool                hasUnoptimizedArgumentsAccess : 1; // True if there are any arguments access beyond the simple case of this.apply pattern
@@ -1030,6 +1042,7 @@ static const unsigned __int64 c_debugFillPattern8 = 0xcececececececece;
     Func * const        topFunc;
     Func * const        parentFunc;
     StackSym *          m_inlineeFrameStartSym;
+    IR::Instr *         inlineeStart;
     uint                maxInlineeArgOutSize;
     const bool          m_isBackgroundJIT;
     bool                hasInstrNumber;
diff --git a/lib/Backend/GlobOpt.cpp b/lib/Backend/GlobOpt.cpp
@@ -175,7 +175,7 @@ GlobOpt::Optimize()
 
         // Still need to run the dead store phase to calculate the live reg on back edge
         this->BackwardPass(Js::DeadStorePhase);
-        CannotAllocateArgumentsObjectOnStack();
+        CannotAllocateArgumentsObjectOnStack(nullptr);
         return;
     }
 
@@ -887,7 +887,7 @@ GlobOpt::ToTypeSpec(BVSparse<JitArenaAllocator> *bv, BasicBlock *block, IRType t
         // instruction itself should disable arguments object optimization.
         if(block->globOptData.argObjSyms && block->globOptData.IsArgumentsSymID(id))
         {
-            CannotAllocateArgumentsObjectOnStack();
+            CannotAllocateArgumentsObjectOnStack(nullptr);
         }
 
         if (block->globOptData.liveVarSyms->Test(id))
@@ -1512,7 +1512,7 @@ GlobOpt::OptArguments(IR::Instr *instr)
 
         if (instr->m_func->GetJITFunctionBody()->GetInParamsCount() != 1 && !instr->m_func->IsStackArgsEnabled())
         {
-            CannotAllocateArgumentsObjectOnStack();
+            CannotAllocateArgumentsObjectOnStack(instr->m_func);
         }
         else
         {
@@ -1527,7 +1527,7 @@ GlobOpt::OptArguments(IR::Instr *instr)
         // In the debug mode, we don't want to optimize away the aliases. Since we may have to show them on the inspection.
         if (((!AreFromSameBytecodeFunc(src1->AsRegOpnd(), dst->AsRegOpnd()) || this->currentBlock->loop) && instr->m_opcode != Js::OpCode::BytecodeArgOutCapture) || this->func->IsJitInDebugMode())
         {
-            CannotAllocateArgumentsObjectOnStack();
+            CannotAllocateArgumentsObjectOnStack(instr->m_func);
             return;
         }
         if(!dst->AsRegOpnd()->GetStackSym()->m_nonEscapingArgObjAlias)
@@ -1550,7 +1550,7 @@ GlobOpt::OptArguments(IR::Instr *instr)
     }
 
     SymID id = 0;
-
+    
     switch(instr->m_opcode)
     {
     case Js::OpCode::LdElemI_A:
@@ -1561,7 +1561,7 @@ GlobOpt::OptArguments(IR::Instr *instr)
         if (indexOpnd && CurrentBlockData()->IsArgumentsSymID(indexOpnd->m_sym->m_id))
         {
             // Pathological test cases such as a[arguments]
-            CannotAllocateArgumentsObjectOnStack();
+            CannotAllocateArgumentsObjectOnStack(instr->m_func);
             return;
         }
 
@@ -1650,7 +1650,7 @@ GlobOpt::OptArguments(IR::Instr *instr)
                             WritePerfHint(PerfHints::HeapArgumentsCreated, instr->m_func, instr->GetByteCodeOffset());
                         }
 #endif
-                        CannotAllocateArgumentsObjectOnStack();
+                        CannotAllocateArgumentsObjectOnStack(instr->m_func);
                         return;
                     }
                 }
@@ -1668,7 +1668,7 @@ GlobOpt::OptArguments(IR::Instr *instr)
                             WritePerfHint(PerfHints::HeapArgumentsCreated, instr->m_func, instr->GetByteCodeOffset());
                         }
 #endif
-                        CannotAllocateArgumentsObjectOnStack();
+                        CannotAllocateArgumentsObjectOnStack(instr->m_func);
                         return;
                     }
                 }
@@ -1687,7 +1687,7 @@ GlobOpt::OptArguments(IR::Instr *instr)
                             WritePerfHint(PerfHints::HeapArgumentsModification, instr->m_func, instr->GetByteCodeOffset());
                         }
 #endif
-                        CannotAllocateArgumentsObjectOnStack();
+                        CannotAllocateArgumentsObjectOnStack(instr->m_func);
                         return;
                     }
                 }
@@ -1701,7 +1701,7 @@ GlobOpt::OptArguments(IR::Instr *instr)
                             WritePerfHint(PerfHints::HeapArgumentsModification, instr->m_func, instr->GetByteCodeOffset());
                         }
 #endif
-                        CannotAllocateArgumentsObjectOnStack();
+                        CannotAllocateArgumentsObjectOnStack(instr->m_func);
                         return;
                     }
                     CurrentBlockData()->ClearArgumentsSym(dst->AsRegOpnd());
@@ -2446,6 +2446,7 @@ GlobOpt::OptInstr(IR::Instr *&instr, bool* isInstrRemoved)
     OptimizeChecks(instr);
     OptArraySrc(&instr, &src1Val, &src2Val);
     OptNewScObject(&instr, src1Val);
+    OptArgLenAndConst(instr, &src1Val);
 
     instr = this->OptPeep(instr, src1Val, src2Val);
 
@@ -13196,6 +13197,69 @@ GlobOpt::OptArraySrc(IR::Instr ** const instrRef, Value ** src1Val, Value ** src
     arraySrcOpt.Optimize();
 }
 
+void
+GlobOpt::OptArgLenAndConst(IR::Instr* instr, Value** src1Val)
+{
+    if (instr->usesStackArgumentsObject && instr->IsInlined())
+    {
+        IR::Opnd* src1 = instr->GetSrc1();
+        auto replaceInstr = [&](IR::Opnd* newopnd)
+        {
+            this->CaptureByteCodeSymUses(instr);
+            instr->m_opcode = Js::OpCode::Ld_A;
+            instr->ReplaceSrc1(newopnd);
+            if (instr->HasBailOutInfo())
+            {
+                instr->ClearBailOutInfo();
+            }
+            *src1Val = this->OptSrc(instr->GetSrc1(), &instr);
+            instr->m_func->hasArgLenAndConstOpt = true;
+        };
+        Assert(CurrentBlockData()->IsArgumentsOpnd(src1));
+        switch(instr->m_opcode)
+        {
+            case Js::OpCode::LdLen_A:
+            {
+                IR::AddrOpnd* newopnd = IR::AddrOpnd::New(Js::TaggedInt::ToVarUnchecked(instr->m_func->actualCount - 1), IR::AddrOpndKindConstantVar, instr->m_func);
+                replaceInstr(newopnd);
+                break;
+            }
+    
+            case Js::OpCode::LdElemI_A:
+            {
+                IR::IndirOpnd* indirOpndSrc1 = src1->AsIndirOpnd();
+                if (!indirOpndSrc1->GetIndexOpnd())
+                {
+                    int argIndex = indirOpndSrc1->GetOffset() + 1;
+                    IR::Instr* defInstr = nullptr;
+                    IR::Instr* inlineeStart = instr->m_func->GetInlineeStart();
+                    inlineeStart->IterateArgInstrs([&](IR::Instr* argInstr) {
+                        StackSym *argSym = argInstr->GetDst()->AsSymOpnd()->m_sym->AsStackSym();
+                        if (argSym->GetArgSlotNum() - 1 == argIndex)
+                        {
+                            defInstr = argInstr;
+                            return true;
+                        }
+                        return false;
+                    });
+                    // If we cannot find the right instruction. I.E. When calling arguments[2] and no arguments were passed to the func
+                    if (defInstr == nullptr) 
+                    {
+                        IR::Opnd * undefined = IR::AddrOpnd::New(instr->m_func->GetScriptContextInfo()->GetUndefinedAddr(), IR::AddrOpndKindDynamicVar, instr->m_func, true);
+                        undefined->SetValueType(ValueType::Undefined);
+                        replaceInstr(undefined);
+                    }
+                    else
+                    {
+                        replaceInstr(defInstr->GetSrc1());
+                    }
+                }
+                break;
+            }
+        }
+    }
+}
+
 void
 GlobOpt::CaptureNoImplicitCallUses(
     IR::Opnd *opnd,
@@ -15722,16 +15786,23 @@ GlobOpt::TrackArgumentsObject()
 {
     if (PHASE_OFF(Js::StackArgOptPhase, this->func))
     {
-        this->CannotAllocateArgumentsObjectOnStack();
+        this->CannotAllocateArgumentsObjectOnStack(nullptr);
         return false;
     }
 
     return func->GetHasStackArgs();
 }
 
 void
-GlobOpt::CannotAllocateArgumentsObjectOnStack()
+GlobOpt::CannotAllocateArgumentsObjectOnStack(Func * curFunc)
 {
+    if (curFunc != nullptr && curFunc->hasArgLenAndConstOpt)
+    {
+        Assert(!curFunc->GetJITOutput()->GetOutputData()->disableStackArgOpt);
+        curFunc->GetJITOutput()->GetOutputData()->disableStackArgOpt = true;
+        throw Js::RejitException(RejitReason::DisableStackArgLenAndConstOpt);
+    }
+
     func->SetHasStackArgs(false);
 
 #ifdef ENABLE_DEBUG_CONFIG_OPTIONS
diff --git a/lib/Backend/GlobOpt.h b/lib/Backend/GlobOpt.h
@@ -690,6 +690,7 @@ class GlobOpt
     IR::Instr*              CreateBoundsCheckInstr(IR::Opnd* lowerBound, IR::Opnd* upperBound, int offset, IR::BailOutKind bailoutkind, BailOutInfo* bailoutInfo, Func* func);
     IR::Instr*              AttachBoundsCheckData(IR::Instr* instr, IR::Opnd* lowerBound, IR::Opnd* upperBound, int offset);
     void                    OptArraySrc(IR::Instr **const instrRef, Value ** src1Val, Value ** src2Val);
+    void                    OptArgLenAndConst(IR::Instr* instr, Value** src1Val);
 
 private:
     void                    TrackIntSpecializedAddSubConstant(IR::Instr *const instr, const AddSubConstantInfo *const addSubConstantInfo, Value *const dstValue, const bool updateSourceBounds);
@@ -913,7 +914,7 @@ class GlobOpt
     void                    UpdateObjPtrValueType(IR::Opnd * opnd, IR::Instr * instr);
 
     bool                    TrackArgumentsObject();
-    void                    CannotAllocateArgumentsObjectOnStack();
+    void                    CannotAllocateArgumentsObjectOnStack(Func * curFunc);
 
 #if DBG
     bool                    IsPropertySymId(SymID symId) const;
diff --git a/lib/Backend/GlobOptBailOut.cpp b/lib/Backend/GlobOptBailOut.cpp
@@ -848,7 +848,7 @@ void GlobOpt::EndTrackingOfArgObjSymsForInlinee()
             // This means there are arguments object symbols in the current function which are not in the current block.
             // This could happen when one of the blocks has a throw and arguments object aliased in it and other blocks don't see it.
             // Rare case, abort stack arguments optimization in this case.
-            CannotAllocateArgumentsObjectOnStack();
+            CannotAllocateArgumentsObjectOnStack(this->currentBlock->globOptData.curFunc);
         }
         else
         {
diff --git a/lib/Backend/GlobOptBlockData.cpp b/lib/Backend/GlobOptBlockData.cpp
@@ -660,7 +660,7 @@ GlobOptBlockData::MergeBlockData(
     {
         if (!this->argObjSyms->Equal(fromData->argObjSyms))
         {
-            this->globOpt->CannotAllocateArgumentsObjectOnStack();
+            this->globOpt->CannotAllocateArgumentsObjectOnStack(nullptr);
         }
     }
 
diff --git a/lib/Backend/Inline.cpp b/lib/Backend/Inline.cpp
@@ -1360,12 +1360,13 @@ void Inline::InsertOneInlinee(IR::Instr* callInstr, IR::RegOpnd* returnValueOpnd
         Js::ArgSlot actualCount = MapActuals(currentCallInstr, argOuts, Js::InlineeCallInfo::MaxInlineeArgoutCount, inlinee, (Js::ProfileId)callInstr->AsProfiledInstr()->u.profileId, &stackArgsArgOutExpanded);
         Assert(actualCount > 0);
         MapFormals(inlinee, argOuts, funcBody->GetInParamsCount(), actualCount, returnValueOpnd, currentCallInstr->GetSrc1(), symCallerThis, stackArgsArgOutExpanded, fixedFunctionSafeThis, argOuts);
+        inlinee->SetInlineeStart(currentCallInstr);
         currentCallInstr->m_func = inlinee;
 
         // Put the meta arguments that the stack walker expects to find on the stack.
         // As all the argouts are shared among the inlinees, do this only once.
         SetupInlineeFrame(inlinee, currentCallInstr, actualCount, currentCallInstr->GetSrc1());
-
+        
         IR::Instr* inlineeEndInstr = IR::Instr::New(Js::OpCode::InlineeEnd, inlinee);
         inlineeEndInstr->SetByteCodeOffset(inlinee->m_tailInstr->GetPrevRealInstr());
         inlineeEndInstr->SetSrc1(IR::IntConstOpnd::New(actualCount + Js::Constants::InlineeMetaArgCount, TyInt32, inlinee));
@@ -3967,6 +3968,7 @@ Inline::InlineFunctionCommon(IR::Instr *callInstr, bool originalCallTargetOpndIs
     callInstr->m_opcode = Js::OpCode::InlineeStart;
 
     // Set it to belong to the inlinee, so that we can use the actual count when lowering InlineeStart
+    inlinee->SetInlineeStart(callInstr);
     callInstr->m_func = inlinee;
     callInstr->SetDst(IR::RegOpnd::New(returnValueOpnd ? returnValueOpnd->GetType() : TyVar, inlinee));
     // Put the meta arguments that the stack walker expects to find on the stack.
diff --git a/lib/Common/Common/RejitReasons.h b/lib/Common/Common/RejitReasons.h
@@ -49,4 +49,5 @@ REJIT_REASON(ModByPowerOf2)
 REJIT_REASON(NoProfile)
 REJIT_REASON(PowIntIntTypeSpecDisabled)
 REJIT_REASON(DisableStackArgOpt)
+REJIT_REASON(DisableStackArgLenAndConstOpt)
 REJIT_REASON(OptimizeTryFinallyDisabled)

Original file line number	Diff line number	Diff line change
`@@ -7842,10 +7842,12 @@ BackwardPass::ProcessInlineeEnd(IR::Instr* instr)`
`7842`	`7842`	`}`
`7843`	`7843`	`if (this->tag == Js::BackwardPhase)`
`7844`	`7844`	`{`
`7845`		`- if (!GlobOpt::DoInlineArgsOpt(instr->m_func))`
	`7845`	`+ // Commenting out to allow for argument length and argument[constant] optimization`
	`7846`	`+ // Will revisit in phase two`
	`7847`	`+ /*if (!GlobOpt::DoInlineArgsOpt(instr->m_func))`
`7846`	`7848`	`{`
`7847`	`7849`	`return;`
`7848`		`- }`
	`7850`	`+ }*/`
`7849`	`7851`
`7850`	`7852`	`// This adds a use for function sym as part of InlineeStart & all the syms referenced by the args.`
`7851`	`7853`	`// It ensure they do not get cleared from the copy prop sym map.`
Original file line number	Diff line number	Diff line change
`@@ -848,7 +848,7 @@ void GlobOpt::EndTrackingOfArgObjSymsForInlinee()`
`848`	`848`	`// This means there are arguments object symbols in the current function which are not in the current block.`
`849`	`849`	`// This could happen when one of the blocks has a throw and arguments object aliased in it and other blocks don't see it.`
`850`	`850`	`// Rare case, abort stack arguments optimization in this case.`
`851`		`- CannotAllocateArgumentsObjectOnStack();`
	`851`	`+ CannotAllocateArgumentsObjectOnStack(this->currentBlock->globOptData.curFunc);`
`852`	`852`	`}`
`853`	`853`	`else`
`854`	`854`	`{`
Original file line number	Diff line number	Diff line change
`@@ -660,7 +660,7 @@ GlobOptBlockData::MergeBlockData(`
`660`	`660`	`{`
`661`	`661`	`if (!this->argObjSyms->Equal(fromData->argObjSyms))`
`662`	`662`	`{`
`663`		`- this->globOpt->CannotAllocateArgumentsObjectOnStack();`
	`663`	`+ this->globOpt->CannotAllocateArgumentsObjectOnStack(nullptr);`
`664`	`664`	`}`
`665`	`665`	`}`
`666`	`666`