Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (134331 => 134332)
--- trunk/Source/_javascript_Core/ChangeLog 2012-11-13 01:26:17 UTC (rev 134331)
+++ trunk/Source/_javascript_Core/ChangeLog 2012-11-13 01:55:42 UTC (rev 134332)
@@ -1,3 +1,50 @@
+2012-11-12 Filip Pizlo <[email protected]>
+
+ Patching of jumps to stubs should use jump replacement rather than branch destination overwrite
+ https://bugs.webkit.org/show_bug.cgi?id=101909
+
+ Reviewed by Geoffrey Garen.
+
+ This saves a few instructions in inline cases, on those architectures where it is
+ easy to figure out where to put the jump replacement. Sub-1% speed-up across the
+ board.
+
+ * assembler/MacroAssemblerARMv7.h:
+ (MacroAssemblerARMv7):
+ (JSC::MacroAssemblerARMv7::canJumpReplacePatchableBranchPtrWithPatch):
+ (JSC::MacroAssemblerARMv7::startOfPatchableBranchPtrWithPatch):
+ (JSC::MacroAssemblerARMv7::revertJumpReplacementToPatchableBranchPtrWithPatch):
+ * assembler/MacroAssemblerX86.h:
+ (JSC::MacroAssemblerX86::canJumpReplacePatchableBranchPtrWithPatch):
+ (MacroAssemblerX86):
+ (JSC::MacroAssemblerX86::startOfPatchableBranchPtrWithPatch):
+ (JSC::MacroAssemblerX86::revertJumpReplacementToPatchableBranchPtrWithPatch):
+ * assembler/MacroAssemblerX86_64.h:
+ (JSC::MacroAssemblerX86_64::canJumpReplacePatchableBranchPtrWithPatch):
+ (MacroAssemblerX86_64):
+ (JSC::MacroAssemblerX86_64::startOfPatchableBranchPtrWithPatch):
+ (JSC::MacroAssemblerX86_64::revertJumpReplacementToPatchableBranchPtrWithPatch):
+ * assembler/RepatchBuffer.h:
+ (JSC::RepatchBuffer::startOfPatchableBranchPtrWithPatch):
+ (RepatchBuffer):
+ (JSC::RepatchBuffer::replaceWithJump):
+ (JSC::RepatchBuffer::revertJumpReplacementToPatchableBranchPtrWithPatch):
+ * assembler/X86Assembler.h:
+ (X86Assembler):
+ (JSC::X86Assembler::revertJumpTo_movq_i64r):
+ (JSC::X86Assembler::revertJumpTo_cmpl_im_force32):
+ (X86InstructionFormatter):
+ * bytecode/StructureStubInfo.h:
+ * dfg/DFGRepatch.cpp:
+ (JSC::DFG::replaceWithJump):
+ (DFG):
+ (JSC::DFG::tryCacheGetByID):
+ (JSC::DFG::tryBuildGetByIDList):
+ (JSC::DFG::tryBuildGetByIDProtoList):
+ (JSC::DFG::tryCachePutByID):
+ (JSC::DFG::dfgResetGetByID):
+ (JSC::DFG::dfgResetPutByID):
+
2012-11-11 Filip Pizlo <[email protected]>
DFG ArithMul overflow check elimination is too aggressive
Modified: trunk/Source/_javascript_Core/assembler/MacroAssemblerARMv7.h (134331 => 134332)
--- trunk/Source/_javascript_Core/assembler/MacroAssemblerARMv7.h 2012-11-13 01:26:17 UTC (rev 134331)
+++ trunk/Source/_javascript_Core/assembler/MacroAssemblerARMv7.h 2012-11-13 01:55:42 UTC (rev 134332)
@@ -1758,6 +1758,18 @@
{
return FunctionPtr(reinterpret_cast<void(*)()>(ARMv7Assembler::readCallTarget(call.dataLocation())));
}
+
+ static bool canJumpReplacePatchableBranchPtrWithPatch() { return false; }
+
+ static CodeLocationLabel startOfPatchableBranchPtrWithPatch(CodeLocationDataLabelPtr label)
+ {
+ UNREACHABLE_FOR_PLATFORM();
+ }
+
+ static void revertJumpReplacementToPatchableBranchPtrWithPatch(CodeLocationLabel instructionStart, Address, void* initialValue)
+ {
+ UNREACHABLE_FOR_PLATFORM();
+ }
protected:
ALWAYS_INLINE Jump jump()
Modified: trunk/Source/_javascript_Core/assembler/MacroAssemblerX86.h (134331 => 134332)
--- trunk/Source/_javascript_Core/assembler/MacroAssemblerX86.h 2012-11-13 01:26:17 UTC (rev 134331)
+++ trunk/Source/_javascript_Core/assembler/MacroAssemblerX86.h 2012-11-13 01:55:42 UTC (rev 134332)
@@ -253,6 +253,25 @@
return FunctionPtr(reinterpret_cast<void*>(reinterpret_cast<uintptr_t>(call.dataLocation()) + offset));
}
+ static bool canJumpReplacePatchableBranchPtrWithPatch() { return true; }
+
+ static CodeLocationLabel startOfPatchableBranchPtrWithPatch(CodeLocationDataLabelPtr label)
+ {
+ const int opcodeBytes = 1;
+ const int modRMBytes = 1;
+ const int offsetBytes = 0;
+ const int immediateBytes = 4;
+ const int totalBytes = opcodeBytes + modRMBytes + offsetBytes + immediateBytes;
+ ASSERT(totalBytes >= maxJumpReplacementSize());
+ return label.labelAtOffset(-totalBytes);
+ }
+
+ static void revertJumpReplacementToPatchableBranchPtrWithPatch(CodeLocationLabel instructionStart, Address address, void* initialValue)
+ {
+ ASSERT(!address.offset);
+ X86Assembler::revertJumpTo_cmpl_im_force32(instructionStart.executableAddress(), reinterpret_cast<intptr_t>(initialValue), 0, address.base);
+ }
+
private:
friend class LinkBuffer;
friend class RepatchBuffer;
Modified: trunk/Source/_javascript_Core/assembler/MacroAssemblerX86_64.h (134331 => 134332)
--- trunk/Source/_javascript_Core/assembler/MacroAssemblerX86_64.h 2012-11-13 01:26:17 UTC (rev 134331)
+++ trunk/Source/_javascript_Core/assembler/MacroAssemblerX86_64.h 2012-11-13 01:55:42 UTC (rev 134332)
@@ -585,6 +585,23 @@
static RegisterID scratchRegisterForBlinding() { return scratchRegister; }
+ static bool canJumpReplacePatchableBranchPtrWithPatch() { return true; }
+
+ static CodeLocationLabel startOfPatchableBranchPtrWithPatch(CodeLocationDataLabelPtr label)
+ {
+ const int rexBytes = 1;
+ const int opcodeBytes = 1;
+ const int immediateBytes = 8;
+ const int totalBytes = rexBytes + opcodeBytes + immediateBytes;
+ ASSERT(totalBytes >= maxJumpReplacementSize());
+ return label.labelAtOffset(-totalBytes);
+ }
+
+ static void revertJumpReplacementToPatchableBranchPtrWithPatch(CodeLocationLabel instructionStart, Address, void* initialValue)
+ {
+ X86Assembler::revertJumpTo_movq_i64r(instructionStart.executableAddress(), reinterpret_cast<intptr_t>(initialValue), scratchRegister);
+ }
+
private:
friend class LinkBuffer;
friend class RepatchBuffer;
Modified: trunk/Source/_javascript_Core/assembler/RepatchBuffer.h (134331 => 134332)
--- trunk/Source/_javascript_Core/assembler/RepatchBuffer.h 2012-11-13 01:26:17 UTC (rev 134331)
+++ trunk/Source/_javascript_Core/assembler/RepatchBuffer.h 2012-11-13 01:55:42 UTC (rev 134332)
@@ -141,6 +141,24 @@
replaceWithAddressComputation(label);
}
+ static CodeLocationLabel startOfPatchableBranchPtrWithPatch(CodeLocationDataLabelPtr label)
+ {
+ return MacroAssembler::startOfPatchableBranchPtrWithPatch(label);
+ }
+
+ void replaceWithJump(CodeLocationLabel instructionStart, CodeLocationLabel destination)
+ {
+ MacroAssembler::replaceWithJump(instructionStart, destination);
+ }
+
+ // This is a *bit* of a silly API, since we currently always also repatch the
+ // immediate after calling this. But I'm fine with that, since this just feels
+ // less yucky.
+ void revertJumpReplacementToPatchableBranchPtrWithPatch(CodeLocationLabel instructionStart, MacroAssembler::Address address, void* value)
+ {
+ MacroAssembler::revertJumpReplacementToPatchableBranchPtrWithPatch(instructionStart, address, value);
+ }
+
private:
void* m_start;
size_t m_size;
Modified: trunk/Source/_javascript_Core/assembler/X86Assembler.h (134331 => 134332)
--- trunk/Source/_javascript_Core/assembler/X86Assembler.h 2012-11-13 01:26:17 UTC (rev 134331)
+++ trunk/Source/_javascript_Core/assembler/X86Assembler.h 2012-11-13 01:55:42 UTC (rev 134332)
@@ -1883,6 +1883,44 @@
return 5;
}
+#if CPU(X86_64)
+ static void revertJumpTo_movq_i64r(void* instructionStart, int64_t imm, RegisterID dst)
+ {
+ const int rexBytes = 1;
+ const int opcodeBytes = 1;
+ ASSERT(rexBytes + opcodeBytes <= maxJumpReplacementSize());
+ uint8_t* ptr = reinterpret_cast<uint8_t*>(instructionStart);
+ ptr[0] = PRE_REX | (1 << 3) | (dst >> 3);
+ ptr[1] = OP_MOV_EAXIv | (dst & 7);
+
+ union {
+ uint64_t asWord;
+ uint8_t asBytes[8];
+ } u;
+ u.asWord = imm;
+ for (unsigned i = rexBytes + opcodeBytes; i < static_cast<unsigned>(maxJumpReplacementSize()); ++i)
+ ptr[i] = u.asBytes[i - rexBytes - opcodeBytes];
+ }
+#endif
+
+ static void revertJumpTo_cmpl_im_force32(void* instructionStart, int32_t imm, int offset, RegisterID dst)
+ {
+ ASSERT_UNUSED(offset, !offset);
+ const int opcodeBytes = 1;
+ const int modRMBytes = 1;
+ ASSERT(opcodeBytes + modRMBytes <= maxJumpReplacementSize());
+ uint8_t* ptr = reinterpret_cast<uint8_t*>(instructionStart);
+ ptr[0] = OP_GROUP1_EvIz;
+ ptr[1] = (X86InstructionFormatter::ModRmMemoryNoDisp << 6) | (GROUP1_OP_CMP << 3) | dst;
+ union {
+ uint32_t asWord;
+ uint8_t asBytes[4];
+ } u;
+ u.asWord = imm;
+ for (unsigned i = opcodeBytes + modRMBytes; i < static_cast<unsigned>(maxJumpReplacementSize()); ++i)
+ ptr[i] = u.asBytes[i - opcodeBytes - modRMBytes];
+ }
+
static void replaceWithLoad(void* instructionStart)
{
uint8_t* ptr = reinterpret_cast<uint8_t*>(instructionStart);
@@ -1982,6 +2020,13 @@
public:
+ enum ModRmMode {
+ ModRmMemoryNoDisp,
+ ModRmMemoryDisp8,
+ ModRmMemoryDisp32,
+ ModRmRegister,
+ };
+
// Legacy prefix bytes:
//
// These are emmitted prior to the instruction.
@@ -2352,13 +2397,6 @@
inline void emitRexIfNeeded(int, int, int) {}
#endif
- enum ModRmMode {
- ModRmMemoryNoDisp,
- ModRmMemoryDisp8,
- ModRmMemoryDisp32,
- ModRmRegister,
- };
-
void putModRm(ModRmMode mode, int reg, RegisterID rm)
{
m_buffer.putByteUnchecked((mode << 6) | ((reg & 7) << 3) | (rm & 7));
Modified: trunk/Source/_javascript_Core/bytecode/StructureStubInfo.h (134331 => 134332)
--- trunk/Source/_javascript_Core/bytecode/StructureStubInfo.h 2012-11-13 01:26:17 UTC (rev 134331)
+++ trunk/Source/_javascript_Core/bytecode/StructureStubInfo.h 2012-11-13 01:55:42 UTC (rev 134332)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2008 Apple Inc. All rights reserved.
+ * Copyright (C) 2008, 2012 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
Modified: trunk/Source/_javascript_Core/dfg/DFGRepatch.cpp (134331 => 134332)
--- trunk/Source/_javascript_Core/dfg/DFGRepatch.cpp 2012-11-13 01:26:17 UTC (rev 134331)
+++ trunk/Source/_javascript_Core/dfg/DFGRepatch.cpp 2012-11-13 01:55:42 UTC (rev 134332)
@@ -114,6 +114,23 @@
failureCases, scratchGPR);
}
+static void replaceWithJump(RepatchBuffer& repatchBuffer, StructureStubInfo& stubInfo, const MacroAssemblerCodePtr target)
+{
+ if (MacroAssembler::canJumpReplacePatchableBranchPtrWithPatch()) {
+ repatchBuffer.replaceWithJump(
+ RepatchBuffer::startOfPatchableBranchPtrWithPatch(
+ stubInfo.callReturnLocation.dataLabelPtrAtOffset(
+ -(intptr_t)stubInfo.patch.dfg.deltaCheckImmToCall)),
+ CodeLocationLabel(target));
+ return;
+ }
+
+ repatchBuffer.relink(
+ stubInfo.callReturnLocation.jumpAtOffset(
+ stubInfo.patch.dfg.deltaCallToStructCheck),
+ CodeLocationLabel(target));
+}
+
static void emitRestoreScratch(MacroAssembler& stubJit, bool needToRestoreScratch, GPRReg scratchGPR, MacroAssembler::Jump& success, MacroAssembler::Jump& fail, MacroAssembler::JumpList failureCases)
{
if (needToRestoreScratch) {
@@ -284,7 +301,7 @@
stubInfo.patch.dfg.deltaCallToDone).executableAddress()));
RepatchBuffer repatchBuffer(codeBlock);
- repatchBuffer.relink(stubInfo.callReturnLocation.jumpAtOffset(stubInfo.patch.dfg.deltaCallToStructCheck), CodeLocationLabel(stubInfo.stubRoutine->code().code()));
+ replaceWithJump(repatchBuffer, stubInfo, stubInfo.stubRoutine->code().code());
repatchBuffer.relink(stubInfo.callReturnLocation, operationGetById);
return true;
@@ -334,7 +351,7 @@
generateProtoChainAccessStub(exec, stubInfo, prototypeChain, count, offset, structure, stubInfo.callReturnLocation.labelAtOffset(stubInfo.patch.dfg.deltaCallToDone), stubInfo.callReturnLocation.labelAtOffset(stubInfo.patch.dfg.deltaCallToSlowCase), stubInfo.stubRoutine);
RepatchBuffer repatchBuffer(codeBlock);
- repatchBuffer.relink(stubInfo.callReturnLocation.jumpAtOffset(stubInfo.patch.dfg.deltaCallToStructCheck), CodeLocationLabel(stubInfo.stubRoutine->code().code()));
+ replaceWithJump(repatchBuffer, stubInfo, stubInfo.stubRoutine->code().code());
repatchBuffer.relink(stubInfo.callReturnLocation, operationGetByIdProtoBuildList);
stubInfo.initGetByIdChain(*globalData, codeBlock->ownerExecutable(), structure, prototypeChain, count, true);
@@ -518,9 +535,11 @@
polymorphicStructureList->list[listIndex].set(*globalData, codeBlock->ownerExecutable(), stubRoutine, structure, isDirect);
- CodeLocationJump jumpLocation = stubInfo.callReturnLocation.jumpAtOffset(stubInfo.patch.dfg.deltaCallToStructCheck);
RepatchBuffer repatchBuffer(codeBlock);
- repatchBuffer.relink(jumpLocation, CodeLocationLabel(stubRoutine->code().code()));
+ repatchBuffer.relink(
+ stubInfo.callReturnLocation.jumpAtOffset(
+ stubInfo.patch.dfg.deltaCallToStructCheck),
+ CodeLocationLabel(stubRoutine->code().code()));
if (listIndex < (POLYMORPHIC_LIST_CACHE_SIZE - 1))
return true;
@@ -584,9 +603,8 @@
polymorphicStructureList->list[listIndex].set(*globalData, codeBlock->ownerExecutable(), stubRoutine, structure, true);
- CodeLocationJump jumpLocation = stubInfo.callReturnLocation.jumpAtOffset(stubInfo.patch.dfg.deltaCallToStructCheck);
RepatchBuffer repatchBuffer(codeBlock);
- repatchBuffer.relink(jumpLocation, CodeLocationLabel(stubRoutine->code().code()));
+ replaceWithJump(repatchBuffer, stubInfo, stubRoutine->code().code());
if (listIndex < (POLYMORPHIC_LIST_CACHE_SIZE - 1))
return true;
@@ -976,7 +994,10 @@
stubInfo.stubRoutine);
RepatchBuffer repatchBuffer(codeBlock);
- repatchBuffer.relink(stubInfo.callReturnLocation.jumpAtOffset(stubInfo.patch.dfg.deltaCallToStructCheck), CodeLocationLabel(stubInfo.stubRoutine->code().code()));
+ repatchBuffer.relink(
+ stubInfo.callReturnLocation.jumpAtOffset(
+ stubInfo.patch.dfg.deltaCallToStructCheck),
+ CodeLocationLabel(stubInfo.stubRoutine->code().code()));
repatchBuffer.relink(stubInfo.callReturnLocation, appropriateListBuildingPutByIdFunction(slot, putKind));
stubInfo.initPutByIdTransition(*globalData, codeBlock->ownerExecutable(), oldStructure, structure, prototypeChain, putKind == Direct);
@@ -1118,7 +1139,16 @@
void dfgResetGetByID(RepatchBuffer& repatchBuffer, StructureStubInfo& stubInfo)
{
repatchBuffer.relink(stubInfo.callReturnLocation, operationGetByIdOptimize);
- repatchBuffer.repatch(stubInfo.callReturnLocation.dataLabelPtrAtOffset(-(intptr_t)stubInfo.patch.dfg.deltaCheckImmToCall), reinterpret_cast<void*>(-1));
+ CodeLocationDataLabelPtr structureLabel = stubInfo.callReturnLocation.dataLabelPtrAtOffset(-(intptr_t)stubInfo.patch.dfg.deltaCheckImmToCall);
+ if (MacroAssembler::canJumpReplacePatchableBranchPtrWithPatch()) {
+ repatchBuffer.revertJumpReplacementToPatchableBranchPtrWithPatch(
+ RepatchBuffer::startOfPatchableBranchPtrWithPatch(structureLabel),
+ MacroAssembler::Address(
+ static_cast<MacroAssembler::RegisterID>(stubInfo.patch.dfg.baseGPR),
+ JSCell::structureOffset()),
+ reinterpret_cast<void*>(-1));
+ }
+ repatchBuffer.repatch(structureLabel, reinterpret_cast<void*>(-1));
#if USE(JSVALUE64)
repatchBuffer.repatch(stubInfo.callReturnLocation.dataLabelCompactAtOffset(stubInfo.patch.dfg.deltaCallToLoadOrStore), 0);
#else
@@ -1143,7 +1173,16 @@
optimizedFunction = operationPutByIdDirectNonStrictOptimize;
}
repatchBuffer.relink(stubInfo.callReturnLocation, optimizedFunction);
- repatchBuffer.repatch(stubInfo.callReturnLocation.dataLabelPtrAtOffset(-(intptr_t)stubInfo.patch.dfg.deltaCheckImmToCall), reinterpret_cast<void*>(-1));
+ CodeLocationDataLabelPtr structureLabel = stubInfo.callReturnLocation.dataLabelPtrAtOffset(-(intptr_t)stubInfo.patch.dfg.deltaCheckImmToCall);
+ if (MacroAssembler::canJumpReplacePatchableBranchPtrWithPatch()) {
+ repatchBuffer.revertJumpReplacementToPatchableBranchPtrWithPatch(
+ RepatchBuffer::startOfPatchableBranchPtrWithPatch(structureLabel),
+ MacroAssembler::Address(
+ static_cast<MacroAssembler::RegisterID>(stubInfo.patch.dfg.baseGPR),
+ JSCell::structureOffset()),
+ reinterpret_cast<void*>(-1));
+ }
+ repatchBuffer.repatch(structureLabel, reinterpret_cast<void*>(-1));
#if USE(JSVALUE64)
repatchBuffer.repatch(stubInfo.callReturnLocation.dataLabel32AtOffset(stubInfo.patch.dfg.deltaCallToLoadOrStore), 0);
#else
