Bug 1656351 - Add Inline Cache support for CheckPrivateField r=jandem

Differential Revision: https://phabricator.services.mozilla.com/D85474
iaxat · Aug 4, 2020 · 924ae3e · 924ae3e
1 parent fee9617
commit 924ae3e
Show file tree

Hide file tree

Showing 16 changed files with 236 additions and 69 deletions.
diff --git a/js/src/jit-test/tests/fields/private-field-basics.js b/js/src/jit-test/tests/fields/private-field-basics.js
@@ -224,7 +224,7 @@ for (var index in elements) {
 
 // Megamorphic Cache Testing:
 for (var i = 0; i < 100; i++) {
-  var inputs = [{a: 1}, {b: 2}, {c: 3}, {d: 4}, {e: 5}];
+  var inputs = [{a: 1}, {b: 2}, {c: 3}, {d: 4}, {e: 5}, new Proxy({}, {})];
   for (var o of inputs) {
     assertThrows(() => B.gx(o), TypeError);
     assertThrows(() => B.sx(o), TypeError);

diff --git a/js/src/jit/BaselineCacheIRCompiler.cpp b/js/src/jit/BaselineCacheIRCompiler.cpp
@@ -2161,6 +2161,7 @@ bool BaselineCacheIRCompiler::init(CacheKind kind) {
     case CacheKind::SetProp:
     case CacheKind::In:
     case CacheKind::HasOwn:
+    case CacheKind::CheckPrivateField:
     case CacheKind::InstanceOf:
     case CacheKind::BinaryArith:
       MOZ_ASSERT(numInputs == 2);

diff --git a/js/src/jit/BaselineCodeGen.cpp b/js/src/jit/BaselineCodeGen.cpp
@@ -3288,23 +3288,11 @@ bool BaselineCodeGen<Handler>::emit_CheckPrivateField() {
   masm.loadValue(frame.addressOfStackValue(-2), R0);
   masm.loadValue(frame.addressOfStackValue(-1), R1);
 
-  prepareVMCall();
-
-  // Key
-  pushArg(R1);
-  // val
-  pushArg(R0);
-  // pc
-  pushBytecodePCArg();
-
-  using Fn = bool (*)(JSContext*, jsbytecode*, HandleValue, HandleValue, bool*);
-  if (!callVM<Fn, CheckPrivateFieldOperation>()) {
+  if (!emitNextIC()) {
     return false;
   }
 
-  masm.boxNonDouble(JSVAL_TYPE_BOOLEAN, ReturnReg, R0);
   frame.push(R0);
-
   return true;
 }
 

diff --git a/js/src/jit/BaselineIC.cpp b/js/src/jit/BaselineIC.cpp
@@ -397,6 +397,14 @@ bool ICScript::initICEntries(JSContext* cx, JSScript* script) {
         }
         break;
       }
+      case JSOp::CheckPrivateField: {
+        ICStub* stub = alloc.newStub<ICCheckPrivateField_Fallback>(
+            Kind::CheckPrivateField);
+        if (!addIC(loc, stub)) {
+          return false;
+        }
+        break;
+      }
       case JSOp::GetName:
       case JSOp::GetGName: {
         ICStub* stub = alloc.newStub<ICGetName_Fallback>(Kind::GetName);
@@ -2351,6 +2359,53 @@ bool FallbackICCodeCompiler::emit_HasOwn() {
   return tailCallVM<Fn, DoHasOwnFallback>(masm);
 }
 
+//
+// CheckPrivate_Fallback
+//
+
+bool DoCheckPrivateFieldFallback(JSContext* cx, BaselineFrame* frame,
+                                 ICCheckPrivateField_Fallback* stub,
+                                 HandleValue objValue, HandleValue keyValue,
+                                 MutableHandleValue res) {
+  stub->incrementEnteredCount();
+  RootedScript script(cx, frame->script());
+  jsbytecode* pc = stub->icEntry()->pc(script);
+
+  FallbackICSpew(cx, stub, "CheckPrivateField");
+
+  MOZ_ASSERT(keyValue.isSymbol() && keyValue.toSymbol()->isPrivateName());
+
+  TryAttachStub<CheckPrivateFieldIRGenerator>(
+      "CheckPrivate", cx, frame, stub, BaselineCacheIRStubKind::Regular,
+      CacheKind::CheckPrivateField, keyValue, objValue);
+
+  bool result;
+  if (!CheckPrivateFieldOperation(cx, pc, objValue, keyValue, &result)) {
+    return false;
+  }
+
+  res.setBoolean(result);
+  return true;
+}
+
+bool FallbackICCodeCompiler::emit_CheckPrivateField() {
+  EmitRestoreTailCallReg(masm);
+
+  // Sync for the decompiler.
+  masm.pushValue(R0);
+  masm.pushValue(R1);
+
+  // Push arguments.
+  masm.pushValue(R1);
+  masm.pushValue(R0);
+  masm.push(ICStubReg);
+  pushStubPayload(masm, R0.scratchReg());
+
+  using Fn = bool (*)(JSContext*, BaselineFrame*, ICCheckPrivateField_Fallback*,
+                      HandleValue, HandleValue, MutableHandleValue);
+  return tailCallVM<Fn, DoCheckPrivateFieldFallback>(masm);
+}
+
 //
 // GetName_Fallback
 //

diff --git a/js/src/jit/BaselineIC.h b/js/src/jit/BaselineIC.h
@@ -1558,6 +1558,15 @@ class ICHasOwn_Fallback : public ICFallbackStub {
       : ICFallbackStub(ICStub::HasOwn_Fallback, stubCode) {}
 };
 
+// CheckPrivateField
+//      JSOp::CheckPrivateField
+class ICCheckPrivateField_Fallback : public ICFallbackStub {
+  friend class ICStubSpace;
+
+  explicit ICCheckPrivateField_Fallback(TrampolinePtr stubCode)
+      : ICFallbackStub(ICStub::CheckPrivateField_Fallback, stubCode) {}
+};
+
 // GetName
 //      JSOp::GetName
 //      JSOp::GetGName
@@ -1864,6 +1873,12 @@ extern bool DoHasOwnFallback(JSContext* cx, BaselineFrame* frame,
                              ICHasOwn_Fallback* stub, HandleValue keyValue,
                              HandleValue objValue, MutableHandleValue res);
 
+extern bool DoCheckPrivateFieldFallback(JSContext* cx, BaselineFrame* frame,
+                                        ICCheckPrivateField_Fallback* stub,
+                                        HandleValue objValue,
+                                        HandleValue keyValue,
+                                        MutableHandleValue res);
+
 extern bool DoGetNameFallback(JSContext* cx, BaselineFrame* frame,
                               ICGetName_Fallback* stub, HandleObject envChain,
                               MutableHandleValue res);

diff --git a/js/src/jit/BaselineICList.h b/js/src/jit/BaselineICList.h
@@ -39,6 +39,7 @@ namespace jit {
                                       \
   _(In_Fallback)                      \
   _(HasOwn_Fallback)                  \
+  _(CheckPrivateField_Fallback)       \
                                       \
   _(GetName_Fallback)                 \
                                       \
@@ -89,6 +90,7 @@ namespace jit {
   _(SetElem)                                   \
   _(In)                                        \
   _(HasOwn)                                    \
+  _(CheckPrivateField)                         \
   _(GetName)                                   \
   _(BindName)                                  \
   _(GetIntrinsic)                              \

diff --git a/js/src/jit/CacheIR.cpp b/js/src/jit/CacheIR.cpp
@@ -20,8 +20,10 @@
 #include "js/friend/WindowProxy.h"  // js::IsWindow, js::IsWindowProxy, js::ToWindowIfWindowProxy
 #include "js/ScalarType.h"  // js::Scalar::Type
 #include "util/Unicode.h"
+#include "vm/BytecodeUtil.h"
 #include "vm/PlainObject.h"  // js::PlainObject
 #include "vm/SelfHosting.h"
+#include "vm/ThrowMsgKind.h"  // ThrowCondition
 
 #include "jit/MacroAssembler-inl.h"
 #include "vm/EnvironmentObject-inl.h"
@@ -84,6 +86,7 @@ size_t js::jit::NumInputsForCacheKind(CacheKind kind) {
     case CacheKind::SetProp:
     case CacheKind::In:
     case CacheKind::HasOwn:
+    case CacheKind::CheckPrivateField:
     case CacheKind::InstanceOf:
     case CacheKind::BinaryArith:
       return 2;
@@ -3350,6 +3353,75 @@ void HasPropIRGenerator::trackAttached(const char* name) {
 #endif
 }
 
+CheckPrivateFieldIRGenerator::CheckPrivateFieldIRGenerator(
+    JSContext* cx, HandleScript script, jsbytecode* pc, ICState::Mode mode,
+    CacheKind cacheKind, HandleValue idVal, HandleValue val)
+    : IRGenerator(cx, script, pc, cacheKind, mode), val_(val), idVal_(idVal) {
+  MOZ_ASSERT(idVal.isSymbol() && idVal.toSymbol()->isPrivateName());
+}
+
+AttachDecision CheckPrivateFieldIRGenerator::tryAttachStub() {
+  AutoAssertNoPendingException aanpe(cx_);
+
+  ValOperandId valId(writer.setInputOperandId(0));
+  ValOperandId keyId(writer.setInputOperandId(1));
+
+  if (!val_.isObject()) {
+    trackAttached(IRGenerator::NotAttached);
+    return AttachDecision::NoAction;
+  }
+  RootedObject obj(cx_, &val_.toObject());
+  ObjOperandId objId = writer.guardToObject(valId);
+  RootedId key(cx_, SYMBOL_TO_JSID(idVal_.toSymbol()));
+
+  ThrowCondition condition;
+  ThrowMsgKind msgKind;
+  GetCheckPrivateFieldOperands(pc_, &condition, &msgKind);
+
+  bool hasOwn = false;
+  if (!HasOwnDataPropertyPure(cx_, obj, key, &hasOwn)) {
+    // Can't determine if HasOwnProperty purely.
+    return AttachDecision::NoAction;
+  }
+
+  if (CheckPrivateFieldWillThrow(condition, hasOwn)) {
+    // Don't attach a stub if the operation will throw.
+    return AttachDecision::NoAction;
+  }
+
+  TRY_ATTACH(tryAttachNative(obj, objId, key, keyId, hasOwn));
+
+  return AttachDecision::NoAction;
+}
+
+AttachDecision CheckPrivateFieldIRGenerator::tryAttachNative(JSObject* obj,
+                                                             ObjOperandId objId,
+                                                             jsid key,
+                                                             ValOperandId keyId,
+                                                             bool hasOwn) {
+  if (!obj->isNative()) {
+    return AttachDecision::NoAction;
+  }
+
+  Maybe<ObjOperandId> tempId;
+  emitIdGuard(keyId, key);
+  EmitReadSlotGuard(writer, obj, obj, objId, &tempId);
+  writer.loadBooleanResult(hasOwn);
+  writer.returnFromIC();
+
+  trackAttached("NativeCheckPrivateField");
+  return AttachDecision::Attach;
+}
+
+void CheckPrivateFieldIRGenerator::trackAttached(const char* name) {
+#ifdef JS_CACHEIR_SPEW
+  if (const CacheIRSpewer::Guard& sp = CacheIRSpewer::Guard(*this, name)) {
+    sp.valueProperty("base", val_);
+    sp.valueProperty("property", idVal_);
+  }
+#endif
+}
+
 bool IRGenerator::maybeGuardInt32Index(const Value& index, ValOperandId indexId,
                                        uint32_t* int32Index,
                                        Int32OperandId* int32IndexId) {

diff --git a/js/src/jit/CacheIR.h b/js/src/jit/CacheIR.h
@@ -175,6 +175,7 @@ class TypedOperandId : public OperandId {
   _(BindName)             \
   _(In)                   \
   _(HasOwn)               \
+  _(CheckPrivateField)    \
   _(TypeOf)               \
   _(ToPropertyKey)        \
   _(InstanceOf)           \
@@ -1531,6 +1532,23 @@ class MOZ_RAII HasPropIRGenerator : public IRGenerator {
   AttachDecision tryAttachStub();
 };
 
+class MOZ_RAII CheckPrivateFieldIRGenerator : public IRGenerator {
+  HandleValue val_;
+  HandleValue idVal_;
+
+  AttachDecision tryAttachNative(JSObject* obj, ObjOperandId objId, jsid key,
+                                 ValOperandId keyId, bool hasOwn);
+
+  void trackAttached(const char* name);
+
+ public:
+  CheckPrivateFieldIRGenerator(JSContext* cx, HandleScript script,
+                               jsbytecode* pc, ICState::Mode mode,
+                               CacheKind cacheKind, HandleValue idVal,
+                               HandleValue val);
+  AttachDecision tryAttachStub();
+};
+
 class MOZ_RAII InstanceOfIRGenerator : public IRGenerator {
   HandleValue lhsVal_;
   HandleObject rhsObj_;

diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp
@@ -883,6 +883,7 @@ void CodeGenerator::visitOutOfLineICFallback(OutOfLineICFallback* ool) {
     case CacheKind::ToBool:
     case CacheKind::GetIntrinsic:
     case CacheKind::NewObject:
+    case CacheKind::CheckPrivateField:
       MOZ_CRASH("Unsupported IC");
   }
   MOZ_CRASH();

diff --git a/js/src/jit/IonCacheIRCompiler.cpp b/js/src/jit/IonCacheIRCompiler.cpp
@@ -510,6 +510,7 @@ bool IonCacheIRCompiler::init() {
     case CacheKind::ToBool:
     case CacheKind::GetIntrinsic:
     case CacheKind::NewObject:
+    case CacheKind::CheckPrivateField:
       MOZ_CRASH("Unsupported IC");
   }
 
@@ -592,6 +593,7 @@ void IonCacheIRCompiler::assertFloatRegisterAvailable(FloatRegister reg) {
     case CacheKind::GetIterator:
     case CacheKind::In:
     case CacheKind::HasOwn:
+    case CacheKind::CheckPrivateField:
     case CacheKind::InstanceOf:
     case CacheKind::UnaryArith:
     case CacheKind::ToPropertyKey:

diff --git a/js/src/jit/IonIC.cpp b/js/src/jit/IonIC.cpp
@@ -73,6 +73,7 @@ Register IonIC::scratchRegisterForEntryJump() {
     case CacheKind::ToBool:
     case CacheKind::GetIntrinsic:
     case CacheKind::NewObject:
+    case CacheKind::CheckPrivateField:
       MOZ_CRASH("Unsupported IC");
   }
 

diff --git a/js/src/jit/VMFunctionList-inl.h b/js/src/jit/VMFunctionList-inl.h
@@ -280,30 +280,33 @@ namespace jit {
 // The list below is for tail calls. The third argument specifies the number of
 // non-argument Values the VM wrapper should pop from the stack. This is used
 // for Baseline ICs.
-#define TAIL_CALL_VMFUNCTION_LIST(_)                              \
-  _(DoBinaryArithFallback, js::jit::DoBinaryArithFallback, 2)     \
-  _(DoBindNameFallback, js::jit::DoBindNameFallback, 0)           \
-  _(DoCompareFallback, js::jit::DoCompareFallback, 2)             \
-  _(DoConcatStringObject, js::jit::DoConcatStringObject, 2)       \
-  _(DoGetElemFallback, js::jit::DoGetElemFallback, 2)             \
-  _(DoGetElemSuperFallback, js::jit::DoGetElemSuperFallback, 3)   \
-  _(DoGetIntrinsicFallback, js::jit::DoGetIntrinsicFallback, 0)   \
-  _(DoGetIteratorFallback, js::jit::DoGetIteratorFallback, 1)     \
-  _(DoGetNameFallback, js::jit::DoGetNameFallback, 0)             \
-  _(DoGetPropFallback, js::jit::DoGetPropFallback, 1)             \
-  _(DoGetPropSuperFallback, js::jit::DoGetPropSuperFallback, 0)   \
-  _(DoHasOwnFallback, js::jit::DoHasOwnFallback, 2)               \
-  _(DoInFallback, js::jit::DoInFallback, 2)                       \
-  _(DoInstanceOfFallback, js::jit::DoInstanceOfFallback, 2)       \
-  _(DoNewArrayFallback, js::jit::DoNewArrayFallback, 0)           \
-  _(DoNewObjectFallback, js::jit::DoNewObjectFallback, 0)         \
-  _(DoRestFallback, js::jit::DoRestFallback, 0)                   \
-  _(DoSetElemFallback, js::jit::DoSetElemFallback, 2)             \
-  _(DoSetPropFallback, js::jit::DoSetPropFallback, 1)             \
-  _(DoToBoolFallback, js::jit::DoToBoolFallback, 0)               \
-  _(DoToPropertyKeyFallback, js::jit::DoToPropertyKeyFallback, 0) \
-  _(DoTypeMonitorFallback, js::jit::DoTypeMonitorFallback, 0)     \
-  _(DoTypeOfFallback, js::jit::DoTypeOfFallback, 0)               \
+//
+// This list is required to be alphabetized.
+#define TAIL_CALL_VMFUNCTION_LIST(_)                                      \
+  _(DoBinaryArithFallback, js::jit::DoBinaryArithFallback, 2)             \
+  _(DoBindNameFallback, js::jit::DoBindNameFallback, 0)                   \
+  _(DoCheckPrivateFieldFallback, js::jit::DoCheckPrivateFieldFallback, 2) \
+  _(DoCompareFallback, js::jit::DoCompareFallback, 2)                     \
+  _(DoConcatStringObject, js::jit::DoConcatStringObject, 2)               \
+  _(DoGetElemFallback, js::jit::DoGetElemFallback, 2)                     \
+  _(DoGetElemSuperFallback, js::jit::DoGetElemSuperFallback, 3)           \
+  _(DoGetIntrinsicFallback, js::jit::DoGetIntrinsicFallback, 0)           \
+  _(DoGetIteratorFallback, js::jit::DoGetIteratorFallback, 1)             \
+  _(DoGetNameFallback, js::jit::DoGetNameFallback, 0)                     \
+  _(DoGetPropFallback, js::jit::DoGetPropFallback, 1)                     \
+  _(DoGetPropSuperFallback, js::jit::DoGetPropSuperFallback, 0)           \
+  _(DoHasOwnFallback, js::jit::DoHasOwnFallback, 2)                       \
+  _(DoInFallback, js::jit::DoInFallback, 2)                               \
+  _(DoInstanceOfFallback, js::jit::DoInstanceOfFallback, 2)               \
+  _(DoNewArrayFallback, js::jit::DoNewArrayFallback, 0)                   \
+  _(DoNewObjectFallback, js::jit::DoNewObjectFallback, 0)                 \
+  _(DoRestFallback, js::jit::DoRestFallback, 0)                           \
+  _(DoSetElemFallback, js::jit::DoSetElemFallback, 2)                     \
+  _(DoSetPropFallback, js::jit::DoSetPropFallback, 1)                     \
+  _(DoToBoolFallback, js::jit::DoToBoolFallback, 0)                       \
+  _(DoToPropertyKeyFallback, js::jit::DoToPropertyKeyFallback, 0)         \
+  _(DoTypeMonitorFallback, js::jit::DoTypeMonitorFallback, 0)             \
+  _(DoTypeOfFallback, js::jit::DoTypeOfFallback, 0)                       \
   _(DoUnaryArithFallback, js::jit::DoUnaryArithFallback, 1)
 
 #define DEF_ID(name, ...) name,

diff --git a/js/src/jit/WarpBuilder.cpp b/js/src/jit/WarpBuilder.cpp
@@ -3079,6 +3079,7 @@ bool WarpBuilder::buildIC(BytecodeLocation loc, CacheKind kind,
     case CacheKind::TypeOf:
     case CacheKind::Call:
     case CacheKind::NewObject:
+    case CacheKind::CheckPrivateField:
       // We're currently not using an IC or transpiling CacheIR for these kinds.
       MOZ_CRASH("Unexpected kind");
   }
@@ -3120,6 +3121,7 @@ bool WarpBuilder::buildBailoutForColdIC(BytecodeLocation loc, CacheKind kind) {
     case CacheKind::Compare:
     case CacheKind::In:
     case CacheKind::HasOwn:
+    case CacheKind::CheckPrivateField:
     case CacheKind::InstanceOf:
       resultType = MIRType::Boolean;
       break;