[1.10>master] [MERGE #5618 @rajatd] Copy-prop ScopedLdFld. OS#18622681

Merge pull request #5618 from rajatd:scopedLdFld-2

Enable copy-prop of ScopedLdFld and ScopedLdFldForTypeOf.

Author: rajatd

lib/Backend/GlobOpt.cpp

@@ -3289,6 +3289,7 @@ GlobOpt::OptSrc(IR::Opnd *opnd, IR::Instr * *pInstr, Value **indirIndexValRef, I
         case Js::OpCode::BrOnHasProperty:
         case Js::OpCode::LdMethodFldPolyInlineMiss:
         case Js::OpCode::StSlotChkUndecl:
+        case Js::OpCode::ScopedLdInst:
             return nullptr;
         };
 
@@ -3718,6 +3719,8 @@ GlobOpt::CopyProp(IR::Opnd *opnd, IR::Instr *instr, Value *val, IR::IndirOpnd *p
         case Js::OpCode::LdRootMethodFld:
         case Js::OpCode::LdMethodFromFlags:
         case Js::OpCode::ScopedLdMethodFld:
+        case Js::OpCode::ScopedLdFld:
+        case Js::OpCode::ScopedLdFldForTypeOf:
             instr->m_opcode = Js::OpCode::Ld_A;
         case Js::OpCode::Ld_A:
             {
@@ -3956,6 +3959,8 @@ GlobOpt::CopyPropReplaceOpnd(IR::Instr * instr, IR::Opnd * opnd, StackSym * copy
     case Js::OpCode::LdMethodFld:
     case Js::OpCode::LdRootMethodFld:
     case Js::OpCode::ScopedLdMethodFld:
+    case Js::OpCode::ScopedLdFld:
+    case Js::OpCode::ScopedLdFldForTypeOf:
         instr->m_opcode = Js::OpCode::Ld_A;
         break;
 
@@ -4682,13 +4687,16 @@ GlobOpt::ValueNumberDst(IR::Instr **pInstr, Value *src1Val, Value *src2Val)
     case Js::OpCode::LdFld:
     case Js::OpCode::LdFldForTypeOf:
     case Js::OpCode::LdFldForCallApplyTarget:
-    // Do not transfer value type on ldFldForTypeOf to prevent copy-prop to LdRootFld in case the field doesn't exist since LdRootFldForTypeOf does not throw
+    // Do not transfer value type on LdRootFldForTypeOf to prevent copy-prop to LdRootFld in case the field doesn't exist since LdRootFldForTypeOf does not throw.
+    // Same goes for ScopedLdFldForTypeOf as we'll end up loading the property from the root object if the property is not in the scope chain.
     //case Js::OpCode::LdRootFldForTypeOf:
+    //case Js::OpCode::ScopedLdFldForTypeOf:
     case Js::OpCode::LdRootFld:
     case Js::OpCode::LdMethodFld:
     case Js::OpCode::LdRootMethodFld:
     case Js::OpCode::ScopedLdMethodFld:
     case Js::OpCode::LdMethodFromFlags:
+    case Js::OpCode::ScopedLdFld:
         if (instr->IsProfiledInstr())
         {
             ValueType profiledValueType(instr->AsProfiledInstr()->u.FldInfo().valueType);

lib/Backend/GlobOptFields.cpp

@@ -566,8 +566,6 @@ GlobOpt::AssertCanCopyPropOrCSEFieldLoad(IR::Instr * instr)
     // Consider: Hoisting LdRootFld may have complication with exception if the field doesn't exist.
     // We need to have another opcode for the hoisted version to avoid the exception and bailout.
 
-    // Consider: Theoretically, we can copy prop/field hoist ScopedLdFld/ScopedStFld
-    // but Instr::TransferSrcValue blocks that now, and copy prop into that instruction is not supported yet.
     Assert(instr->m_opcode == Js::OpCode::LdSlot || instr->m_opcode == Js::OpCode::LdSlotArr
         || instr->m_opcode == Js::OpCode::LdFld || instr->m_opcode == Js::OpCode::LdFldForCallApplyTarget
         || instr->m_opcode == Js::OpCode::LdLen_A
@@ -578,7 +576,9 @@ GlobOpt::AssertCanCopyPropOrCSEFieldLoad(IR::Instr * instr)
         || instr->m_opcode == Js::OpCode::LdMethodFromFlags
         || instr->m_opcode == Js::OpCode::ScopedLdMethodFld
         || instr->m_opcode == Js::OpCode::CheckFixedFld
-        || instr->m_opcode == Js::OpCode::CheckPropertyGuardAndLoadType);
+        || instr->m_opcode == Js::OpCode::CheckPropertyGuardAndLoadType
+        || instr->m_opcode == Js::OpCode::ScopedLdFld
+        || instr->m_opcode == Js::OpCode::ScopedLdFldForTypeOf);
 
     Assert(instr->m_opcode == Js::OpCode::CheckFixedFld || instr->GetDst()->GetType() == TyVar || instr->m_func->GetJITFunctionBody()->IsAsmJsMode());
     Assert(instr->GetSrc1()->GetType() == TyVar || instr->m_func->GetJITFunctionBody()->IsAsmJsMode());

lib/Backend/IR.cpp

@@ -3305,7 +3305,6 @@ bool Instr::TransfersSrcValue()
     // No point creating an unknown value for the src of a binary instr, as the dst will just be a different
     // Don't create value for instruction without dst as well. The value doesn't go anywhere.
 
-    // if (src2 == nullptr) Disable copy prop for ScopedLdFld/ScopeStFld, etc., consider enabling that in the future
     // Consider: Add opcode attribute to indicate whether the opcode would use the value or not
 
     return this->GetDst() != nullptr && this->GetSrc2() == nullptr && !OpCodeAttr::DoNotTransfer(this->m_opcode) && !this->CallsAccessor();

lib/Backend/IRBuilder.cpp

@@ -3150,10 +3150,9 @@ IRBuilder::BuildElementScopedC(Js::OpCode newOpcode, uint32 offset, Js::RegSlot
     case Js::OpCode::ScopedDeleteFld:
     case Js::OpCode::ScopedDeleteFldStrict:
     {
-        // Implicit root object as default instance
-        IR::Opnd * instance2Opnd = this->BuildSrcOpnd(Js::FunctionBody::RootObjectRegSlot);
+        Assert(this->m_func->GetScriptContextInfo()->GetAddr() == this->m_func->GetTopFunc()->GetScriptContextInfo()->GetAddr());
         regOpnd = this->BuildDstOpnd(regSlot);
-        instr = IR::Instr::New(newOpcode, regOpnd, fieldSymOpnd, instance2Opnd, m_func);
+        instr = IR::Instr::New(newOpcode, regOpnd, fieldSymOpnd, m_func);
         break;
     }
 
@@ -4412,13 +4411,12 @@ IRBuilder::BuildElementP(Js::OpCode newOpcode, uint32 offset, Js::RegSlot regSlo
     case Js::OpCode::ScopedLdFldForTypeOf:
     {
         Assert(!isProfiled);
+        Assert(this->m_func->GetScriptContextInfo()->GetAddr() == this->m_func->GetTopFunc()->GetScriptContextInfo()->GetAddr());
 
         fieldSymOpnd = this->BuildFieldOpnd(newOpcode, instance, propertyId, (Js::PropertyIdIndexType)-1, PropertyKindData, inlineCacheIndex);
 
-        // Implicit root object as default instance
-        IR::Opnd * instance2Opnd = this->BuildSrcOpnd(Js::FunctionBody::RootObjectRegSlot);
         regOpnd = this->BuildDstOpnd(regSlot);
-        instr = IR::Instr::New(newOpcode, regOpnd, fieldSymOpnd, instance2Opnd, m_func);
+        instr = IR::Instr::New(newOpcode, regOpnd, fieldSymOpnd, m_func);
         break;
     }
 

lib/Backend/Lower.cpp

@@ -6927,8 +6927,8 @@ Lowerer::LowerScopedLdFld(IR::Instr * ldFldInstr, IR::JnHelperMethod helperMetho
         LoadScriptContext(ldFldInstr);
     }
 
-    src = ldFldInstr->UnlinkSrc2();
-    AssertMsg(src->IsRegOpnd(), "Expected reg opnd as src2");
+    intptr_t rootObject = m_func->GetJITFunctionBody()->GetRootObject();
+    src = IR::AddrOpnd::New(rootObject, IR::AddrOpndKindDynamicVar, this->m_func, true);
     instrPrev = m_lowererMD.LoadHelperArgument(ldFldInstr, src);
 
     src = ldFldInstr->UnlinkSrc1();

lib/Backend/LowerMDShared.cpp

@@ -4110,11 +4110,6 @@ LowererMD::GenerateFastScopedLdFld(IR::Instr * instrLdScopedFld)
 
     opndBase = propertySymOpnd->CreatePropertyOwnerOpnd(m_func);
 
-    IR::Opnd *srcBase =    instrLdScopedFld->GetSrc2();
-    AssertMsg(srcBase->IsRegOpnd(), "Expected reg opnd as src2");
-    //opndBase = srcBase;
-
-    //IR::IndirOpnd * indirOpnd = src->AsIndirOpnd();
     labelHelper = IR::LabelInstr::New(Js::OpCode::Label, this->m_func, true);
 
     AssertMsg(opndBase->m_sym->m_isSingleDef, "We assume this isn't redefined");