Fixing and refactoring the load of Func object operand in lower code

A hot fix for an ARM bug related to StackArgs opt was provided in the source depot branch sometime back to unblock the RI.
This change is a more elaborate fix which has other refactoring around the fix.

lib/Backend/Lower.cpp

@@ -308,30 +308,10 @@ Lowerer::LowerRange(IR::Instr *instrStart, IR::Instr *instrEnd, bool defaultDoFa
                     // s3 = formals are let decls
                     this->m_lowererMD.LoadHelperArgument(instr, IR::IntConstOpnd::New(currFunc->GetHasNonSimpleParams() ? TRUE : FALSE, TyUint8, currFunc));
 
-                    if (currFunc->IsInlinee())
-                    {
-                        // s2 = current function.
-                        this->m_lowererMD.LoadHelperArgument(instr, currFunc->GetInlineeFunctionObjectSlotOpnd());
-                    }
-                    else
-                    {
-                        // s2 = current function
-                        StackSym * paramSym = StackSym::New(TyMachReg, currFunc);
-                        currFunc->SetArgOffset(paramSym, 2 * MachPtr);
-                        IR::Opnd * srcOpnd = IR::SymOpnd::New(paramSym, TyMachReg, currFunc);
-
-                        if (currFunc->GetJITFunctionBody()->IsCoroutine())
-                        {
-                            // the function object for generator calls is a GeneratorVirtualScriptFunction object
-                            // and we need to pass the real JavascriptGeneratorFunction object so grab it instead
-                            IR::RegOpnd *tmpOpnd = IR::RegOpnd::New(TyMachReg, currFunc);
-                            LowererMD::CreateAssign(tmpOpnd, srcOpnd, instr);
-
-                            srcOpnd = IR::IndirOpnd::New(tmpOpnd, Js::GeneratorVirtualScriptFunction::GetRealFunctionOffset(), TyMachPtr, currFunc);
-                        }
-
-                        this->m_lowererMD.LoadHelperArgument(instr, srcOpnd);
-                    }
+                    // s2 = current function.
+                    IR::Opnd * paramOpnd = GetFuncObjectOpnd(instr);
+                    this->m_lowererMD.LoadHelperArgument(instr, paramOpnd);
+                    
                     m_lowererMD.ChangeToHelperCallMem(instr, IR::HelperOP_NewScopeObjectWithFormals);
                 }
                 else
