Jelajahi Sumber

[MERGE #2593 @meg-gupta] Check validity of memop on newly added typspec instructions

Merge pull request #2593 from meg-gupta:memopfix

When floattypespec is turned off either explicitly or via profile info, an
array copy loop may look like this in the IR :

    s32[LikelyCanBeTaggedValue_Int].var = LdElemI_A [s18[NativeFloatArray_NoMissingValues][seg: s50][segLen: s51][><].var+s48(s23).i32].var #00da  Bailout: #00e0 (BailOutOnImplicitCalls)
    s47(s32).f64    =  FromVar s32[LikelyCanBeTaggedValue_Int].var     #00e0  Bailout: #00e0 (BailOutNumberOnly)
    NoImplicitCallUses s18[NativeFloatArray_NoMissingValues][seg: s50][segLen: s51][><].var #00e0
    ByteCodeUses   s23, s32 #00e0
    [s18[NativeFloatArray_NoMissingValues][seg: s50][segLen: s51][><].var+s48(s23).u32].var = StElemI_A s47(s32).f64!  #00e0  Bailout: #00e0 (BailOutConventionalNativeArrayAccessOnly | BailOutOnArrayAccessHelperCall

The memcopy optimization can replace LdElem and StElem with a MemCopy. The
DeadStore pass is expected to cleanup rest of the loop. During deadstore,
the byteCodeUses instructions get removed and recorded. However FromVar
cannot be removed because it has a non dead storable bailout kind. The
FromVar instr was added during typespec and we need to check the validity
of memop of the current loop with the addition of these new instructions.
Meghana Gupta 9 tahun lalu
induk
melakukan
2c363b560b
2 mengubah file dengan 24 tambahan dan 18 penghapusan
  1. 23 17
      lib/Backend/GlobOpt.cpp
  2. 1 1
      lib/Backend/GlobOpt.h

+ 23 - 17
lib/Backend/GlobOpt.cpp

@@ -4694,7 +4694,7 @@ GlobOpt::CollectMemOpStElementI(IR::Instr *instr, Loop *loop)
 }
 
 bool
-GlobOpt::CollectMemOpInfo(IR::Instr *instr, Value *src1Val, Value *src2Val)
+GlobOpt::CollectMemOpInfo(IR::Instr *instrBegin, IR::Instr *instr, Value *src1Val, Value *src2Val)
 {
     Assert(this->currentBlock->loop);
 
@@ -4715,6 +4715,7 @@ GlobOpt::CollectMemOpInfo(IR::Instr *instr, Value *src1Val, Value *src2Val)
     Assert(loop->doMemOp);
 
     bool isIncr = true, isChangedByOne = false;
+
     switch (instr->m_opcode)
     {
     case Js::OpCode::StElemI_A:
@@ -4820,30 +4821,34 @@ MemOpCheckInductionVariable:
         // Fallthrough if not an induction variable
     }
     default:
-        if (IsInstrInvalidForMemOp(instr, loop, src1Val, src2Val))
+        FOREACH_INSTR_IN_RANGE(chkInstr, instrBegin->m_next, instr)
         {
-            loop->doMemOp = false;
-            return false;
-        }
+            if (IsInstrInvalidForMemOp(chkInstr, loop, src1Val, src2Val))
+            {
+                loop->doMemOp = false;
+                return false;
+            }
 
-        // Make sure this instruction doesn't use the memcopy transfer sym before it is checked by StElemI
-        if (loop->memOpInfo && !loop->memOpInfo->candidates->Empty())
-        {
-            Loop::MemOpCandidate* prevCandidate = loop->memOpInfo->candidates->Head();
-            if (prevCandidate->IsMemCopy())
+            // Make sure this instruction doesn't use the memcopy transfer sym before it is checked by StElemI
+            if (loop->memOpInfo && !loop->memOpInfo->candidates->Empty())
             {
-                Loop::MemCopyCandidate* memcopyCandidate = prevCandidate->AsMemCopy();
-                if (memcopyCandidate->base == Js::Constants::InvalidSymID)
+                Loop::MemOpCandidate* prevCandidate = loop->memOpInfo->candidates->Head();
+                if (prevCandidate->IsMemCopy())
                 {
-                    if (instr->FindRegUse(memcopyCandidate->transferSym))
+                    Loop::MemCopyCandidate* memcopyCandidate = prevCandidate->AsMemCopy();
+                    if (memcopyCandidate->base == Js::Constants::InvalidSymID)
                     {
-                        loop->doMemOp = false;
-                        TRACE_MEMOP_PHASE_VERBOSE(MemCopy, loop, instr, _u("Found illegal use of LdElemI value(s%d)"), GetVarSymID(memcopyCandidate->transferSym));
-                        return false;
+                        if (chkInstr->FindRegUse(memcopyCandidate->transferSym))
+                        {
+                            loop->doMemOp = false;
+                            TRACE_MEMOP_PHASE_VERBOSE(MemCopy, loop, chkInstr, _u("Found illegal use of LdElemI value(s%d)"), GetVarSymID(memcopyCandidate->transferSym));
+                            return false;
+                        }
                     }
                 }
             }
         }
+        NEXT_INSTR_IN_RANGE;
     }
 
     return true;
@@ -4888,6 +4893,7 @@ GlobOpt::IsInstrInvalidForMemOp(IR::Instr *instr, Loop *loop, Value *src1Val, Va
         TRACE_MEMOP_VERBOSE(loop, instr, _u("Implicit call bailout detected"));
         return true;
     }
+
     return false;
 }
 
@@ -5218,7 +5224,7 @@ GlobOpt::OptInstr(IR::Instr *&instr, bool* isInstrRemoved)
         (func->HasProfileInfo() && !func->GetReadOnlyProfileInfo()->IsMemOpDisabled()) &&
         this->currentBlock->loop->doMemOp)
     {
-        CollectMemOpInfo(instr, src1Val, src2Val);
+        CollectMemOpInfo(instrPrev, instr, src1Val, src2Val);
     }
 
     InsertNoImplicitCallUses(instr);

+ 1 - 1
lib/Backend/GlobOpt.h

@@ -1513,7 +1513,7 @@ private:
     ValueType               GetDivValueType(IR::Instr* instr, Value* src1Val, Value* src2Val, bool specialize);
 
     bool                    IsInstrInvalidForMemOp(IR::Instr *, Loop *, Value *, Value *);
-    bool                    CollectMemOpInfo(IR::Instr *, Value *, Value *);
+    bool                    CollectMemOpInfo(IR::Instr *, IR::Instr *, Value *, Value *);
     bool                    CollectMemOpStElementI(IR::Instr *, Loop *);
     bool                    CollectMemsetStElementI(IR::Instr *, Loop *);
     bool                    CollectMemcopyStElementI(IR::Instr *, Loop *);