[1.6>1.7] [MERGE #3424 @rajatd] OS#9726517 : Give NewScFunc the corre…

…ct nested function index when we transfrom GetCachedFunc to NewScFunc in the backend Merge pull request #3424 from rajatd:slotIdToNestedIndex As part of stack args optimization, we remove all references to scope object from the IR. One of the instructions referencing the scope object (cached scope object in this case) is GetCachedFunc, which loads the nested function object from a particulat index from the cached scope. In the deadstore phase, we change it to NewScFunc and create a new function object instead of using the cached one. Slot values in the cached scope and the function body's nested function array may correspond to different nested functions. In this situation, we were incorrectly setting the index of the nested function on NewScFunc by reusing the index from GetCachedFunc.
chakra-core · Aug 8, 2017 · c19c1c0 · c19c1c0
2 parents 895f892 + 5119d79
commit c19c1c0
Show file tree

Hide file tree

Showing 9 changed files with 125 additions and 15 deletions.
diff --git a/lib/Backend/BackwardPass.cpp b/lib/Backend/BackwardPass.cpp
@@ -3062,8 +3062,12 @@ BackwardPass::DeadStoreOrChangeInstrForScopeObjRemoval(IR::Instr ** pInstrPrev)
                 {
                     instr->m_opcode = Js::OpCode::NewScFunc;
                     IR::Opnd * intConstOpnd = instr->UnlinkSrc2();
+                    Assert(intConstOpnd->IsIntConstOpnd());
 
-                    instr->ReplaceSrc1(intConstOpnd);
+                    uint nestedFuncIndex = instr->m_func->GetJITFunctionBody()->GetNestedFuncIndexForSlotIdInCachedScope(intConstOpnd->AsIntConstOpnd()->AsUint32());
+                    intConstOpnd->Free(instr->m_func);
+
+                    instr->ReplaceSrc1(IR::IntConstOpnd::New(nestedFuncIndex, TyUint32, instr->m_func));
                     instr->SetSrc2(IR::RegOpnd::New(currFunc->GetLocalFrameDisplaySym(), IRType::TyVar, currFunc));
                 }
                 break;

diff --git a/lib/Backend/JITTimeFunctionBody.cpp b/lib/Backend/JITTimeFunctionBody.cpp
@@ -243,6 +243,12 @@ JITTimeFunctionBody::InitializeJITFunctionData(
     jitBody->literalRegexCount = functionBody->GetLiteralRegexCount();
     jitBody->literalRegexes = (intptr_t*)functionBody->GetLiteralRegexesWithLock();
 
+    Js::AuxArray<uint32> * slotIdInCachedScopeToNestedIndexArray = functionBody->GetSlotIdInCachedScopeToNestedIndexArrayWithLock();
+    if (slotIdInCachedScopeToNestedIndexArray)
+    {
+        jitBody->functionSlotsInCachedScopeCount = slotIdInCachedScopeToNestedIndexArray->count;
+        jitBody->slotIdInCachedScopeToNestedIndexArray = slotIdInCachedScopeToNestedIndexArray->elements;
+    }
 #ifdef ASMJS_PLAT
     if (functionBody->GetIsAsmJsFunction())
     {
@@ -924,6 +930,14 @@ JITTimeFunctionBody::GetLiteralRegexAddr(uint index) const
     return m_bodyData.literalRegexes[index];
 }
 
+uint
+JITTimeFunctionBody::GetNestedFuncIndexForSlotIdInCachedScope(uint index) const
+{
+    AssertOrFailFast(m_bodyData.slotIdInCachedScopeToNestedIndexArray != nullptr);
+    AssertOrFailFast(index < m_bodyData.functionSlotsInCachedScopeCount);
+    return m_bodyData.slotIdInCachedScopeToNestedIndexArray[index];
+}
+
 void *
 JITTimeFunctionBody::GetConstTable() const
 {

diff --git a/lib/Backend/JITTimeFunctionBody.h b/lib/Backend/JITTimeFunctionBody.h
@@ -167,6 +167,7 @@ class JITTimeFunctionBody
     intptr_t GetFormalsPropIdArrayAddr() const;
     intptr_t GetObjectLiteralTypeRef(uint index) const;
     intptr_t GetLiteralRegexAddr(uint index) const;
+    uint GetNestedFuncIndexForSlotIdInCachedScope(uint index) const;
     const AsmJsJITInfo * GetAsmJsInfo() const;
     const JITTimeProfileInfo * GetReadOnlyProfileInfo() const;
     JITTimeProfileInfo * GetProfileInfo() const;

diff --git a/lib/JITIDL/JITTypes.h b/lib/JITIDL/JITTypes.h
@@ -546,6 +546,7 @@ typedef struct FunctionBodyDataIDL
     unsigned int literalRegexCount;
     unsigned int auxDataCount;
     unsigned int auxContextDataCount;
+    unsigned int functionSlotsInCachedScopeCount;
 
     unsigned int fullStatementMapCount;
     unsigned int propertyIdsForRegSlotsCount;
@@ -576,6 +577,8 @@ typedef struct FunctionBodyDataIDL
 
     IDL_DEF([size_is(auxContextDataCount)]) byte * auxContextData;
 
+    IDL_DEF([size_is(functionSlotsInCachedScopeCount)]) unsigned int * slotIdInCachedScopeToNestedIndexArray;
+
     ProfileDataIDL * profileData;
 
     AsmJsDataIDL * asmJsData;

diff --git a/lib/Runtime/Base/FunctionBody.cpp b/lib/Runtime/Base/FunctionBody.cpp
@@ -4915,6 +4915,7 @@ namespace Js
         this->SetFormalsPropIdArray(nullptr);
         this->SetReferencedPropertyIdMap(nullptr);
         this->SetLiteralRegexs(nullptr);
+        this->SetSlotIdInCachedScopeToNestedIndexArray(nullptr);
         this->SetStatementMaps(nullptr);
         this->SetCodeGenGetSetRuntimeData(nullptr);
         this->SetPropertyIdOnRegSlotsContainer(nullptr);
@@ -6302,6 +6303,13 @@ namespace Js
         this->SetLiteralRegexs(nullptr);
     }
 
+    Js::AuxArray<uint32> * FunctionBody::AllocateSlotIdInCachedScopeToNestedIndexArray(uint32 slotCount)
+    {
+        Js::AuxArray<uint32> * slotIdToNestedIndexArray = RecyclerNewPlusLeaf(GetScriptContext()->GetRecycler(), slotCount * sizeof(uint32), Js::AuxArray<uint32>, slotCount);
+        SetSlotIdInCachedScopeToNestedIndexArray(slotIdToNestedIndexArray);
+        return slotIdToNestedIndexArray;
+    }
+
     void FunctionBody::ResetProfileIds()
     {
 #if ENABLE_PROFILE_INFO
@@ -6329,6 +6337,7 @@ namespace Js
         ResetLiteralRegexes();
         ResetLoops();
         ResetProfileIds();
+        ResetSlotIdInCachedScopeToNestedIndexArray();
 
         SetFirstTmpRegister(Constants::NoRegister);
         SetLocalClosureRegister(Constants::NoRegister);

diff --git a/lib/Runtime/Base/FunctionBody.h b/lib/Runtime/Base/FunctionBody.h
@@ -1358,6 +1358,7 @@ namespace Js
             ScopeInfo = 20,
             FormalsPropIdArray = 21,
             ForInCacheArray = 22,
+            SlotIdInCachedScopeToNestedIndexArray = 23,
 
             Max,
             Invalid = 0xff
@@ -3463,10 +3464,17 @@ namespace Js
         void SetLiteralRegex(const uint index, UnifiedRegex::RegexPattern *const pattern);
         Field(DynamicType*)* GetObjectLiteralTypes() const { return static_cast<Field(DynamicType*)*>(this->GetAuxPtr(AuxPointerType::ObjLiteralTypes)); }
         Field(DynamicType*)* GetObjectLiteralTypesWithLock() const { return static_cast<Field(DynamicType*)*>(this->GetAuxPtrWithLock(AuxPointerType::ObjLiteralTypes)); }
+
+        Js::AuxArray<uint32> * GetSlotIdInCachedScopeToNestedIndexArray() const { return static_cast<Js::AuxArray<uint32> *>(this->GetAuxPtr(AuxPointerType::SlotIdInCachedScopeToNestedIndexArray)); }
+        Js::AuxArray<uint32> * GetSlotIdInCachedScopeToNestedIndexArrayWithLock() const { return static_cast<Js::AuxArray<uint32> *>(this->GetAuxPtrWithLock(AuxPointerType::SlotIdInCachedScopeToNestedIndexArray)); }
+        Js::AuxArray<uint32> * AllocateSlotIdInCachedScopeToNestedIndexArray(uint32 slotCount);
+
     private:
         void ResetLiteralRegexes();
         void ResetObjectLiteralTypes();
         void SetObjectLiteralTypes(DynamicType** objLiteralTypes) { this->SetAuxPtr(AuxPointerType::ObjLiteralTypes, objLiteralTypes); };
+        void SetSlotIdInCachedScopeToNestedIndexArray(Js::AuxArray<uint32> * slotIdInCachedScopeToNestedIndexArray) { this->SetAuxPtr(AuxPointerType::SlotIdInCachedScopeToNestedIndexArray, slotIdInCachedScopeToNestedIndexArray); }
+        void ResetSlotIdInCachedScopeToNestedIndexArray() { SetSlotIdInCachedScopeToNestedIndexArray(nullptr); }
     public:
 
         void ResetByteCodeGenState();

diff --git a/lib/Runtime/ByteCode/ByteCodeCacheReleaseFileVersion.h b/lib/Runtime/ByteCode/ByteCodeCacheReleaseFileVersion.h
@@ -4,6 +4,6 @@
 //-------------------------------------------------------------------------------------------------------
 // NOTE: If there is a merge conflict the correct fix is to make a new GUID.
 
-// {0c4030fc-3b4d-4d52-99f0-9c0995c1a7ef}
+// {5C44723E-F7DF-4397-A24B-AC1A90D221F9}
 const GUID byteCodeCacheReleaseFileVersion =
-{ 0x0c4030fc, 0x3b4d, 0x4d52, { 0x99, 0xf0, 0x9c, 0x09, 0x95, 0xc1, 0xa7, 0xef } };
+{ 0x5C44723E, 0xF7DF, 0x4397, { 0xA2, 0x4B, 0xAC, 0x1A, 0x90, 0xD2, 0x21, 0xF9 } };
diff --git a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp
@@ -1058,6 +1058,9 @@ void ByteCodeGenerator::DefineCachedFunctions(FuncInfo *funcInfoParent)
 
     Js::FuncInfoArray *info = AnewPlus(alloc, extraBytes, Js::FuncInfoArray, slotCount);
 
+    // slotCount is guaranteed to be non-zero here.
+    Js::AuxArray<uint32> * slotIdInCachedScopeToNestedIndexArray = funcInfoParent->GetParsedFunctionBody()->AllocateSlotIdInCachedScopeToNestedIndexArray(slotCount);
+
     slotCount = 0;
 
     auto fillEntries = [&](ParseNode *pnodeFnc)
@@ -1069,6 +1072,8 @@ void ByteCodeGenerator::DefineCachedFunctions(FuncInfo *funcInfoParent)
             Js::FuncInfoEntry *entry = &info->elements[slotCount];
             entry->nestedIndex = pnodeFnc->sxFnc.nestedIndex;
             entry->scopeSlot = sym->GetScopeSlot();
+
+            slotIdInCachedScopeToNestedIndexArray->elements[slotCount] = pnodeFnc->sxFnc.nestedIndex;
             slotCount++;
         }
     };

diff --git a/lib/Runtime/ByteCode/ByteCodeSerializer.cpp b/lib/Runtime/ByteCode/ByteCodeSerializer.cpp
@@ -39,12 +39,12 @@ namespace Js
     const int magicEndOfCacheIdToPropIdMap = *(int*)"]cid";
     const int magicStartOfReferencedPropIdMap = *(int*)"rid[";
     const int magicEndOfReferencedPropIdMap = *(int*)"]rid";
-    const int magicStartOfPropertyIdsForScopeSlotArray = *(int*)"scope[";
-    const int magicEndOfPropertyIdsForScopeSlotArray = *(int*)"]scope";
-    const int magicStartOfDebuggerScopes = *(int*)"dbgscope[";
-    const int magicEndOfDebuggerScopes = *(int*)"]dbgscope";
-    const int magicStartOfDebuggerScopeProperties = *(int*)"dbgscopeprop[";
-    const int magicEndOfDebuggerScopeProperties = *(int*)"]dbgscopeprop";
+    const int magicStartOfPropertyIdsForScopeSlotArray = *(int*)"scp[";
+    const int magicEndOfPropertyIdsForScopeSlotArray = *(int*)"]scp";
+    const int magicStartOfDebuggerScopes = *(int*)"dsc[";
+    const int magicEndOfDebuggerScopes = *(int*)"]dsc";
+    const int magicStartOfDebuggerScopeProperties = *(int*)"dsp[";
+    const int magicEndOfDebuggerScopeProperties = *(int*)"]dsp";
     const int magicStartOfAux = *(int*)"aux[";
     const int magicEndOfAux = *(int*)"]aux";
     const int magicStartOfAuxVarArray = *(int*)"ava[";
@@ -57,12 +57,14 @@ namespace Js
     const int magicEndOfAuxPropIdArray = *(int*)"]api";
     const int magicStartOfAuxFuncInfoArray = *(int*)"afi[";
     const int magicEndOfAuxFuncInfoArray = *(int*)"]afi";
-    const int magicStartOfAsmJsFuncInfo = *(int*)"asmfuncinfo[";
-    const int magicEndOfAsmJsFuncInfo = *(int*)"]asmfuncinfo";
-    const int magicStartOfAsmJsModuleInfo = *(int*)"asmmodinfo[";
-    const int magicEndOfAsmJsModuleInfo = *(int*)"]asmmodinfo";
-    const int magicStartOfPropIdsOfFormals = *(int*)"propIdOfFormals[";
-    const int magicEndOfPropIdsOfFormals = *(int*)"]propIdOfFormals";
+    const int magicStartOfAsmJsFuncInfo = *(int*)"aFI[";
+    const int magicEndOfAsmJsFuncInfo = *(int*)"]aFI";
+    const int magicStartOfAsmJsModuleInfo = *(int*)"ami[";
+    const int magicEndOfAsmJsModuleInfo = *(int*)"]ami";
+    const int magicStartOfPropIdsOfFormals = *(int*)"pif[";
+    const int magicEndOfPropIdsOfFormals = *(int*)"]pif";
+    const int magicStartOfSlotIdToNestedIndexArray = *(int*)"sni[";
+    const int magicEndOfSlotIdToNestedIndexArray = *(int*)"]sni"
 #endif
 
     // Serialized files are architecture specific
@@ -129,6 +131,7 @@ struct SerializedFieldList {
     bool has_m_lineNumber: 1;
     bool has_m_columnNumber: 1;
     bool has_m_nestedCount: 1;
+    bool has_slotIdInCachedScopeToNestedIndexArray : 1;
 };
 
 C_ASSERT(sizeof(GUID)==sizeof(DWORD)*4);
@@ -1566,6 +1569,27 @@ class ByteCodeBufferBuilder
         return size;
     }
 