@@ -2755,7 +2735,7 @@ Lowerer::LowerRange(IR::Instr *instrStart, IR::Instr *instrEnd, bool defaultDoFa
 
         case Js::OpCode::LdFuncExpr:
             // src = function Expression
-            m_lowererMD.LoadFuncExpression(instr);
+            LoadFuncExpression(instr);
             this->GenerateGetCurrentFunctionObject(instr);
             break;
 
@@ -12791,6 +12771,63 @@ void Lowerer::LowerBailOnCreatedMissingValue(IR::Instr *const instr, const bool
     //     $skipBailOut:
 }
 
+
+IR::Opnd*
+Lowerer::GetFuncObjectOpnd(IR::Instr* insertBeforeInstr)
+{
+    Func * func = insertBeforeInstr->m_func;
+    IR::Opnd *paramOpnd = nullptr;
+    if (func->IsInlinee())
+    {
+        paramOpnd = func->GetInlineeFunctionObjectSlotOpnd();
+    }
+    else
+    {
+#if defined(_M_ARM)
+        StackSym * paramSym = this->m_lowererMD.GetImplicitParamSlotSym(0);
+#else
+        StackSym *paramSym = StackSym::New(TyMachReg, this->m_func);
+        this->m_func->SetArgOffset(paramSym, 2 * MachPtr);
+        this->m_func->SetHasImplicitParamLoad();
+#endif
+        paramOpnd = IR::SymOpnd::New(paramSym, TyMachReg, this->m_func);
+    }
+
+    if (func->GetJITFunctionBody()->IsCoroutine())
+    {
+        // the function object for generator calls is a GeneratorVirtualScriptFunction object
+        // and we need to return the real JavascriptGeneratorFunction object so grab it before
+        // assigning to the dst
+        Assert(!func->IsInlinee());
+        IR::RegOpnd *tmpOpnd = IR::RegOpnd::New(TyMachReg, func);
+        LowererMD::CreateAssign(tmpOpnd, paramOpnd, insertBeforeInstr);
+
+        paramOpnd = IR::IndirOpnd::New(tmpOpnd, Js::GeneratorVirtualScriptFunction::GetRealFunctionOffset(), TyMachPtr, func);
+    }
+    return paramOpnd;
+}
+
+///----------------------------------------------------------------------------
+///
+/// Lowerer::LoadFuncExpression
+///
+///     Load the function expression to src1 from [ebp + 8]
+///
+///----------------------------------------------------------------------------
+
+IR::Instr *
+Lowerer::LoadFuncExpression(IR::Instr *instrFuncExpr)
+{
+    ASSERT_INLINEE_FUNC(instrFuncExpr);
+    IR::Opnd *paramOpnd = GetFuncObjectOpnd(instrFuncExpr);
+
+    // mov dst, param
+    instrFuncExpr->SetSrc1(paramOpnd);
+    LowererMD::ChangeToAssign(instrFuncExpr);
+
+    return instrFuncExpr;
+}
+
 void Lowerer::LowerBoundCheck(IR::Instr *const instr)
 {
     Assert(instr);
@@ -22232,7 +22269,7 @@ Lowerer::GenerateLoadNewTarget(IR::Instr* instrInsert)
 
     IR::Instr* loadFuncInstr = IR::Instr::New(Js::OpCode::AND, func);
     loadFuncInstr->SetDst(instrInsert->GetDst());
-    m_lowererMD.LoadFuncExpression(loadFuncInstr);
+    LoadFuncExpression(loadFuncInstr);
 
     instrInsert->InsertBefore(loadFuncInstr);
     InsertBranch(Js::OpCode::Br, labelDone, instrInsert);
@@ -22285,7 +22322,7 @@ Lowerer::GetInlineCacheFromFuncObjectForRuntimeUse(IR::Instr * instr, IR::Proper
     IR::RegOpnd * funcObjOpnd = IR::RegOpnd::New(TyMachPtr, this->m_func);
     IR::Instr * funcObjInstr = IR::Instr::New(Js::OpCode::Ld_A, funcObjOpnd, instr->m_func);
     instr->InsertBefore(funcObjInstr);
-    this->m_lowererMD.LoadFuncExpression(funcObjInstr);
+    LoadFuncExpression(funcObjInstr);
 
     IR::RegOpnd * funcObjHasInlineCachesOpnd = IR::RegOpnd::New(TyMachPtr, instr->m_func);
     this->m_lowererMD.CreateAssign(funcObjHasInlineCachesOpnd, IR::IndirOpnd::New(funcObjOpnd, Js::ScriptFunction::GetOffsetOfHasInlineCaches(), TyUint8, instr->m_func), instr);

lib/Backend/Lower.h

@@ -441,6 +441,8 @@ private:
     void            LowerBailOnInvalidatedArrayLength(IR::Instr *const instr, const bool isInHelperBlock);
     void            LowerBailOnCreatedMissingValue(IR::Instr *const instr, const bool isInHelperBlock);
     void            LowerBoundCheck(IR::Instr *const instr);
+    IR::Opnd*       GetFuncObjectOpnd(IR::Instr* insertBeforeInstr);
+    IR::Instr*      LoadFuncExpression(IR::Instr *instrFuncExpr);
 
     IR::Instr *     LowerBailTarget(IR::Instr * instr);
     IR::Instr *     SplitBailOnImplicitCall(IR::Instr *& instr);

lib/Backend/LowerMDShared.cpp

@@ -654,12 +654,6 @@ LowererMD::LoadHeapArgsCached(IR::Instr * instrArgs)
     return this->lowererMDArch.LoadHeapArgsCached(instrArgs);
 }
 
-IR::Instr *
-LowererMD::LoadFuncExpression(IR::Instr * instrFuncExpr)
-{
-    return this->lowererMDArch.LoadFuncExpression(instrFuncExpr);
-}
-
 ///----------------------------------------------------------------------------
 ///
 /// LowererMD::ChangeToHelperCall

lib/Backend/LowerMDShared.h

@@ -97,7 +97,6 @@ public:
             IR::Instr *     LoadArgumentCount(IR::Instr * instr);
             IR::Instr *     LoadHeapArguments(IR::Instr * instr);
             IR::Instr *     LoadHeapArgsCached(IR::Instr * instr);
-            IR::Instr *     LoadFuncExpression(IR::Instr * instr);
             IR::Instr *     LowerRet(IR::Instr * instr);
             IR::Instr *     LowerUncondBranch(IR::Instr * instr);
             IR::Instr *     LowerMultiBranch(IR::Instr * instr);

lib/Backend/amd64/LowererMDArch.cpp

@@ -408,50 +408,6 @@ LowererMDArch::LoadHeapArguments(IR::Instr *instrArgs)
     return instrPrev;
 }
 
-///----------------------------------------------------------------------------
-///
-/// LowererMDArch::LoadFuncExpression
-///
-///     Load the function expression to src1 from [ebp + 8]
-///
-///----------------------------------------------------------------------------
-
-IR::Instr *
-LowererMDArch::LoadFuncExpression(IR::Instr *instrFuncExpr)
-{
-    ASSERT_INLINEE_FUNC(instrFuncExpr);
-    Func *func = instrFuncExpr->m_func;
-
-    IR::Opnd *paramOpnd = nullptr;
-    if (func->IsInlinee())
-    {
-        paramOpnd = func->GetInlineeFunctionObjectSlotOpnd();
-    }
-    else
-    {
-        StackSym *paramSym = StackSym::New(TyMachReg, this->m_func);
-        this->m_func->SetArgOffset(paramSym, 2 * MachPtr);
-        paramOpnd = IR::SymOpnd::New(paramSym, TyMachReg, this->m_func);
-    }
-
-    if (instrFuncExpr->m_func->GetJITFunctionBody()->IsCoroutine())
-    {
-        // the function object for generator calls is a GeneratorVirtualScriptFunction object
-        // and we need to return the real JavascriptGeneratorFunction object so grab it before
-        // assigning to the dst
-        IR::RegOpnd *tmpOpnd = IR::RegOpnd::New(TyMachReg, func);
-        LowererMD::CreateAssign(tmpOpnd, paramOpnd, instrFuncExpr);
-
-        paramOpnd = IR::IndirOpnd::New(tmpOpnd, Js::GeneratorVirtualScriptFunction::GetRealFunctionOffset(), TyMachPtr, func);
-    }
-
-    // mov dst, param
-    instrFuncExpr->SetSrc1(paramOpnd);
-    LowererMD::ChangeToAssign(instrFuncExpr);
-
-    return instrFuncExpr;
-}
-
 //
 // Load the parameter in the first argument slot
 //

lib/Backend/amd64/LowererMDArch.h

@@ -82,7 +82,6 @@ public:
     IR::Instr *         LoadStackArgPtr(IR::Instr * instr);
     IR::Instr *         LoadHeapArguments(IR::Instr * instr);
     IR::Instr *         LoadHeapArgsCached(IR::Instr * instr);
-    IR::Instr *         LoadFuncExpression(IR::Instr * instr);
     IR::Instr *         LowerEntryInstr(IR::EntryInstr * entryInstr);
     void                GeneratePrologueStackProbe(IR::Instr *entryInstr, IntConstType frameSize);
     IR::Instr *         LowerExitInstr(IR::ExitInstr * exitInstr);

lib/Backend/arm/LowerMD.cpp

@@ -2243,28 +2243,6 @@ LowererMD::LoadHeapArgsCached(IR::Instr * instrArgs)
     return instrPrev;
 }
 
