- Revision
- 285332
- Author
- [email protected]
- Date
- 2021-11-04 20:40:04 -0700 (Thu, 04 Nov 2021)
Log Message
Cherry-pick r284036. rdar://problem/85034297
Fix spec-correctness when inlining __proto__ intrinsic using get_by_id_with_this
https://bugs.webkit.org/show_bug.cgi?id=231559
Reviewed by Yusuke Suzuki.
My original fix in r283512 has some theoretical spec correctness issues.
I'm not sure if they can be materialized or not since we only use
get_by_id_with_this in very limited scenarios. However, this patch just
makes it so we call the getter instead of attempting to inline it
when using get_by_id_with_this.
* jit/AssemblyHelpers.cpp:
(JSC::AssemblyHelpers::emitLoadPrototypeWithoutCheck): Deleted.
* jit/AssemblyHelpers.h:
* jit/IntrinsicEmitter.cpp:
(JSC::IntrinsicGetterAccessCase::canEmitIntrinsicGetter):
(JSC::IntrinsicGetterAccessCase::emitIntrinsicGetter):
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@284036 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Diff
Modified: branches/safari-612-branch/Source/_javascript_Core/ChangeLog (285331 => 285332)
--- branches/safari-612-branch/Source/_javascript_Core/ChangeLog 2021-11-05 03:40:02 UTC (rev 285331)
+++ branches/safari-612-branch/Source/_javascript_Core/ChangeLog 2021-11-05 03:40:04 UTC (rev 285332)
@@ -1,5 +1,50 @@
2021-11-04 Russell Epstein <[email protected]>
+ Cherry-pick r284036. rdar://problem/85034297
+
+ Fix spec-correctness when inlining __proto__ intrinsic using get_by_id_with_this
+ https://bugs.webkit.org/show_bug.cgi?id=231559
+
+ Reviewed by Yusuke Suzuki.
+
+ My original fix in r283512 has some theoretical spec correctness issues.
+ I'm not sure if they can be materialized or not since we only use
+ get_by_id_with_this in very limited scenarios. However, this patch just
+ makes it so we call the getter instead of attempting to inline it
+ when using get_by_id_with_this.
+
+ * jit/AssemblyHelpers.cpp:
+ (JSC::AssemblyHelpers::emitLoadPrototypeWithoutCheck): Deleted.
+ * jit/AssemblyHelpers.h:
+ * jit/IntrinsicEmitter.cpp:
+ (JSC::IntrinsicGetterAccessCase::canEmitIntrinsicGetter):
+ (JSC::IntrinsicGetterAccessCase::emitIntrinsicGetter):
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@284036 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2021-10-12 Saam Barati <[email protected]>
+
+ Fix spec-correctness when inlining __proto__ intrinsic using get_by_id_with_this
+ https://bugs.webkit.org/show_bug.cgi?id=231559
+
+ Reviewed by Yusuke Suzuki.
+
+ My original fix in r283512 has some theoretical spec correctness issues.
+ I'm not sure if they can be materialized or not since we only use
+ get_by_id_with_this in very limited scenarios. However, this patch just
+ makes it so we call the getter instead of attempting to inline it
+ when using get_by_id_with_this.
+
+ * jit/AssemblyHelpers.cpp:
+ (JSC::AssemblyHelpers::emitLoadPrototypeWithoutCheck): Deleted.
+ * jit/AssemblyHelpers.h:
+ * jit/IntrinsicEmitter.cpp:
+ (JSC::IntrinsicGetterAccessCase::canEmitIntrinsicGetter):
+ (JSC::IntrinsicGetterAccessCase::emitIntrinsicGetter):
+
+2021-11-04 Russell Epstein <[email protected]>
+
Cherry-pick r283954. rdar://problem/85045208
Share more code that uses ScratchRegisterAllocator in the ICs
Modified: branches/safari-612-branch/Source/_javascript_Core/jit/AssemblyHelpers.cpp (285331 => 285332)
--- branches/safari-612-branch/Source/_javascript_Core/jit/AssemblyHelpers.cpp 2021-11-05 03:40:02 UTC (rev 285331)
+++ branches/safari-612-branch/Source/_javascript_Core/jit/AssemblyHelpers.cpp 2021-11-05 03:40:04 UTC (rev 285332)
@@ -413,36 +413,6 @@
hasMonoProto.link(this);
}
-void AssemblyHelpers::emitLoadPrototypeWithoutCheck(VM& vm, GPRReg objectGPR, JSValueRegs resultRegs, GPRReg scratchGPR, GPRReg scratch2GPR)
-{
- ASSERT(objectGPR != scratchGPR);
- ASSERT(objectGPR != scratch2GPR);
- ASSERT(resultRegs.payloadGPR() != scratchGPR);
- ASSERT(resultRegs.payloadGPR() != scratch2GPR);
-#if USE(JSVALUE32_64)
- ASSERT(resultRegs.tagGPR() != scratchGPR);
- ASSERT(resultRegs.tagGPR() != scratch2GPR);
-#endif
-
- emitLoadStructure(vm, objectGPR, scratchGPR, scratch2GPR);
-#if USE(JSVALUE64)
- loadValue(MacroAssembler::Address(scratchGPR, Structure::prototypeOffset()), JSValueRegs(scratch2GPR));
-#else
- load32(MacroAssembler::Address(scratchGPR, Structure::prototypeOffset() + TagOffset), scratch2GPR);
-#endif
- auto hasMonoProto = branchIfNotEmpty(scratch2GPR);
- loadValue(MacroAssembler::Address(objectGPR, offsetRelativeToBase(knownPolyProtoOffset)), resultRegs);
- auto done = jump();
- hasMonoProto.link(this);
-#if USE(JSVALUE64)
- move(scratch2GPR, resultRegs.payloadGPR());
-#else
- load32(MacroAssembler::Address(scratchGPR, Structure::prototypeOffset() + PayloadOffset), resultRegs.payloadGPR());
- move(scratch2GPR, resultRegs.tagGPR());
-#endif
- done.link(this);
-}
-
void AssemblyHelpers::makeSpaceOnStackForCCall()
{
unsigned stackOffset = WTF::roundUpToMultipleOf(stackAlignmentBytes(), maxFrameExtentForSlowPathCall);
Modified: branches/safari-612-branch/Source/_javascript_Core/jit/AssemblyHelpers.h (285331 => 285332)
--- branches/safari-612-branch/Source/_javascript_Core/jit/AssemblyHelpers.h 2021-11-05 03:40:02 UTC (rev 285331)
+++ branches/safari-612-branch/Source/_javascript_Core/jit/AssemblyHelpers.h 2021-11-05 03:40:04 UTC (rev 285332)
@@ -1518,7 +1518,6 @@
void emitLoadStructure(VM&, RegisterID source, RegisterID dest, RegisterID scratch);
void emitLoadPrototype(VM&, GPRReg objectGPR, JSValueRegs resultRegs, GPRReg scratchGPR, JumpList& slowPath);
- void emitLoadPrototypeWithoutCheck(VM&, GPRReg objectGPR, JSValueRegs resultRegs, GPRReg scratchGPR, GPRReg scratch2GPR);
void emitStoreStructureWithTypeInfo(TrustedImmPtr structure, RegisterID dest, RegisterID)
{
Modified: branches/safari-612-branch/Source/_javascript_Core/jit/IntrinsicEmitter.cpp (285331 => 285332)
--- branches/safari-612-branch/Source/_javascript_Core/jit/IntrinsicEmitter.cpp 2021-11-05 03:40:02 UTC (rev 285331)
+++ branches/safari-612-branch/Source/_javascript_Core/jit/IntrinsicEmitter.cpp 2021-11-05 03:40:04 UTC (rev 285332)
@@ -48,15 +48,18 @@
bool IntrinsicGetterAccessCase::canEmitIntrinsicGetter(StructureStubInfo& stubInfo, JSFunction* getter, Structure* structure)
{
+ // We aren't structure checking the this value, so we don't know:
+ // - For type array loads, that it's a typed array.
+ // - For __proto__ getter, that the incoming value is an object,
+ // and if it overrides getPrototype structure flags.
+ // So for these cases, it's simpler to just call the getter directly.
+ if (stubInfo.thisValueIsInThisGPR())
+ return false;
switch (getter->intrinsic()) {
case TypedArrayByteOffsetIntrinsic:
case TypedArrayByteLengthIntrinsic:
case TypedArrayLengthIntrinsic: {
- // We aren't structure checking the this value, so we don't know it's a typed array.
- if (stubInfo.thisValueIsInThisGPR())
- return false;
-
TypedArrayType type = structure->classInfo()->typedArrayStorageType;
if (!isTypedView(type))
@@ -150,21 +153,10 @@
}
case UnderscoreProtoIntrinsic: {
- StructureStubInfo& stubInfo = *state.stubInfo;
- if (stubInfo.thisValueIsInThisGPR()) {
- auto allocator = state.makeDefaultScratchAllocator(state.scratchGPR);
- GPRReg scratch2GPR = allocator.allocateScratchGPR();
- auto preservedState = allocator.preserveReusedRegistersByPushing(jit, ScratchRegisterAllocator::ExtraStackSpace::NoExtraSpace);
-
- jit.emitLoadPrototypeWithoutCheck(state.m_vm, state.u.thisGPR, valueRegs, state.scratchGPR, scratch2GPR);
-
- allocator.restoreReusedRegistersByPopping(jit, preservedState);
- } else {
- if (structure()->hasPolyProto())
- jit.loadValue(CCallHelpers::Address(baseGPR, offsetRelativeToBase(knownPolyProtoOffset)), valueRegs);
- else
- jit.moveValue(structure()->storedPrototype(), valueRegs);
- }
+ if (structure()->hasPolyProto())
+ jit.loadValue(CCallHelpers::Address(baseGPR, offsetRelativeToBase(knownPolyProtoOffset)), valueRegs);
+ else
+ jit.moveValue(structure()->storedPrototype(), valueRegs);
state.succeed();
return;
}