+    uint32 AddSlotIdInCachedScopeToNestedIndexArray(BufferBuilderList& builder, FunctionBody * functionBody)
+    {
+        uint32 size = 0;
+
+#ifdef BYTE_CODE_MAGIC_CONSTANTS
+        size += PrependInt32(builder, _u("Start SlotIdInCachedScopeToNestedIndexArray"), magicStartOfSlotIdToNestedIndexArray);
+#endif
+
+        Js::AuxArray<uint32> * slotIdToNestedIndexArray = functionBody->GetSlotIdInCachedScopeToNestedIndexArray();
+        size += PrependInt32(builder, _u("SlotIdInCachedScopeToNestedIndexArray count"), slotIdToNestedIndexArray->count);
+        for (uint i = 0; i < slotIdToNestedIndexArray->count; i++)
+        {
+            size += PrependInt32(builder, _u("Nested function index for slot id in cached scope"), slotIdToNestedIndexArray->elements[i]);
+        }
+
+#ifdef BYTE_CODE_MAGIC_CONSTANTS
+        size += PrependInt32(builder, _u("End magicStartOfSlotIdToNestedIndexArray"), magicEndOfSlotIdToNestedIndexArray);
+#endif
+        return size;
+    }
+
     // Gets the number of debugger slot array scopes there are in the function body's scope chain list.
     uint32 GetDebuggerScopeSlotArrayCount(FunctionBody * function)
     {
@@ -2130,6 +2154,16 @@ class ByteCodeBufferBuilder
 
             AddPropertyIdsForScopeSlotArray(builder, function);
 
+            if (function->GetSlotIdInCachedScopeToNestedIndexArray() == nullptr)
+            {
+                definedFields.has_slotIdInCachedScopeToNestedIndexArray = false;
+            }
+            else
+            {
+                definedFields.has_slotIdInCachedScopeToNestedIndexArray = true;
+                AddSlotIdInCachedScopeToNestedIndexArray(builder, function);
+            }
+
             uint debuggerScopeSlotArraySize = GetDebuggerScopeSlotArrayCount(function);
             PrependInt32(builder, _u("Debugger Scope Slot Array Size"), debuggerScopeSlotArraySize);
             AddSlotArrayDebuggerScopes(builder, function, debuggerScopeSlotArraySize);
@@ -2985,6 +3019,33 @@ class ByteCodeBufferReader
         return current;
     }
 
