Skip to content

Commit cff87d0

Browse files
committed
Address CR comments
1 parent bf1d565 commit cff87d0

File tree

3 files changed

+3
-5
lines changed

3 files changed

+3
-5
lines changed

lib/Backend/GlobOpt.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4687,15 +4687,16 @@ GlobOpt::ValueNumberDst(IR::Instr **pInstr, Value *src1Val, Value *src2Val)
46874687
case Js::OpCode::LdFld:
46884688
case Js::OpCode::LdFldForTypeOf:
46894689
case Js::OpCode::LdFldForCallApplyTarget:
4690-
// 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
4690+
// 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.
4691+
// 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.
46914692
//case Js::OpCode::LdRootFldForTypeOf:
4693+
//case Js::OpCode::ScopedLdFldForTypeOf:
46924694
case Js::OpCode::LdRootFld:
46934695
case Js::OpCode::LdMethodFld:
46944696
case Js::OpCode::LdRootMethodFld:
46954697
case Js::OpCode::ScopedLdMethodFld:
46964698
case Js::OpCode::LdMethodFromFlags:
46974699
case Js::OpCode::ScopedLdFld:
4698-
case Js::OpCode::ScopedLdFldForTypeOf:
46994700
if (instr->IsProfiledInstr())
47004701
{
47014702
ValueType profiledValueType(instr->AsProfiledInstr()->u.FldInfo().valueType);

lib/Backend/GlobOptFields.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -566,8 +566,6 @@ GlobOpt::AssertCanCopyPropOrCSEFieldLoad(IR::Instr * instr)
566566
// Consider: Hoisting LdRootFld may have complication with exception if the field doesn't exist.
567567
// We need to have another opcode for the hoisted version to avoid the exception and bailout.
568568

569-
// Consider: Theoretically, we can copy prop/field hoist ScopedLdFld/ScopedStFld
570-
// but Instr::TransferSrcValue blocks that now, and copy prop into that instruction is not supported yet.
571569
Assert(instr->m_opcode == Js::OpCode::LdSlot || instr->m_opcode == Js::OpCode::LdSlotArr
572570
|| instr->m_opcode == Js::OpCode::LdFld || instr->m_opcode == Js::OpCode::LdFldForCallApplyTarget
573571
|| instr->m_opcode == Js::OpCode::LdLen_A

lib/Backend/IR.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3305,7 +3305,6 @@ bool Instr::TransfersSrcValue()
33053305
// No point creating an unknown value for the src of a binary instr, as the dst will just be a different
33063306
// Don't create value for instruction without dst as well. The value doesn't go anywhere.
33073307

3308-
// if (src2 == nullptr) Disable copy prop for ScopedLdFld/ScopeStFld, etc., consider enabling that in the future
33093308
// Consider: Add opcode attribute to indicate whether the opcode would use the value or not
33103309

33113310
return this->GetDst() != nullptr && this->GetSrc2() == nullptr && !OpCodeAttr::DoNotTransfer(this->m_opcode) && !this->CallsAccessor();

0 commit comments

Comments
 (0)