Bug 1688033: Recover CreateArgumentsObject on bailout r=nbp

The original version of this patch only recovered CreateArgumentsObject on bailout if it didn't escape, but I removed that code based on discussion in #warpbuilder.

This code is tested by `/ion/dce-with-rinstructions.js`. Prior to this patch, it failed with `--scalar-replace-arguments`.

Differential Revision: https://phabricator.services.mozilla.com/D104489

js/src/jit-test/tests/ion/recover-arguments.js

@@ -0,0 +1,22 @@
+// |jit-test| --scalar-replace-arguments
+
+setJitCompilerOption("baseline.warmup.trigger", 9);
+setJitCompilerOption("ion.warmup.trigger", 20);
+
+// Prevent the GC from cancelling compilations, when we expect them to succeed.
+gczeal(0);
+
+function rcreate_arguments_object_nouse() {
+    assertRecoveredOnBailout(arguments, true);
+}
+
+function rcreate_arguments_object_oneuse() {
+  assertRecoveredOnBailout(arguments, true);
+  return arguments[0];
+}
+
+with ({}) {}
+for (var i = 0; i < 100; i++) {
+    rcreate_arguments_object_nouse();
+    rcreate_arguments_object_oneuse(0);
+}

js/src/jit/CompileInfo.h

@@ -303,10 +303,11 @@ class CompileInfo {
       return SlotObservableKind::NotObservable;
     }
 
-    // The arguments object is observable and not recoverable.
+    // The arguments object is observable. If it does not escape, it can
+    // be recovered.
     if (hasArguments() && slot == argsObjSlot()) {
       MOZ_ASSERT(funMaybeLazy());
-      return SlotObservableKind::ObservableNotRecoverable;
+      return SlotObservableKind::ObservableRecoverable;
     }
 
     MOZ_ASSERT(slot == returnValueSlot());

js/src/jit/JSJitFrameIter.h

@@ -467,7 +467,7 @@ class SnapshotIterator {
     return snapshot_.numAllocationsRead() < numAllocations();
   }
 
-  int32_t readOuterNumActualArgs() const;
+  JitFrameLayout* frame() { return fp_; };
 
   // Used by recover instruction to store the value back into the instruction
   // results array.

js/src/jit/JitFrames.cpp

@@ -1498,10 +1498,6 @@ SnapshotIterator::SnapshotIterator()
       ionScript_(nullptr),
       instructionResults_(nullptr) {}
 
-int32_t SnapshotIterator::readOuterNumActualArgs() const {
-  return fp_->numActualArgs();
-}
-
 uintptr_t SnapshotIterator::fromStack(int32_t offset) const {
   return ReadFrameSlot(fp_, offset);
 }

js/src/jit/MIR.h

@@ -3112,6 +3112,10 @@ class MCreateArgumentsObject : public MUnaryInstruction,
   AliasSet getAliasSet() const override { return AliasSet::None(); }
 
   bool possiblyCalls() const override { return true; }
+
+  [[nodiscard]] bool writeRecoverData(
+      CompactBufferWriter& writer) const override;
+  bool canRecoverOnBailout() const override { return true; }
 };
 
 class MGetArgumentsObjectArg : public MUnaryInstruction,

js/src/jit/Recover.cpp

@@ -887,7 +887,7 @@ RArgumentsLength::RArgumentsLength(CompactBufferReader& reader) {}
 bool RArgumentsLength::recover(JSContext* cx, SnapshotIterator& iter) const {
   RootedValue result(cx);
 
-  result.setInt32(iter.readOuterNumActualArgs());
+  result.setInt32(iter.frame()->numActualArgs());
 
   iter.storeInstructionResult(result);
   return true;
@@ -1947,3 +1947,25 @@ bool RBigIntAsUintN::recover(JSContext* cx, SnapshotIterator& iter) const {
   iter.storeInstructionResult(JS::BigIntValue(result));
   return true;
 }
+
+bool MCreateArgumentsObject::writeRecoverData(
+    CompactBufferWriter& writer) const {
+  MOZ_ASSERT(canRecoverOnBailout());
+  writer.writeUnsigned(uint32_t(RInstruction::Recover_CreateArgumentsObject));
+  return true;
+}
+
+RCreateArgumentsObject::RCreateArgumentsObject(CompactBufferReader& reader) {}
+
+bool RCreateArgumentsObject::recover(JSContext* cx,
+                                     SnapshotIterator& iter) const {
+  RootedObject callObject(cx, &iter.read().toObject());
+  RootedObject result(
+      cx, ArgumentsObject::createForIon(cx, iter.frame(), callObject));
+  if (!result) {
+    return false;
+  }
+
+  iter.storeInstructionResult(JS::ObjectValue(*result));
+  return true;
+}

js/src/jit/Recover.h

@@ -132,6 +132,7 @@ namespace jit {
   _(AtomicIsLockFree)          \
   _(BigIntAsIntN)              \
   _(BigIntAsUintN)             \
+  _(CreateArgumentsObject)     \
   _(AssertRecoveredOnBailout)
 
 class RResumePoint;
@@ -870,6 +871,14 @@ class RBigIntAsUintN final : public RInstruction {
                              SnapshotIterator& iter) const override;
 };
 
+class RCreateArgumentsObject final : public RInstruction {
+ public:
+  RINSTRUCTION_HEADER_NUM_OP_(CreateArgumentsObject, 1)
+
+  [[nodiscard]] bool recover(JSContext* cx,
+                             SnapshotIterator& iter) const override;
+};
+
 class RAssertRecoveredOnBailout final : public RInstruction {
  public:
   RINSTRUCTION_HEADER_NUM_OP_(AssertRecoveredOnBailout, 1)

js/src/jit/ScalarReplacement.cpp

@@ -1255,6 +1255,11 @@ static bool IsArgumentsObjectEscaped(MInstruction* ins) {
       case MDefinition::Opcode::LoadArgumentsObjectArg:
         break;
 
+      // This instruction is a no-op used to test that scalar replacement
+      // is working as expected.
+      case MDefinition::Opcode::AssertRecoveredOnBailout:
+        break;
+
       default:
         JitSpewDef(JitSpew_Escape, "is escaped by\n", def);
         return true;
@@ -1285,6 +1290,7 @@ class ArgumentsReplacer : public MDefinitionVisitorDefaultNoop {
   }
 
   bool run();
+  void assertSuccess();
 };
 
 // Replacing the arguments object is simpler than replacing an object
@@ -1320,10 +1326,15 @@ bool ArgumentsReplacer::run() {
     }
   }
 
-  MOZ_ASSERT(!args_->hasLiveDefUses());
+  assertSuccess();
   return true;
 }
 
+void ArgumentsReplacer::assertSuccess() {
+  MOZ_ASSERT(args_->canRecoverOnBailout());
+  MOZ_ASSERT(!args_->hasLiveDefUses());
+}
+
 void ArgumentsReplacer::visitGuardToClass(MGuardToClass* ins) {
   // Skip guards on other objects.
   if (ins->object() != args_) {