From edd79fcb431f62ace80e8e774c7cea9565eb4add Mon Sep 17 00:00:00 2001 From: Jan de Mooij Date: Fri, 22 Nov 2024 13:40:21 +0000 Subject: [PATCH] Bug 1932305 part 13 - Optimize calls to Map.prototype.delete and Set.prototype.delete. r=iain Differential Revision: https://phabricator.services.mozilla.com/D229613 --- js/src/builtin/MapObject.cpp | 4 +- .../jit-test/tests/cacheir/map-set-delete.js | 17 +++++ js/src/jit/CacheIR.cpp | 66 +++++++++++++++++++ js/src/jit/CacheIRCompiler.cpp | 36 ++++++++++ js/src/jit/CacheIRGenerator.h | 2 + js/src/jit/CacheIROps.yaml | 16 +++++ js/src/jit/CodeGenerator.cpp | 14 ++++ js/src/jit/InlinableNatives.cpp | 2 + js/src/jit/InlinableNatives.h | 2 + js/src/jit/Lowering.cpp | 14 ++++ js/src/jit/MIROps.yaml | 16 +++++ js/src/jit/VMFunctionList-inl.h | 2 + js/src/jit/VMFunctions.cpp | 10 +++ js/src/jit/VMFunctions.h | 4 ++ js/src/jit/WarpCacheIRTranspiler.cpp | 24 +++++++ 15 files changed, 227 insertions(+), 2 deletions(-) create mode 100644 js/src/jit-test/tests/cacheir/map-set-delete.js diff --git a/js/src/builtin/MapObject.cpp b/js/src/builtin/MapObject.cpp index 12a7de8ec1cb..b8ded0ecfddb 100644 --- a/js/src/builtin/MapObject.cpp +++ b/js/src/builtin/MapObject.cpp @@ -433,7 +433,7 @@ const JSFunctionSpec MapObject::methods[] = { JS_INLINABLE_FN("get", get, 1, 0, MapGet), JS_INLINABLE_FN("has", has, 1, 0, MapHas), JS_INLINABLE_FN("set", set, 2, 0, MapSet), - JS_FN("delete", delete_, 1, 0), + JS_INLINABLE_FN("delete", delete_, 1, 0, MapDelete), JS_FN("keys", keys, 0, 0), JS_FN("values", values, 0, 0), JS_FN("clear", clear, 0, 0), @@ -1192,7 +1192,7 @@ const JSPropertySpec SetObject::properties[] = { const JSFunctionSpec SetObject::methods[] = { JS_INLINABLE_FN("has", has, 1, 0, SetHas), JS_INLINABLE_FN("add", add, 1, 0, SetAdd), - JS_FN("delete", delete_, 1, 0), + JS_INLINABLE_FN("delete", delete_, 1, 0, SetDelete), JS_FN("entries", entries, 0, 0), JS_FN("clear", clear, 0, 0), JS_SELF_HOSTED_FN("forEach", "SetForEach", 2, 0), diff --git a/js/src/jit-test/tests/cacheir/map-set-delete.js b/js/src/jit-test/tests/cacheir/map-set-delete.js new file mode 100644 index 000000000000..66eb792452a6 --- /dev/null +++ b/js/src/jit-test/tests/cacheir/map-set-delete.js @@ -0,0 +1,17 @@ +function test() { + var m = new Map(); + var s = new Set(); + for (var i = 0; i < 100; i++) { + assertEq(m.delete("a" + (i - 1)), i > 0); + assertEq(m.delete("a" + (i - 1)), false); + + assertEq(s.delete("b" + (i - 1)), i > 0); + assertEq(s.delete("b" + (i - 1)), false); + + m.set("a" + i, i); + s.add("b" + i); + } + assertEq(m.size, 1); + assertEq(s.size, 1); +} +test(); diff --git a/js/src/jit/CacheIR.cpp b/js/src/jit/CacheIR.cpp index 3612add36c5c..75ee60b33f61 100644 --- a/js/src/jit/CacheIR.cpp +++ b/js/src/jit/CacheIR.cpp @@ -10242,6 +10242,37 @@ AttachDecision InlinableNativeIRGenerator::tryAttachSetHas() { return AttachDecision::Attach; } +AttachDecision InlinableNativeIRGenerator::tryAttachSetDelete() { + // Ensure |this| is a SetObject. + if (!thisval_.isObject() || !thisval_.toObject().is()) { + return AttachDecision::NoAction; + } + + // Need a single argument. + if (argc_ != 1) { + return AttachDecision::NoAction; + } + + // Initialize the input operand. + initializeInputOperand(); + + // Guard callee is the 'delete' native function. + emitNativeCalleeGuard(); + + // Guard |this| is a SetObject. + ValOperandId thisValId = + writer.loadArgumentFixedSlot(ArgumentKind::This, argc_); + ObjOperandId objId = writer.guardToObject(thisValId); + emitOptimisticClassGuard(objId, &thisval_.toObject(), GuardClassKind::Set); + + ValOperandId argId = writer.loadArgumentFixedSlot(ArgumentKind::Arg0, argc_); + writer.setDeleteResult(objId, argId); + writer.returnFromIC(); + + trackAttached("SetDelete"); + return AttachDecision::Attach; +} + AttachDecision InlinableNativeIRGenerator::tryAttachSetAdd() { // Ensure |this| is a SetObject. if (!thisval_.isObject() || !thisval_.toObject().is()) { @@ -10477,6 +10508,37 @@ AttachDecision InlinableNativeIRGenerator::tryAttachMapGet() { return AttachDecision::Attach; } +AttachDecision InlinableNativeIRGenerator::tryAttachMapDelete() { + // Ensure |this| is a MapObject. + if (!thisval_.isObject() || !thisval_.toObject().is()) { + return AttachDecision::NoAction; + } + + // Need a single argument. + if (argc_ != 1) { + return AttachDecision::NoAction; + } + + // Initialize the input operand. + initializeInputOperand(); + + // Guard callee is the 'delete' native function. + emitNativeCalleeGuard(); + + // Guard |this| is a MapObject. + ValOperandId thisValId = + writer.loadArgumentFixedSlot(ArgumentKind::This, argc_); + ObjOperandId objId = writer.guardToObject(thisValId); + emitOptimisticClassGuard(objId, &thisval_.toObject(), GuardClassKind::Map); + + ValOperandId argId = writer.loadArgumentFixedSlot(ArgumentKind::Arg0, argc_); + writer.mapDeleteResult(objId, argId); + writer.returnFromIC(); + + trackAttached("MapDelete"); + return AttachDecision::Attach; +} + AttachDecision InlinableNativeIRGenerator::tryAttachMapSet() { #ifdef JS_CODEGEN_X86 // 32-bit x86 does not have enough registers for the AutoCallVM output, the @@ -12401,6 +12463,8 @@ AttachDecision InlinableNativeIRGenerator::tryAttachStub() { return AttachDecision::NoAction; // Not callable. case InlinableNative::SetHas: return tryAttachSetHas(); + case InlinableNative::SetDelete: + return tryAttachSetDelete(); case InlinableNative::SetAdd: return tryAttachSetAdd(); case InlinableNative::SetSize: @@ -12413,6 +12477,8 @@ AttachDecision InlinableNativeIRGenerator::tryAttachStub() { return tryAttachMapHas(); case InlinableNative::MapGet: return tryAttachMapGet(); + case InlinableNative::MapDelete: + return tryAttachMapDelete(); case InlinableNative::MapSet: return tryAttachMapSet(); diff --git a/js/src/jit/CacheIRCompiler.cpp b/js/src/jit/CacheIRCompiler.cpp index 76dcc7d5d0e1..64a41733c4e0 100644 --- a/js/src/jit/CacheIRCompiler.cpp +++ b/js/src/jit/CacheIRCompiler.cpp @@ -10584,6 +10584,24 @@ bool CacheIRCompiler::emitSetHasObjectResult(ObjOperandId setId, return true; } +bool CacheIRCompiler::emitSetDeleteResult(ObjOperandId setId, + ValOperandId valId) { + JitSpew(JitSpew_Codegen, "%s", __FUNCTION__); + + AutoCallVM callvm(masm, this, allocator); + + Register set = allocator.useRegister(masm, setId); + ValueOperand val = allocator.useValueRegister(masm, valId); + + callvm.prepare(); + masm.Push(val); + masm.Push(set); + + using Fn = bool (*)(JSContext*, Handle, HandleValue, bool*); + callvm.call(); + return true; +} + bool CacheIRCompiler::emitSetAddResult(ObjOperandId setId, ValOperandId keyId) { JitSpew(JitSpew_Codegen, "%s", __FUNCTION__); @@ -10753,6 +10771,24 @@ bool CacheIRCompiler::emitMapGetResult(ObjOperandId mapId, ValOperandId valId) { return true; } +bool CacheIRCompiler::emitMapDeleteResult(ObjOperandId mapId, + ValOperandId valId) { + JitSpew(JitSpew_Codegen, "%s", __FUNCTION__); + + AutoCallVM callvm(masm, this, allocator); + + Register map = allocator.useRegister(masm, mapId); + ValueOperand val = allocator.useValueRegister(masm, valId); + + callvm.prepare(); + masm.Push(val); + masm.Push(map); + + using Fn = bool (*)(JSContext*, Handle, HandleValue, bool*); + callvm.call(); + return true; +} + bool CacheIRCompiler::emitMapSetResult(ObjOperandId mapId, ValOperandId keyId, ValOperandId valId) { JitSpew(JitSpew_Codegen, "%s", __FUNCTION__); diff --git a/js/src/jit/CacheIRGenerator.h b/js/src/jit/CacheIRGenerator.h index 00da513ce7b8..a2e4506f7af4 100644 --- a/js/src/jit/CacheIRGenerator.h +++ b/js/src/jit/CacheIRGenerator.h @@ -771,10 +771,12 @@ class MOZ_RAII InlinableNativeIRGenerator { AttachDecision tryAttachBigIntAsIntN(); AttachDecision tryAttachBigIntAsUintN(); AttachDecision tryAttachSetHas(); + AttachDecision tryAttachSetDelete(); AttachDecision tryAttachSetAdd(); AttachDecision tryAttachSetSize(); AttachDecision tryAttachMapHas(); AttachDecision tryAttachMapGet(); + AttachDecision tryAttachMapDelete(); AttachDecision tryAttachMapSet(); AttachDecision tryAttachDateGetTime(InlinableNative native); AttachDecision tryAttachDateGet(DateComponent component); diff --git a/js/src/jit/CacheIROps.yaml b/js/src/jit/CacheIROps.yaml index 0e6b96963f93..0614e487ef3f 100644 --- a/js/src/jit/CacheIROps.yaml +++ b/js/src/jit/CacheIROps.yaml @@ -3420,6 +3420,14 @@ set: ObjId obj: ObjId +- name: SetDeleteResult + shared: true + transpile: true + cost_estimate: 5 + args: + set: ObjId + val: ValId + - name: SetAddResult shared: true transpile: true @@ -3531,6 +3539,14 @@ map: ObjId obj: ObjId +- name: MapDeleteResult + shared: true + transpile: true + cost_estimate: 5 + args: + map: ObjId + val: ValId + - name: MapSetResult shared: true transpile: true diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp index 97618725e828..0bc38e314af4 100644 --- a/js/src/jit/CodeGenerator.cpp +++ b/js/src/jit/CodeGenerator.cpp @@ -21667,6 +21667,13 @@ void CodeGenerator::visitSetObjectHasValueVMCall( callVM(ins); } +void CodeGenerator::visitSetObjectDelete(LSetObjectDelete* ins) { + pushArg(ToValue(ins, LSetObjectDelete::KeyIndex)); + pushArg(ToRegister(ins->setObject())); + using Fn = bool (*)(JSContext*, Handle, HandleValue, bool*); + callVM(ins); +} + void CodeGenerator::visitSetObjectAdd(LSetObjectAdd* ins) { pushArg(ToValue(ins, LSetObjectAdd::KeyIndex)); pushArg(ToRegister(ins->setObject())); @@ -21779,6 +21786,13 @@ void CodeGenerator::visitMapObjectGetValueVMCall( callVM(ins); } +void CodeGenerator::visitMapObjectDelete(LMapObjectDelete* ins) { + pushArg(ToValue(ins, LMapObjectDelete::KeyIndex)); + pushArg(ToRegister(ins->mapObject())); + using Fn = bool (*)(JSContext*, Handle, HandleValue, bool*); + callVM(ins); +} + void CodeGenerator::visitMapObjectSet(LMapObjectSet* ins) { pushArg(ToValue(ins, LMapObjectSet::ValueIndex)); pushArg(ToValue(ins, LMapObjectSet::KeyIndex)); diff --git a/js/src/jit/InlinableNatives.cpp b/js/src/jit/InlinableNatives.cpp index c4152ccca87e..48bc2fe07664 100644 --- a/js/src/jit/InlinableNatives.cpp +++ b/js/src/jit/InlinableNatives.cpp @@ -313,6 +313,7 @@ bool js::jit::CanInlineNativeCrossRealm(InlinableNative native) { case InlinableNative::MapConstructor: case InlinableNative::MapGet: case InlinableNative::MapHas: + case InlinableNative::MapDelete: case InlinableNative::MapSet: case InlinableNative::Number: case InlinableNative::NumberParseInt: @@ -320,6 +321,7 @@ bool js::jit::CanInlineNativeCrossRealm(InlinableNative native) { case InlinableNative::ReflectGetPrototypeOf: case InlinableNative::SetConstructor: case InlinableNative::SetHas: + case InlinableNative::SetDelete: case InlinableNative::SetAdd: case InlinableNative::SetSize: case InlinableNative::String: diff --git a/js/src/jit/InlinableNatives.h b/js/src/jit/InlinableNatives.h index 5f0d8859dd34..3b68fa6695b2 100644 --- a/js/src/jit/InlinableNatives.h +++ b/js/src/jit/InlinableNatives.h @@ -97,6 +97,7 @@ _(IntlGuardToSegmentIterator) \ \ _(MapConstructor) \ + _(MapDelete) \ _(MapGet) \ _(MapHas) \ _(MapSet) \ @@ -155,6 +156,7 @@ _(GetFirstDollarIndex) \ \ _(SetConstructor) \ + _(SetDelete) \ _(SetHas) \ _(SetAdd) \ _(SetSize) \ diff --git a/js/src/jit/Lowering.cpp b/js/src/jit/Lowering.cpp index a35fca54b884..b57913f2c4ed 100644 --- a/js/src/jit/Lowering.cpp +++ b/js/src/jit/Lowering.cpp @@ -7666,6 +7666,13 @@ void LIRGenerator::visitSetObjectHasValueVMCall(MSetObjectHasValueVMCall* ins) { assignSafepoint(lir, ins); } +void LIRGenerator::visitSetObjectDelete(MSetObjectDelete* ins) { + auto* lir = new (alloc()) LSetObjectDelete( + useRegisterAtStart(ins->setObject()), useBoxAtStart(ins->key())); + defineReturn(lir, ins); + assignSafepoint(lir, ins); +} + void LIRGenerator::visitSetObjectAdd(MSetObjectAdd* ins) { auto* lir = new (alloc()) LSetObjectAdd(useRegisterAtStart(ins->setObject()), useBoxAtStart(ins->key())); @@ -7734,6 +7741,13 @@ void LIRGenerator::visitMapObjectGetValueVMCall(MMapObjectGetValueVMCall* ins) { assignSafepoint(lir, ins); } +void LIRGenerator::visitMapObjectDelete(MMapObjectDelete* ins) { + auto* lir = new (alloc()) LMapObjectDelete( + useRegisterAtStart(ins->mapObject()), useBoxAtStart(ins->key())); + defineReturn(lir, ins); + assignSafepoint(lir, ins); +} + void LIRGenerator::visitMapObjectSet(MMapObjectSet* ins) { auto* lir = new (alloc()) LMapObjectSet(useRegisterAtStart(ins->mapObject()), diff --git a/js/src/jit/MIROps.yaml b/js/src/jit/MIROps.yaml index 256cce0dbe62..027bf26aa9b3 100644 --- a/js/src/jit/MIROps.yaml +++ b/js/src/jit/MIROps.yaml @@ -3252,6 +3252,14 @@ alias_set: custom possibly_calls: true +- name: SetObjectDelete + operands: + setObject: Object + key: Value + result_type: Boolean + possibly_calls: true + generate_lir: true + - name: SetObjectAdd operands: setObject: Object @@ -3349,6 +3357,14 @@ alias_set: custom possibly_calls: true +- name: MapObjectDelete + operands: + mapObject: Object + key: Value + result_type: Boolean + possibly_calls: true + generate_lir: true + - name: MapObjectSet operands: mapObject: Object diff --git a/js/src/jit/VMFunctionList-inl.h b/js/src/jit/VMFunctionList-inl.h index 8fbf7885ab45..f6365fe875b8 100644 --- a/js/src/jit/VMFunctionList-inl.h +++ b/js/src/jit/VMFunctionList-inl.h @@ -248,6 +248,7 @@ namespace jit { _(LinearizeForCharAccess, js::jit::LinearizeForCharAccess) \ _(LoadAliasedDebugVar, js::LoadAliasedDebugVar) \ _(MapObjectCreate, js::MapObject::create) \ + _(MapObjectDelete, js::jit::MapObjectDelete) \ _(MapObjectGet, js::jit::MapObjectGet) \ _(MapObjectHas, js::jit::MapObjectHas) \ _(MapObjectSet, js::jit::MapObjectSet) \ @@ -309,6 +310,7 @@ namespace jit { _(SetObjectAdd, js::jit::SetObjectAdd) \ _(SetObjectAddFromIC, js::jit::SetObjectAddFromIC) \ _(SetObjectCreate, js::SetObject::create) \ + _(SetObjectDelete, js::jit::SetObjectDelete) \ _(SetObjectHas, js::jit::SetObjectHas) \ _(SetPropertyMegamorphicNoCache, js::jit::SetPropertyMegamorphic) \ _(SetPropertyMegamorphicYesCache, js::jit::SetPropertyMegamorphic) \ diff --git a/js/src/jit/VMFunctions.cpp b/js/src/jit/VMFunctions.cpp index 1c6413e20e45..05e1313527cd 100644 --- a/js/src/jit/VMFunctions.cpp +++ b/js/src/jit/VMFunctions.cpp @@ -3115,6 +3115,11 @@ bool SetObjectHas(JSContext* cx, Handle obj, HandleValue key, return obj->has(cx, key, rval); } +bool SetObjectDelete(JSContext* cx, Handle obj, HandleValue key, + bool* rval) { + return obj->delete_(cx, key, rval); +} + bool SetObjectAdd(JSContext* cx, Handle obj, HandleValue key) { return obj->add(cx, key); } @@ -3138,6 +3143,11 @@ bool MapObjectGet(JSContext* cx, Handle obj, HandleValue key, return obj->get(cx, key, rval); } +bool MapObjectDelete(JSContext* cx, Handle obj, HandleValue key, + bool* rval) { + return obj->delete_(cx, key, rval); +} + bool MapObjectSet(JSContext* cx, Handle obj, HandleValue key, HandleValue val) { return obj->set(cx, key, val); diff --git a/js/src/jit/VMFunctions.h b/js/src/jit/VMFunctions.h index 16cc15836281..4a7006255a20 100644 --- a/js/src/jit/VMFunctions.h +++ b/js/src/jit/VMFunctions.h @@ -705,6 +705,8 @@ JSAtom* AtomizeStringNoGC(JSContext* cx, JSString* str); bool SetObjectHas(JSContext* cx, Handle obj, HandleValue key, bool* rval); +bool SetObjectDelete(JSContext* cx, Handle obj, HandleValue key, + bool* rval); bool SetObjectAdd(JSContext* cx, Handle obj, HandleValue key); bool SetObjectAddFromIC(JSContext* cx, Handle obj, HandleValue key, MutableHandleValue rval); @@ -712,6 +714,8 @@ bool MapObjectHas(JSContext* cx, Handle obj, HandleValue key, bool* rval); bool MapObjectGet(JSContext* cx, Handle obj, HandleValue key, MutableHandleValue rval); +bool MapObjectDelete(JSContext* cx, Handle obj, HandleValue key, + bool* rval); bool MapObjectSet(JSContext* cx, Handle obj, HandleValue key, HandleValue val); bool MapObjectSetFromIC(JSContext* cx, Handle obj, HandleValue key, diff --git a/js/src/jit/WarpCacheIRTranspiler.cpp b/js/src/jit/WarpCacheIRTranspiler.cpp index 4dab7423c055..b49ce735578e 100644 --- a/js/src/jit/WarpCacheIRTranspiler.cpp +++ b/js/src/jit/WarpCacheIRTranspiler.cpp @@ -5282,6 +5282,18 @@ bool WarpCacheIRTranspiler::emitSetHasResult(ObjOperandId setId, return true; } +bool WarpCacheIRTranspiler::emitSetDeleteResult(ObjOperandId setId, + ValOperandId keyId) { + MDefinition* set = getOperand(setId); + MDefinition* key = getOperand(keyId); + + auto* ins = MSetObjectDelete::New(alloc(), set, key); + addEffectful(ins); + + pushResult(ins); + return resumeAfter(ins); +} + bool WarpCacheIRTranspiler::emitSetAddResult(ObjOperandId setId, ValOperandId keyId) { MDefinition* set = getOperand(setId); @@ -5512,6 +5524,18 @@ bool WarpCacheIRTranspiler::emitMapGetResult(ObjOperandId mapId, return true; } +bool WarpCacheIRTranspiler::emitMapDeleteResult(ObjOperandId mapId, + ValOperandId keyId) { + MDefinition* map = getOperand(mapId); + MDefinition* key = getOperand(keyId); + + auto* ins = MMapObjectDelete::New(alloc(), map, key); + addEffectful(ins); + + pushResult(ins); + return resumeAfter(ins); +} + bool WarpCacheIRTranspiler::emitMapSetResult(ObjOperandId mapId, ValOperandId keyId, ValOperandId valId) {