-IR::Instr *
-LowererMD::LoadFuncExpression(IR::Instr * instrFuncExpr)
-{
-    ASSERT_INLINEE_FUNC(instrFuncExpr);
-    Func *func = instrFuncExpr->m_func;
-
-    IR::Opnd *paramOpnd = nullptr;
-    if (func->IsInlinee())
-    {
-        paramOpnd = func->GetInlineeFunctionObjectSlotOpnd();
-    }
-    else
-    {
-        //function object is first argument and mark it as IsParamSlotSym.
-        StackSym * paramSym = GetImplicitParamSlotSym(0);
-        paramOpnd = IR::SymOpnd::New(paramSym, TyMachReg, this->m_func);
-    }
-    instrFuncExpr->SetSrc1(paramOpnd);
-    this->ChangeToAssign(instrFuncExpr);
-    return instrFuncExpr;
-}
-
 ///----------------------------------------------------------------------------
 ///
 /// LowererMD::ChangeToHelperCall

lib/Backend/arm/LowerMD.h

@@ -83,7 +83,6 @@ public:
             IR::Instr *     LoadInputParamPtr(IR::Instr * instrInsert, IR::RegOpnd * optionalDstOpnd = nullptr);
             IR::Instr *     LoadInputParamCount(IR::Instr * instr, int adjust = 0, bool needFlags = false);
             IR::Instr *     LoadArgumentsFromFrame(IR::Instr * instr);
-            IR::Instr *     LoadFuncExpression(IR::Instr * instr);
             IR::Instr *     LowerRet(IR::Instr * instr);
     static  IR::Instr *     LowerUncondBranch(IR::Instr * instr);
     static  IR::Instr *     LowerMultiBranch(IR::Instr * instr);

lib/Backend/arm64/LowerMD.h

