Fix value propagation on loop back-edge with aggressive value transfers

When using aggressive value transfer in loop prepasses, the data flow
analysis can incorrectly determine that two syms share a value through
the entire loop. Ensure that all syms assigned to within a loop are
given unique value numbers when merging values from the back-edge.

lib/Backend/GlobOptBlockData.cpp

@@ -767,12 +767,23 @@ GlobOptBlockData::MergeValueMaps(
 
             if (iter2.IsValid() && bucket.value->m_id == iter2.Data().value->m_id)
             {
+                // Syms that are assigned to within the loop must have unique
+                // value numbers in the loop header after merging; a single
+                // prepass is not adequate to determine that sym values are
+                // equivalent through all possible loop paths.
+                bool forceUniqueValue =
+                    isLoopBackEdge &&
+                    !this->globOpt->IsLoopPrePass() &&
+                    loop &&
+                    loop->symsAssignedToInLoop->Test(bucket.value->m_id);
+
                 newValue =
                     this->MergeValues(
                         bucket.element,
                         iter2.Data().element,
                         iter2.Data().value,
                         isLoopBackEdge,
+                        forceUniqueValue,
                         symsRequiringCompensation,
                         symsCreatedForMerge);
             }
@@ -847,6 +858,7 @@ GlobOptBlockData::MergeValues(
     Value *fromDataValue,
     Sym *fromDataSym,
     bool isLoopBackEdge,
+    bool forceUniqueValue,
     BVSparse<JitArenaAllocator> *const symsRequiringCompensation,
     BVSparse<JitArenaAllocator> *const symsCreatedForMerge)
 {
@@ -879,22 +891,30 @@ GlobOptBlockData::MergeValues(
         return toDataValue;
     }
 
-    // There may be other syms in toData that haven't been merged yet, referring to the current toData value for this sym. If
-    // the merge produced a new value info, don't corrupt the value info for the other sym by changing the same value. Instead,
-    // create one value per source value number pair per merge and reuse that for new value infos.
-    Value *newValue = this->globOpt->valuesCreatedForMerge->Lookup(sourceValueNumberPair, nullptr);
-    if(newValue)
+    Value *newValue = nullptr;
+    if (forceUniqueValue)
     {
-        Assert(sameValueNumber == (newValue->GetValueNumber() == toDataValue->GetValueNumber()));
-
-        // This is an exception where Value::SetValueInfo is called directly instead of GlobOpt::ChangeValueInfo, because we're
-        // actually generating new value info through merges.
-        newValue->SetValueInfo(newValueInfo);
+        newValue = this->globOpt->NewValue(newValueInfo);
     }
     else
     {
-        newValue = this->globOpt->NewValue(sameValueNumber ? sourceValueNumberPair.First() : this->globOpt->NewValueNumber(), newValueInfo);
-        this->globOpt->valuesCreatedForMerge->Add(sourceValueNumberPair, newValue);
+        // There may be other syms in toData that haven't been merged yet, referring to the current toData value for this sym. If
+        // the merge produced a new value info, don't corrupt the value info for the other sym by changing the same value. Instead,
+        // create one value per source value number pair per merge and reuse that for new value infos.
+        newValue = this->globOpt->valuesCreatedForMerge->Lookup(sourceValueNumberPair, nullptr);
+        if (newValue)
+        {
+            Assert(sameValueNumber == (newValue->GetValueNumber() == toDataValue->GetValueNumber()));
+
+            // This is an exception where Value::SetValueInfo is called directly instead of GlobOpt::ChangeValueInfo, because we're
+            // actually generating new value info through merges.
+            newValue->SetValueInfo(newValueInfo);
+        }
+        else
+        {
+            newValue = this->globOpt->NewValue(sameValueNumber ? sourceValueNumberPair.First() : this->globOpt->NewValueNumber(), newValueInfo);
+            this->globOpt->valuesCreatedForMerge->Add(sourceValueNumberPair, newValue);
+        }
     }
 
     // Set symStore if same on both paths.

lib/Backend/GlobOptBlockData.h

@@ -261,7 +261,7 @@ private:
     template <typename CaptureList, typename CapturedItemsAreEqual>
     void                    MergeCapturedValues(SListBase<CaptureList>* toList, SListBase<CaptureList> * fromList, CapturedItemsAreEqual itemsAreEqual);
     void                    MergeValueMaps(BasicBlock *toBlock, BasicBlock *fromBlock, BVSparse<JitArenaAllocator> *const symsRequiringCompensation, BVSparse<JitArenaAllocator> *const symsCreatedForMerge);
-    Value *                 MergeValues(Value *toDataValue, Value *fromDataValue, Sym *fromDataSym, bool isLoopBackEdge, BVSparse<JitArenaAllocator> *const symsRequiringCompensation, BVSparse<JitArenaAllocator> *const symsCreatedForMerge);
+    Value *                 MergeValues(Value *toDataValue, Value *fromDataValue, Sym *fromDataSym, bool isLoopBackEdge, bool forceUniqueValue, BVSparse<JitArenaAllocator> *const symsRequiringCompensation, BVSparse<JitArenaAllocator> *const symsCreatedForMerge);
     ValueInfo *             MergeValueInfo(Value *toDataVal, Value *fromDataVal, Sym *fromDataSym, bool isLoopBackEdge, bool sameValueNumber, BVSparse<JitArenaAllocator> *const symsRequiringCompensation, BVSparse<JitArenaAllocator> *const symsCreatedForMerge);
     JsTypeValueInfo *       MergeJsTypeValueInfo(JsTypeValueInfo * toValueInfo, JsTypeValueInfo * fromValueInfo, bool isLoopBackEdge, bool sameValueNumber);
     ValueInfo *             MergeArrayValueInfo(const ValueType mergedValueType, const ArrayValueInfo *const toDataValueInfo, const ArrayValueInfo *const fromDataValueInfo, Sym *const arraySym, BVSparse<JitArenaAllocator> *const symsRequiringCompensation, BVSparse<JitArenaAllocator> *const symsCreatedForMerge, bool isLoopBackEdge);

test/Optimizer/PrePassEntanglement.js

@@ -0,0 +1,28 @@
+
+function test() {
+  var line = '"Value1InQuotes",Value2,Value 3 ,0.33,,,Last Value';
+  var inQuotes = false;
+  var quoted = false;
+  for (var i = 0; i < line.length; i++) {
+    if (inQuotes) {
+      if (line[i] === '"') {
+          inQuotes = false;
+      }
+    } else {
+      if (line[i] === '"') {
+        inQuotes = true;
+        quoted = true;
+      } else if (line[i] === ',') {
+        if (line[i - 1] === '"' && !quoted) {
+          WScript.Echo('Read from wrong var');
+          return false;
+        }
+      }
+    }
+  }
+  return true;
+}
+
+if (test() && test() && test()) {
+  WScript.Echo('Passed');
+}

test/Optimizer/rlexe.xml

@@ -1211,6 +1211,11 @@
       <files>PrePassValues.js</files>
     </default>
   </test>
+  <test>
+    <default>
+      <files>PrePassEntanglement.js</files>
+    </default>
+  </test>
   <test>
     <default>
       <files>missing_len.js</files>

test/PRE/pre1.baseline

@@ -19,7 +19,6 @@ testInlined
 TestTrace fieldcopyprop [in landing pad]: function inlinee ( (#1.3), #4) inlined caller function testInlined ( (#1.4), #5) opcode: LdRootFld field: Direction 
 TestTrace fieldcopyprop [in landing pad]: function inlinee ( (#1.3), #4) inlined caller function testInlined ( (#1.4), #5) opcode: LdRootFld field: Direction 
 TestTrace fieldcopyprop [in landing pad]: function inlinee ( (#1.3), #4) inlined caller function testInlined ( (#1.4), #5) opcode: LdFld field: FORWARD 
-TestTrace fieldcopyprop: function inlinee ( (#1.3), #4) inlined caller function testInlined ( (#1.4), #5) opcode: LdFld field: count 
 TestTrace fieldcopyprop: function inlinee ( (#1.3), #4) inlined caller function testInlined ( (#1.4), #5) opcode: LdRootFld field: Direction 
 TestTrace fieldcopyprop: function inlinee ( (#1.3), #4) inlined caller function testInlined ( (#1.4), #5) opcode: LdFld field: FORWARD 
 TestTrace fieldcopyprop: function inlinee ( (#1.3), #4) inlined caller function testInlined ( (#1.4), #5) opcode: LdFld field: count 
@@ -30,7 +29,6 @@ undefined
 TestTrace fieldcopyprop [in landing pad]: function inlinee ( (#1.3), #4) inlined caller function testInlined ( (#1.4), #5) opcode: LdRootFld field: Direction 
 TestTrace fieldcopyprop [in landing pad]: function inlinee ( (#1.3), #4) inlined caller function testInlined ( (#1.4), #5) opcode: LdRootFld field: Direction 
 TestTrace fieldcopyprop [in landing pad]: function inlinee ( (#1.3), #4) inlined caller function testInlined ( (#1.4), #5) opcode: LdFld field: FORWARD 
-TestTrace fieldcopyprop: function inlinee ( (#1.3), #4) inlined caller function testInlined ( (#1.4), #5) opcode: LdFld field: count 
 TestTrace fieldcopyprop: function inlinee ( (#1.3), #4) inlined caller function testInlined ( (#1.4), #5) opcode: LdRootFld field: Direction 
 TestTrace fieldcopyprop: function inlinee ( (#1.3), #4) inlined caller function testInlined ( (#1.4), #5) opcode: LdFld field: FORWARD 
 TestTrace fieldcopyprop: function inlinee ( (#1.3), #4) inlined caller function testInlined ( (#1.4), #5) opcode: LdFld field: count