Aggregate byteCodeUpwardExposedUsed for ByteCodeUses instrs across ToVar, Ld_A, and BailOnNotObject

Author: rajatd

lib/Backend/GlobOpt.cpp

@@ -2643,6 +2643,16 @@ GlobOpt::OptInstr(IR::Instr *&instr, bool* isInstrRemoved)
         this->InsertByteCodeUses(instr);
     }
 
+    if (!IsLoopPrePass() && instr->HasBailOutInfo())
+    {
+        // Aggregate byteCodeUpwardExposedUsed of preceding ByteCodeUses instrs with the same bytecode offset.
+        // This is required as different ByteCodeUses instrs may be inserted for an instr in the loop pre-pass
+        // and the main pass (and there may be additional instructions inserted between the two sets of 
+        // ByteCodeUses instructions), but the Backward Pass only processes immediately preceding consecutive
+        // ByteCodeUses instructions before processing a pre-op bailout.
+        instr->AggregateByteCodeUses();
+    }
+
     if (!this->IsLoopPrePass() && !isHoisted && this->IsImplicitCallBailOutCurrentlyNeeded(instr, src1Val, src2Val))
     {
         IR::BailOutKind kind = IR::BailOutOnImplicitCalls;
@@ -9588,7 +9598,7 @@ GlobOpt::TypeSpecializeBinary(IR::Instr **pInstr, Value **pSrc1Val, Value **pSrc
                 // calculation, we want to insert the Conv_bool after the whole compare instruction
                 // block.
                 IR::Instr *putAfter = instr;
-                while (putAfter->m_next && putAfter->m_next->m_opcode == Js::OpCode::ByteCodeUses)
+                while (putAfter->m_next && putAfter->m_next->IsByteCodeUsesInstrFor(instr))
                 {
                     putAfter = putAfter->m_next;
                 }

lib/Backend/GlobOptBailOut.cpp

@@ -1076,7 +1076,7 @@ GlobOpt::ConvertToByteCodeUses(IR::Instr * instr)
     instr->Remove();
     if (byteCodeUsesInstr)
     {
-        byteCodeUsesInstr->Aggregate();
+        byteCodeUsesInstr->AggregateFollowingByteCodeUses();
     }
     return byteCodeUsesInstr;
 }

lib/Backend/IR.cpp

@@ -984,28 +984,69 @@ void ByteCodeUsesInstr::SetBV(BVSparse<JitArenaAllocator>* newbv)
 // a compare, but still need to generate them for bailouts. Without this, we cause
 // problems because we end up with an instruction losing atomicity in terms of its
 // bytecode use and generation lifetimes.
-void ByteCodeUsesInstr::Aggregate()
+void ByteCodeUsesInstr::AggregateFollowingByteCodeUses()
 {
     IR::Instr* scanner = this->m_next;
     while (scanner && scanner->m_opcode == Js::OpCode::ByteCodeUses && scanner->GetByteCodeOffset() == this->GetByteCodeOffset() && scanner->GetDst() == nullptr)
     {
         IR::ByteCodeUsesInstr* target = scanner->AsByteCodeUsesInstr();
-        Assert(this->m_func == target->m_func);
-        if (target->byteCodeUpwardExposedUsed)
+        this->Aggregate(target);
+        scanner = scanner->m_next;
+    }
+}
+
+void ByteCodeUsesInstr::AggregatePrecedingByteCodeUses()
+{
+    IR::Instr * instr = this->m_prev;
+    while (instr && this->CanAggregateByteCodeUsesAcrossInstr(instr))
+    {
+        if (instr->IsByteCodeUsesInstr())
         {
-            if (this->byteCodeUpwardExposedUsed)
-            {
-                this->byteCodeUpwardExposedUsed->Or(target->byteCodeUpwardExposedUsed);
-                JitAdelete(target->byteCodeUpwardExposedUsed->GetAllocator(), target->byteCodeUpwardExposedUsed);
-                target->byteCodeUpwardExposedUsed = nullptr;
-            }
-            else
-            {
-                this->byteCodeUpwardExposedUsed = target->byteCodeUpwardExposedUsed;
-                target->byteCodeUpwardExposedUsed = nullptr;
-            }
+            IR::ByteCodeUsesInstr* precedingByteCodeUsesInstr = instr->AsByteCodeUsesInstr();
+            this->Aggregate(precedingByteCodeUsesInstr);
+        }
+        instr = instr->m_prev;
+    }
+}
+
+void ByteCodeUsesInstr::Aggregate(ByteCodeUsesInstr * byteCodeUsesInstr)
+{
+    Assert(this->m_func == byteCodeUsesInstr->m_func);
+    if (byteCodeUsesInstr->byteCodeUpwardExposedUsed)
+    {
+        if (this->byteCodeUpwardExposedUsed)
+        {
+            this->byteCodeUpwardExposedUsed->Or(byteCodeUsesInstr->byteCodeUpwardExposedUsed);
+            JitAdelete(byteCodeUsesInstr->byteCodeUpwardExposedUsed->GetAllocator(), byteCodeUsesInstr->byteCodeUpwardExposedUsed);
+            byteCodeUsesInstr->byteCodeUpwardExposedUsed = nullptr;
+        }
+        else
+        {
+            this->byteCodeUpwardExposedUsed = byteCodeUsesInstr->byteCodeUpwardExposedUsed;
+            byteCodeUsesInstr->byteCodeUpwardExposedUsed = nullptr;
+        }
+    }
+}
+
+bool Instr::CanAggregateByteCodeUsesAcrossInstr(Instr * instr)
+{
+    if (instr->GetByteCodeOffset() != Js::Constants::NoByteCodeOffset)
+    {
+        return (instr->GetByteCodeOffset() == this->GetByteCodeOffset()) &&
+                    (instr->IsByteCodeUsesInstr() ||
+                     instr->m_opcode == Js::OpCode::ToVar);
+    }
+    else
+    {
+        if (instr->HasBailOutInfo())
+        {
+            return (instr->GetBailOutInfo()->bailOutOffset == this->GetByteCodeOffset()) &&
+                instr->m_opcode == Js::OpCode::BailOnNotObject;
+        }
+        else
+        {
+            return (instr->m_opcode == Js::OpCode::Ld_A || instr->m_opcode == Js::OpCode::Ld_I4) && instr->GetSrc1()->IsImmediateOpnd(); // could have been inserted by PreLowerCanonicalize
         }
-        scanner = scanner->m_next;
     }
 }
 
@@ -2838,6 +2879,27 @@ IR::Instr *Instr::GetInsertBeforeByteCodeUsesInstr()
     return insertBeforeInstr;
 }
 