@@ -76,7 +76,6 @@ public:
               IR::Instr *     LoadHeapArgsCached(IR::Instr * instr) { __debugbreak(); return 0; }
               IR::Instr *     LoadInputParamCount(IR::Instr * instr, int adjust = 0, bool needFlags = false) { __debugbreak(); return 0; }
               IR::Instr *     LoadArgumentsFromFrame(IR::Instr * instr) { __debugbreak(); return 0; }
-              IR::Instr *     LoadFuncExpression(IR::Instr * instr) { __debugbreak(); return 0; }
               IR::Instr *     LowerRet(IR::Instr * instr) { __debugbreak(); return 0; }
       static  IR::Instr *     LowerUncondBranch(IR::Instr * instr) { __debugbreak(); return 0; }
       static  IR::Instr *     LowerMultiBranch(IR::Instr * instr) { __debugbreak(); return 0; }

lib/Backend/i386/LowererMDArch.cpp

@@ -473,53 +473,6 @@ LowererMDArch::LoadHeapArgsCached(IR::Instr *instrArgs)
     return instrPrev;
 }
 
-///----------------------------------------------------------------------------
-///
-/// LowererMDArch::LoadFuncExpression
-///
-///     Load the function expression to src1 from [ebp + 8]
-///
-///----------------------------------------------------------------------------
-
-IR::Instr *
-LowererMDArch::LoadFuncExpression(IR::Instr *instrFuncExpr)
-{
-    ASSERT_INLINEE_FUNC(instrFuncExpr);
-    Func *func = instrFuncExpr->m_func;
-
-    IR::Opnd *paramOpnd = nullptr;
-    if (func->IsInlinee())
-    {
-        paramOpnd = func->GetInlineeFunctionObjectSlotOpnd();
-    }
-    else
-    {
-        //
-        // dst = current function ([ebp + 8])
-        //
-        StackSym *paramSym = StackSym::New(TyMachReg, func);
-        this->m_func->SetArgOffset(paramSym, 2 * MachPtr);
-        paramOpnd = IR::SymOpnd::New(paramSym, TyMachReg, func);
-    }
-
-    if (instrFuncExpr->m_func->GetJITFunctionBody()->IsCoroutine())
-    {
-        // the function object for generator calls is a GeneratorVirtualScriptFunction object
-        // and we need to return the real JavascriptGeneratorFunction object so grab it before
-        // assigning to the dst
-        IR::RegOpnd *tmpOpnd = IR::RegOpnd::New(TyMachReg, func);
-        LowererMD::CreateAssign(tmpOpnd, paramOpnd, instrFuncExpr);
-
-        paramOpnd = IR::IndirOpnd::New(tmpOpnd, Js::GeneratorVirtualScriptFunction::GetRealFunctionOffset(), TyMachPtr, func);
-    }
-
-    // mov dst, param
-    instrFuncExpr->SetSrc1(paramOpnd);
-    LowererMD::ChangeToAssign(instrFuncExpr);
-
-    return instrFuncExpr;
-}
-
 //
 // Load the parameter in the first argument slot
 //

lib/Backend/i386/LowererMDArch.h

@@ -68,7 +68,6 @@ public:
             IR::Instr *         LoadStackArgPtr(IR::Instr * instr);
             IR::Instr *         LoadHeapArguments(IR::Instr * instr);
             IR::Instr *         LoadHeapArgsCached(IR::Instr * instr);
-            IR::Instr *         LoadFuncExpression(IR::Instr * instr);
             IR::Instr *         LowerEntryInstr(IR::EntryInstr * entryInstr);
             IR::Instr *         LowerExitInstr(IR::ExitInstr * exitInstr);
             IR::Instr *         LowerEntryInstrAsmJs(IR::EntryInstr * entryInstr);

test/Function/StackArgsWithFormals.baseline

@@ -21,4 +21,5 @@ StackArgFormals : test16 (22) :Attaching the scope object with the heap argument
 StackArgFormals : test17 (23) :Removing Heap Arguments object creation in Lowerer. 
 StackArgFormals : test17 (23) :Attaching the scope object with the heap arguments object in the bail out path. 
 StackArgFormals : test19 (25) :Removing Heap Arguments object creation in Lowerer. 
+StackArgFormals : test20 (26) :Removing Heap Arguments object creation in Lowerer. 
 PASSED

test/Function/StackArgsWithFormals.js

@@ -291,6 +291,18 @@ test19(1, 2);
 test19(3, 4);
 verify([4, 8], "TEST 19");
 
+function test20(v16) {
+  while ({}.prop0) {
+    var v17 = 0;
+    function test20_inner() {
+      v17;
+    }
+    arguments;
+  }
+}
+test20();
+test20();
+
 if(hasAllPassed)
 {
     print("PASSED");