Fix issue with unsigned compare peep in dead code.

The unsigned compare peep optimization, after being changed to have
the bytecodeuses instruction emitted after the compare instead of before
it, had issues with lowering of code hidden behind a bailonnoprofile, in
that it would have conflicts on lifetimes due to the loss of atomicity
of the bytecodereg use and generation of the compare. This patch attempts
to restore that by aggregating the bytecodeuse information for a single
instruction into once place.

Author: Penguinwizzard

lib/Backend/GlobOptBailOut.cpp

@@ -1085,6 +1085,43 @@ GlobOpt::ConvertToByteCodeUses(IR::Instr * instr)
     this->CaptureByteCodeSymUses(instr);
     IR::ByteCodeUsesInstr * byteCodeUsesInstr = this->InsertByteCodeUses(instr, true);
     instr->Remove();
+    // If possible, we want to aggregate with subsequent ByteCodeUses Instructions, so
+    // that we can do some optimizations in other places where we can simplify args in
+    // 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.
+    if (byteCodeUsesInstr &&
+        byteCodeUsesInstr->m_next &&
+        byteCodeUsesInstr->m_next->m_opcode == Js::OpCode::ByteCodeUses &&
+        byteCodeUsesInstr->GetByteCodeOffset() == byteCodeUsesInstr->m_next->GetByteCodeOffset()
+        )
+    {
+        IR::ByteCodeUsesInstr * nextinstr = byteCodeUsesInstr->m_next->AsByteCodeUsesInstr();
+        if (nextinstr->GetDst() == nullptr)
+        {
+            // We move all of the uses of the next bytecodeuses instruction into this one. The
+            // instruction is notably not removed or repurposed; unfortunately, at this point,
+            // doing either would be somewhat complex. Removing the instruction breaks some of
+            // the assumptions made in other code (notably RemoveCodeAfterNoFallthroughInstr),
+            // and repurposing it by removing the newly-added byteCodeUsesInstr requires us to
+            // be able to handle transferring the Dst reg in a couple more cases (like if it's
+            // single-def).
+            if (nextinstr->byteCodeUpwardExposedUsed)
+            {
+                if (byteCodeUsesInstr->byteCodeUpwardExposedUsed)
+                {
+                    byteCodeUsesInstr->byteCodeUpwardExposedUsed->Or(nextinstr->byteCodeUpwardExposedUsed);
+                    JitAdelete(nextinstr->byteCodeUpwardExposedUsed->GetAllocator(), nextinstr->byteCodeUpwardExposedUsed);
+                    nextinstr->byteCodeUpwardExposedUsed = nullptr;
+                }
+                else
+                {
+                    byteCodeUsesInstr->byteCodeUpwardExposedUsed = nextinstr->byteCodeUpwardExposedUsed;
+                    nextinstr->byteCodeUpwardExposedUsed = nullptr;
+                }
+            }
+        }
+    }
     return byteCodeUsesInstr;
 }
 

test/Bugs/bug9080773_2.js

@@ -0,0 +1,15 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+function test0() {
+  var protoObj1 = {};
+  for (var __loopSecondaryVar1000_0 = 10; protoObj1.length; protoObj1) {
+    var v4 = prop0 >>> 0 < 0;
+  }
+}
+test0();
+test0();
+test0();
+
+WScript.Echo("PASSED");

test/Bugs/rlexe.xml

@@ -290,6 +290,12 @@
       <compile-flags>-maxinterpretcount:1 -off:simplejit</compile-flags>
     </default>
   </test>
+  <test>
+    <default>
+      <files>bug9080773_2.js</files>
+      <compile-flags>-maxinterpretcount:1 -maxsimplejitruncount:1</compile-flags>
+    </default>
+  </test>
   <test>
     <default>
       <files>b208_asmjs.js</files>