Bug 1688033: Add support for LoadArgumentsObjArg r=jandem
☠☠ backed out by a5f845544279 ☠ ☠
authorIain Ireland <iireland@mozilla.com>
Wed, 27 Jan 2021 18:50:06 +0000
changeset 565526 ee7b88395348b5850a5340610cd525201b5fcadb
parent 565525 2205f04dd8bafe579d91cc7322a728b4b0e2448d
child 565527 35053583b9a751daed53f428f12219425a84004c
push id38161
push userabutkovits@mozilla.com
push dateTue, 02 Feb 2021 03:35:00 +0000
treeherdermozilla-central@babdc3b3a300 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1688033
milestone87.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1688033: Add support for LoadArgumentsObjArg r=jandem This adds support for `arguments[i]`. It is based directly on WarpCacheIRTranspiler::emitLoadFrameArgumentResult. If this bounds check fails, then baseline will attach a new stub in tryAttachGenericElement, preventing a bailout loop. We could also disable scalar replacement of arguments immediately, using a special BailoutKind. Depends on D103111 Differential Revision: https://phabricator.services.mozilla.com/D103112
js/src/jit/ScalarReplacement.cpp
--- a/js/src/jit/ScalarReplacement.cpp
+++ b/js/src/jit/ScalarReplacement.cpp
@@ -1247,16 +1247,17 @@ static bool IsArgumentsObjectEscaped(MIn
           return true;
         }
         break;
       }
 
       // This is a replaceable consumer.
       case MDefinition::Opcode::ArgumentsObjectLength:
       case MDefinition::Opcode::GetArgumentsObjectArg:
+      case MDefinition::Opcode::LoadArgumentsObjectArg:
         break;
 
       default:
         JitSpewDef(JitSpew_Escape, "is escaped by\n", def);
         return true;
     }
   }
 
@@ -1269,16 +1270,17 @@ class ArgumentsReplacer : public MDefini
   MIRGenerator* mir_;
   MIRGraph& graph_;
   MInstruction* args_;
 
   TempAllocator& alloc() { return graph_.alloc(); }
 
   void visitGuardToClass(MGuardToClass* ins);
   void visitGetArgumentsObjectArg(MGetArgumentsObjectArg* ins);
+  void visitLoadArgumentsObjectArg(MLoadArgumentsObjectArg* ins);
   void visitArgumentsObjectLength(MArgumentsObjectLength* ins);
 
  public:
   ArgumentsReplacer(MIRGenerator* mir, MIRGraph& graph, MInstruction* args)
       : mir_(mir), graph_(graph), args_(args) {
     MOZ_ASSERT(IsOptimizableArgumentsInstruction(args_));
   }
 
@@ -1351,16 +1353,47 @@ void ArgumentsReplacer::visitGetArgument
   auto* loadArg = MGetFrameArgument::New(alloc(), index);
   ins->block()->insertBefore(ins, loadArg);
   ins->replaceAllUsesWith(loadArg);
 
   // Remove original instruction.
   ins->block()->discard(ins);
 }
 
+void ArgumentsReplacer::visitLoadArgumentsObjectArg(
+    MLoadArgumentsObjectArg* ins) {
+  // There is only one possible arguments object.
+  // TODO: support inlining.
+  MOZ_ASSERT(ins->getArgsObject() == args_);
+
+  MDefinition* index = ins->index();
+
+  // Insert bounds check.
+  auto* length = MArgumentsLength::New(alloc());
+  ins->block()->insertBefore(ins, length);
+
+  MInstruction* check = MBoundsCheck::New(alloc(), index, length);
+  ins->block()->insertBefore(ins, check);
+
+  // TODO: Set special bailout kind?
+  check->setBailoutKind(ins->bailoutKind());
+
+  if (JitOptions.spectreIndexMasking) {
+    check = MSpectreMaskIndex::New(alloc(), check, length);
+    ins->block()->insertBefore(ins, check);
+  }
+
+  auto* loadArg = MGetFrameArgument::New(alloc(), check);
+  ins->block()->insertBefore(ins, loadArg);
+  ins->replaceAllUsesWith(loadArg);
+
+  // Remove original instruction.
+  ins->block()->discard(ins);
+}
+
 void ArgumentsReplacer::visitArgumentsObjectLength(
     MArgumentsObjectLength* ins) {
   // There is only one possible arguments object.
   // TODO: support inlining.
   MOZ_ASSERT(ins->getArgsObject() == args_);
 
   auto* length = MArgumentsLength::New(alloc());
   ins->block()->insertBefore(ins, length);