+IR::ByteCodeUsesInstr *
+Instr::GetFirstByteCodeUsesInstrBackward()
+{
+    IR::Instr * prevInstr = this->m_prev;
+    while (prevInstr && this->CanAggregateByteCodeUsesAcrossInstr(prevInstr))
+    {
+        if (prevInstr->IsByteCodeUsesInstr())
+        {
+            return prevInstr->AsByteCodeUsesInstr();
+        }
+        prevInstr = prevInstr->m_prev;
+    }
+    return nullptr;
+}
+
+bool
+Instr::IsByteCodeUsesInstrFor(IR::Instr * instr) const
+{
+    return this->IsByteCodeUsesInstr() && this->GetByteCodeOffset() == instr->GetByteCodeOffset();
+}
+
 ///----------------------------------------------------------------------------
 ///
 /// Instr::GetOrCreateContinueLabel
@@ -3341,6 +3403,22 @@ void Instr::Move(IR::Instr* insertInstr)
     insertInstr->InsertBefore(this);
 }
 
+void Instr::AggregateByteCodeUses()
+{
+    // Currently, this only aggregates byteCodeUpwardExposedUsed of those ByteCodeUses instructions 
+    // associated with this instr which have at most a ToVar, Ld_A, Ld_I4, or a BailOnNotObject between them.
+    IR::ByteCodeUsesInstr * primaryByteCodeUsesInstr = this->GetFirstByteCodeUsesInstrBackward();
+    if (primaryByteCodeUsesInstr)
+    {
+        primaryByteCodeUsesInstr->AggregatePrecedingByteCodeUses();
+        if (primaryByteCodeUsesInstr != this->m_prev)
+        {
+            primaryByteCodeUsesInstr->Unlink();
+            this->InsertBefore(primaryByteCodeUsesInstr);
+        }
+    }
+}
+
 IR::Instr* Instr::GetBytecodeArgOutCapture()
 {
     Assert(this->m_opcode == Js::OpCode::ArgOut_A_Inline ||

lib/Backend/IR.h

@@ -290,13 +290,16 @@ class Instr
     IR::Instr *     GetPrevRealInstr() const;
     IR::Instr *     GetPrevRealInstrOrLabel() const;
     IR::Instr *     GetInsertBeforeByteCodeUsesInstr();
+    IR::ByteCodeUsesInstr * GetFirstByteCodeUsesInstrBackward();
+    bool            IsByteCodeUsesInstrFor(IR::Instr * instr) const;
     IR::LabelInstr *GetOrCreateContinueLabel(const bool isHelper = false);
     static bool     HasSymUseSrc(StackSym *sym, IR::Opnd*);
     static bool     HasSymUseDst(StackSym *sym, IR::Opnd*);
     bool            HasSymUse(StackSym *sym);
     static bool     HasSymUseInRange(StackSym *sym, Instr *instrBegin, Instr *instrEnd);
     RegOpnd *       FindRegDef(StackSym *sym);
     static Instr*   FindSingleDefInstr(Js::OpCode opCode, Opnd* src);
+    bool            CanAggregateByteCodeUsesAcrossInstr(IR::Instr * instr);
 
     BranchInstr *   ChangeCmCCToBranchInstr(LabelInstr *targetInstr);
     static void     MoveRangeAfter(Instr * instrStart, Instr * instrLast, Instr * instrAfter);
@@ -466,6 +469,7 @@ class Instr
     bool       UsesAllFields();
     void       MoveArgs(bool generateByteCodeCapture = false);
     void       Move(IR::Instr* insertInstr);
+    void       AggregateByteCodeUses();
 private:
     void            ClearNumber() { this->m_number = 0; }
     void            SetNumber(uint32 number);
@@ -571,7 +575,11 @@ class ByteCodeUsesInstr : public Instr
     // a compare, but still need to generate them for bailouts. Without this, we cause
     // problems because we end up with an instruction losing atomicity in terms of its
     // bytecode use and generation lifetimes.
-    void Aggregate();
+    void AggregateFollowingByteCodeUses();
+    void AggregatePrecedingByteCodeUses();
+
+private:
+    void Aggregate(ByteCodeUsesInstr * byteCodeUsesInstr);
 };
 
 class JitProfilingInstr : public Instr