+    const byte * ReadSlotIdInCachedScopeToNestedIndexArray(const byte * current, FunctionBody * functionBody)
+    {
+#ifdef BYTE_CODE_MAGIC_CONSTANTS
+        int constant;
+        current = ReadInt32(current, &constant);
+        Assert(constant == magicStartOfSlotIdToNestedIndexArray);
+#endif
+        uint32 count;
+        current = ReadUInt32(current, &count);
+
+        Js::AuxArray<uint32> * slotIdInCachedScopeToNestedIndexArray = functionBody->AllocateSlotIdInCachedScopeToNestedIndexArray(count);
+
+        uint32 value;
+        for (uint i = 0; i < count; i++)
+        {
+            current = ReadUInt32(current, &value);
+            slotIdInCachedScopeToNestedIndexArray->elements[i] = value;
+        }
+#ifdef BYTE_CODE_MAGIC_CONSTANTS
+        current = ReadInt32(current, &constant);
+        Assert(constant == magicEndOfSlotIdToNestedIndexArray);
+#endif
+
+        return current;
+    }
+
+
     const byte * ReadSlotArrayDebuggerScopeProperties(const byte * current, FunctionBody* function, DebuggerScope* debuggerScope, uint propertyCount)
     {
         Assert(function);
@@ -3766,6 +3827,11 @@ class ByteCodeBufferReader
 
             current = ReadPropertyIdsForScopeSlotArray(current, *functionBody);
 
+            if (definedFields->has_slotIdInCachedScopeToNestedIndexArray)
+            {
+                current = ReadSlotIdInCachedScopeToNestedIndexArray(current, *functionBody);
+            }
+
             uint debuggerScopeCount = 0;
             current = ReadUInt32(current, &debuggerScopeCount);
             current = ReadSlotArrayDebuggerScopes(current, *functionBody, debuggerScopeCount);