[MERGE #5252 @rajatd] Fixing obj-ptr copy-prop in prepass. OS 17555305

Merge pull request #5252 from rajatd:ac-master

For copy-propping in prepass, it is not sufficient that the two syms are safe to tranfer values.  For instance, the copy sym may not be live on back-edge but, may become live on back edge after the copy-prop.

In addition to the safe-to-transfer checks, the copy-sym shouldn't be written to if the original sym is live on back edge. If, however, the original sym is not live on back edge, then we can do the copy prop, provided both symbols satisfied the safe-to-transfer checks.

Example:

    s1 = s2
    $Loop:
        s3 = s1  <-- can't copy-prop s2 here even though both s1 and s2 are safe to transfer
        s2 = s4
        Br $Loop

Author: rajatd

lib/Backend/GlobOpt.cpp

@@ -5560,6 +5560,30 @@ GlobOpt::IsSafeToTransferInPrepass(StackSym * const srcSym, ValueInfo *const src
         !currentBlock->loop->IsSymAssignedToInSelfOrParents(srcSym);
 }
 
+bool
+GlobOpt::SafeToCopyPropInPrepass(StackSym * const originalSym, StackSym * const copySym, Value *const value) const
+{
+    Assert(this->currentBlock->globOptData.GetCopyPropSym(originalSym, value) == copySym);
+
+    // In the following example, to copy-prop s2 into s1, it is not enough to check if s1 and s2 are safe to transfer.
+    // In fact, both s1 and s2 are safe to transfer, but it is not legal to copy prop s2 into s1. 
+    //
+    // s1 = s2
+    // $Loop:
+    //   s3 = s1
+    //   s2 = s4
+    //   Br $Loop
+    //
+    // In general, requirements for copy-propping in prepass are more restricted than those for transferring values.
+    // For copy prop in prepass, if the original sym is live on back-edge, then the copy-prop sym should not be written to
+    // in the loop (or its parents)
+    
+    ValueInfo* const valueInfo = value->GetValueInfo();
+    return IsSafeToTransferInPrepass(originalSym, valueInfo) &&
+        IsSafeToTransferInPrepass(copySym, valueInfo) &&
+        (!currentBlock->loop->regAlloc.liveOnBackEdgeSyms->Test(originalSym->m_id) || !currentBlock->loop->IsSymAssignedToInSelfOrParents(copySym));
+}
+
 Value *GlobOpt::CreateDstUntransferredIntValue(
     const int32 min,
     const int32 max,

lib/Backend/GlobOpt.h

@@ -565,6 +565,7 @@ class GlobOpt
     bool                    IsPrepassSrcValueInfoPrecise(IR::Opnd *const src, Value *const srcValue, bool * canTransferValueNumberToDst = nullptr) const;
     bool                    IsPrepassSrcValueInfoPrecise(IR::Instr *const instr, Value *const src1Value, Value *const src2Value, bool * canTransferValueNumberToDst = nullptr) const;
     bool                    IsSafeToTransferInPrepass(StackSym * const sym, ValueInfo *const srcValueInfo) const;
+    bool                    SafeToCopyPropInPrepass(StackSym * const originalSym, StackSym * const copySym, Value *const value) const;
     Value *                 CreateDstUntransferredIntValue(const int32 min, const int32 max, IR::Instr *const instr, Value *const src1Value, Value *const src2Value);
     Value *                 CreateDstUntransferredValue(const ValueType desiredValueType, IR::Instr *const instr, Value *const src1Value, Value *const src2Value);
     Value *                 ValueNumberTransferDst(IR::Instr *const instr, Value *src1Val);

lib/Backend/GlobOptFields.cpp

@@ -1710,8 +1710,7 @@ GlobOpt::CopyPropPropertySymObj(IR::SymOpnd *symOpnd, IR::Instr *instr)
             PropertySym *newProp = PropertySym::FindOrCreate(
                 copySym->m_id, propertySym->m_propertyId, propertySym->GetPropertyIdIndex(), propertySym->GetInlineCacheIndex(), propertySym->m_fieldKind, this->func);
 
-            if (!this->IsLoopPrePass() || 
-                 (IsSafeToTransferInPrepass(objSym, val->GetValueInfo()) && IsSafeToTransferInPrepass(copySym, val->GetValueInfo())))
+            if (!this->IsLoopPrePass() || SafeToCopyPropInPrepass(objSym, copySym, val))
             {
 #if DBG_DUMP
                 if (Js::Configuration::Global.flags.Trace.IsEnabled(Js::GlobOptPhase, this->func->GetSourceContextId(), this->func->GetLocalFunctionId()))

test/Optimizer/rlexe.xml

@@ -1488,6 +1488,12 @@
       <files>test150.js</files>
     </default>
   </test>
+  <test>
+    <default>
+      <files>test151.js</files>
+      <compile-flags>-off:usefixeddataprops -off:objtypespec</compile-flags>
+    </default>
+  </test>
   <test>
     <default>
       <files>IsIn_ArrayNoMissingValues.js</files>

test/Optimizer/test151.js

@@ -0,0 +1,30 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+function foo(n, a, o, i) {
+    
+    var g;
+    var k = n || i.tmpl;
+    for(var j = 0; j<a.length; j++)
+    {
+        g = o[j];
+        k.tmpls && g.tmpls;
+        k.tmpls[0];
+        n = g.props.tmpl;
+    }
+
+}
+
+n = {tmpls: [1,2,3,4,5]};
+a = [1,2];
+o = {};
+
+o[0] = {props: {tmpl: 10}, tmpls: [1,2,3,4,5]};
+o[1] = {props: {tmpl: 20}, tmpls: [1,2,3,4,5]};
+
+foo(n, a, o);
+foo(n, a, o);
+foo(n, a, o);
+print("passed");