- Revision
- 160493
- Author
- [email protected]
- Date
- 2013-12-12 10:38:39 -0800 (Thu, 12 Dec 2013)
Log Message
ARM64: Hang running pdfjs test, suspect DFG generated code for "in"
https://bugs.webkit.org/show_bug.cgi?id=124727
<rdar://problem/15566923>
Reviewed by Michael Saboff.
Get rid of In's hackish use of StructureStubInfo. Previously it was using hotPathBegin,
and it was the only IC that used that field, which was wasteful. Moreover, it used it
to store two separate locations: the label for patching the jump and the label right
after the jump. The code was relying on those two being the same label, which is true
on X86 and some other platforms, but it isn't true on ARM64.
This gets rid of hotPathBegin and makes In express those two locations as offsets from
the callReturnLocation, which is analogous to what the other IC's do.
This fixes a bug where any successful In patching would result in a trivially infinite
loop - and hence a hang - on ARM64.
* bytecode/StructureStubInfo.h:
* dfg/DFGJITCompiler.cpp:
(JSC::DFG::JITCompiler::link):
* dfg/DFGJITCompiler.h:
(JSC::DFG::InRecord::InRecord):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileIn):
* jit/JITInlineCacheGenerator.cpp:
(JSC::JITByIdGenerator::finalize):
* jit/Repatch.cpp:
(JSC::replaceWithJump):
(JSC::patchJumpToGetByIdStub):
(JSC::tryCachePutByID):
(JSC::tryBuildPutByIdList):
(JSC::tryRepatchIn):
(JSC::resetGetByID):
(JSC::resetPutByID):
(JSC::resetIn):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (160492 => 160493)
--- trunk/Source/_javascript_Core/ChangeLog 2013-12-12 18:31:47 UTC (rev 160492)
+++ trunk/Source/_javascript_Core/ChangeLog 2013-12-12 18:38:39 UTC (rev 160493)
@@ -1,3 +1,42 @@
+2013-12-11 Filip Pizlo <[email protected]>
+
+ ARM64: Hang running pdfjs test, suspect DFG generated code for "in"
+ https://bugs.webkit.org/show_bug.cgi?id=124727
+ <rdar://problem/15566923>
+
+ Reviewed by Michael Saboff.
+
+ Get rid of In's hackish use of StructureStubInfo. Previously it was using hotPathBegin,
+ and it was the only IC that used that field, which was wasteful. Moreover, it used it
+ to store two separate locations: the label for patching the jump and the label right
+ after the jump. The code was relying on those two being the same label, which is true
+ on X86 and some other platforms, but it isn't true on ARM64.
+
+ This gets rid of hotPathBegin and makes In express those two locations as offsets from
+ the callReturnLocation, which is analogous to what the other IC's do.
+
+ This fixes a bug where any successful In patching would result in a trivially infinite
+ loop - and hence a hang - on ARM64.
+
+ * bytecode/StructureStubInfo.h:
+ * dfg/DFGJITCompiler.cpp:
+ (JSC::DFG::JITCompiler::link):
+ * dfg/DFGJITCompiler.h:
+ (JSC::DFG::InRecord::InRecord):
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::compileIn):
+ * jit/JITInlineCacheGenerator.cpp:
+ (JSC::JITByIdGenerator::finalize):
+ * jit/Repatch.cpp:
+ (JSC::replaceWithJump):
+ (JSC::patchJumpToGetByIdStub):
+ (JSC::tryCachePutByID):
+ (JSC::tryBuildPutByIdList):
+ (JSC::tryRepatchIn):
+ (JSC::resetGetByID):
+ (JSC::resetPutByID):
+ (JSC::resetIn):
+
2013-12-11 Joseph Pecoraro <[email protected]>
Web Inspector: Push More Inspector Required Classes Down into _javascript_Core
Modified: trunk/Source/_javascript_Core/bytecode/StructureStubInfo.h (160492 => 160493)
--- trunk/Source/_javascript_Core/bytecode/StructureStubInfo.h 2013-12-12 18:31:47 UTC (rev 160492)
+++ trunk/Source/_javascript_Core/bytecode/StructureStubInfo.h 2013-12-12 18:38:39 UTC (rev 160493)
@@ -234,7 +234,7 @@
RegisterSet usedRegisters;
int32_t deltaCallToDone;
int32_t deltaCallToStorageLoad;
- int32_t deltaCallToStructCheck;
+ int32_t deltaCallToJump;
int32_t deltaCallToSlowCase;
int32_t deltaCheckImmToCall;
#if USE(JSVALUE64)
@@ -291,7 +291,6 @@
RefPtr<JITStubRoutine> stubRoutine;
CodeLocationCall callReturnLocation;
- CodeLocationLabel hotPathBegin; // FIXME: This is only used by DFG In IC.
RefPtr<WatchpointsOnStructureStubInfo> watchpoints;
};
Modified: trunk/Source/_javascript_Core/dfg/DFGJITCompiler.cpp (160492 => 160493)
--- trunk/Source/_javascript_Core/dfg/DFGJITCompiler.cpp 2013-12-12 18:31:47 UTC (rev 160492)
+++ trunk/Source/_javascript_Core/dfg/DFGJITCompiler.cpp 2013-12-12 18:38:39 UTC (rev 160493)
@@ -221,9 +221,9 @@
for (unsigned i = 0; i < m_ins.size(); ++i) {
StructureStubInfo& info = *m_ins[i].m_stubInfo;
- CodeLocationLabel jump = linkBuffer.locationOf(m_ins[i].m_jump);
CodeLocationCall callReturnLocation = linkBuffer.locationOf(m_ins[i].m_slowPathGenerator->call());
- info.hotPathBegin = jump;
+ info.patch.deltaCallToDone = differenceBetweenCodePtr(callReturnLocation, linkBuffer.locationOf(m_ins[i].m_done));
+ info.patch.deltaCallToJump = differenceBetweenCodePtr(callReturnLocation, linkBuffer.locationOf(m_ins[i].m_jump));
info.callReturnLocation = callReturnLocation;
info.patch.deltaCallToSlowCase = differenceBetweenCodePtr(callReturnLocation, linkBuffer.locationOf(m_ins[i].m_slowPathGenerator->label()));
}
Modified: trunk/Source/_javascript_Core/dfg/DFGJITCompiler.h (160492 => 160493)
--- trunk/Source/_javascript_Core/dfg/DFGJITCompiler.h 2013-12-12 18:31:47 UTC (rev 160492)
+++ trunk/Source/_javascript_Core/dfg/DFGJITCompiler.h 2013-12-12 18:38:39 UTC (rev 160493)
@@ -81,15 +81,17 @@
struct InRecord {
InRecord(
- MacroAssembler::PatchableJump jump, SlowPathGenerator* slowPathGenerator,
- StructureStubInfo* stubInfo)
+ MacroAssembler::PatchableJump jump, MacroAssembler::Label done,
+ SlowPathGenerator* slowPathGenerator, StructureStubInfo* stubInfo)
: m_jump(jump)
+ , m_done(done)
, m_slowPathGenerator(slowPathGenerator)
, m_stubInfo(stubInfo)
{
}
MacroAssembler::PatchableJump m_jump;
+ MacroAssembler::Label m_done;
SlowPathGenerator* m_slowPathGenerator;
StructureStubInfo* m_stubInfo;
};
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (160492 => 160493)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2013-12-12 18:31:47 UTC (rev 160492)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2013-12-12 18:38:39 UTC (rev 160493)
@@ -972,8 +972,9 @@
GPRReg resultGPR = result.gpr();
use(node->child1());
-
+
MacroAssembler::PatchableJump jump = m_jit.patchableJump();
+ MacroAssembler::Label done = m_jit.label();
OwnPtr<SlowPathGenerator> slowPath = slowPathCall(
jump.m_jump, this, operationInOptimize,
@@ -986,7 +987,7 @@
stubInfo->patch.usedRegisters = usedRegisters();
stubInfo->patch.registersFlushed = false;
- m_jit.addIn(InRecord(jump, slowPath.get(), stubInfo));
+ m_jit.addIn(InRecord(jump, done, slowPath.get(), stubInfo));
addSlowPathGenerator(slowPath.release());
base.use();
Modified: trunk/Source/_javascript_Core/jit/JITInlineCacheGenerator.cpp (160492 => 160493)
--- trunk/Source/_javascript_Core/jit/JITInlineCacheGenerator.cpp 2013-12-12 18:31:47 UTC (rev 160492)
+++ trunk/Source/_javascript_Core/jit/JITInlineCacheGenerator.cpp 2013-12-12 18:38:39 UTC (rev 160493)
@@ -76,7 +76,7 @@
m_stubInfo->callReturnLocation = callReturnLocation;
m_stubInfo->patch.deltaCheckImmToCall = MacroAssembler::differenceBetweenCodePtr(
fastPath.locationOf(m_structureImm), callReturnLocation);
- m_stubInfo->patch.deltaCallToStructCheck = MacroAssembler::differenceBetweenCodePtr(
+ m_stubInfo->patch.deltaCallToJump = MacroAssembler::differenceBetweenCodePtr(
callReturnLocation, fastPath.locationOf(m_structureCheck));
#if USE(JSVALUE64)
m_stubInfo->patch.deltaCallToLoadOrStore = MacroAssembler::differenceBetweenCodePtr(
Modified: trunk/Source/_javascript_Core/jit/Repatch.cpp (160492 => 160493)
--- trunk/Source/_javascript_Core/jit/Repatch.cpp 2013-12-12 18:31:47 UTC (rev 160492)
+++ trunk/Source/_javascript_Core/jit/Repatch.cpp 2013-12-12 18:38:39 UTC (rev 160493)
@@ -174,7 +174,7 @@
repatchBuffer.relink(
stubInfo.callReturnLocation.jumpAtOffset(
- stubInfo.patch.deltaCallToStructCheck),
+ stubInfo.patch.deltaCallToJump),
CodeLocationLabel(target));
}
@@ -457,7 +457,7 @@
if (stubInfo.u.getByIdSelfList.didSelfPatching) {
repatchBuffer.relink(
stubInfo.callReturnLocation.jumpAtOffset(
- stubInfo.patch.deltaCallToStructCheck),
+ stubInfo.patch.deltaCallToJump),
CodeLocationLabel(stubRoutine->code().code()));
return;
}
@@ -1033,7 +1033,7 @@
RepatchBuffer repatchBuffer(codeBlock);
repatchBuffer.relink(
stubInfo.callReturnLocation.jumpAtOffset(
- stubInfo.patch.deltaCallToStructCheck),
+ stubInfo.patch.deltaCallToJump),
CodeLocationLabel(stubInfo.stubRoutine->code().code()));
repatchCall(repatchBuffer, stubInfo.callReturnLocation, appropriateListBuildingPutByIdFunction(slot, putKind));
@@ -1137,7 +1137,7 @@
}
RepatchBuffer repatchBuffer(codeBlock);
- repatchBuffer.relink(stubInfo.callReturnLocation.jumpAtOffset(stubInfo.patch.deltaCallToStructCheck), CodeLocationLabel(stubRoutine->code().code()));
+ repatchBuffer.relink(stubInfo.callReturnLocation.jumpAtOffset(stubInfo.patch.deltaCallToJump), CodeLocationLabel(stubRoutine->code().code()));
if (list->isFull())
repatchCall(repatchBuffer, stubInfo.callReturnLocation, appropriateGenericPutByIdFunction(slot, putKind));
@@ -1181,7 +1181,7 @@
PolymorphicAccessStructureList* polymorphicStructureList;
int listIndex;
- CodeLocationLabel successLabel = stubInfo.hotPathBegin;
+ CodeLocationLabel successLabel = stubInfo.callReturnLocation.labelAtOffset(stubInfo.patch.deltaCallToDone);
CodeLocationLabel slowCaseLabel;
if (stubInfo.accessType == access_unset) {
@@ -1259,7 +1259,7 @@
stubInfo.u.inList.listSize++;
RepatchBuffer repatchBuffer(codeBlock);
- repatchBuffer.relink(stubInfo.hotPathBegin.jumpAtOffset(0), CodeLocationLabel(stubRoutine->code().code()));
+ repatchBuffer.relink(stubInfo.callReturnLocation.jumpAtOffset(stubInfo.patch.deltaCallToJump), CodeLocationLabel(stubRoutine->code().code()));
return listIndex < (POLYMORPHIC_LIST_CACHE_SIZE - 1);
}
@@ -1432,7 +1432,7 @@
repatchBuffer.repatch(stubInfo.callReturnLocation.dataLabelCompactAtOffset(stubInfo.patch.deltaCallToTagLoadOrStore), 0);
repatchBuffer.repatch(stubInfo.callReturnLocation.dataLabelCompactAtOffset(stubInfo.patch.deltaCallToPayloadLoadOrStore), 0);
#endif
- repatchBuffer.relink(stubInfo.callReturnLocation.jumpAtOffset(stubInfo.patch.deltaCallToStructCheck), stubInfo.callReturnLocation.labelAtOffset(stubInfo.patch.deltaCallToSlowCase));
+ repatchBuffer.relink(stubInfo.callReturnLocation.jumpAtOffset(stubInfo.patch.deltaCallToJump), stubInfo.callReturnLocation.labelAtOffset(stubInfo.patch.deltaCallToSlowCase));
}
void resetPutByID(RepatchBuffer& repatchBuffer, StructureStubInfo& stubInfo)
@@ -1466,12 +1466,12 @@
repatchBuffer.repatch(stubInfo.callReturnLocation.dataLabel32AtOffset(stubInfo.patch.deltaCallToTagLoadOrStore), 0);
repatchBuffer.repatch(stubInfo.callReturnLocation.dataLabel32AtOffset(stubInfo.patch.deltaCallToPayloadLoadOrStore), 0);
#endif
- repatchBuffer.relink(stubInfo.callReturnLocation.jumpAtOffset(stubInfo.patch.deltaCallToStructCheck), stubInfo.callReturnLocation.labelAtOffset(stubInfo.patch.deltaCallToSlowCase));
+ repatchBuffer.relink(stubInfo.callReturnLocation.jumpAtOffset(stubInfo.patch.deltaCallToJump), stubInfo.callReturnLocation.labelAtOffset(stubInfo.patch.deltaCallToSlowCase));
}
void resetIn(RepatchBuffer& repatchBuffer, StructureStubInfo& stubInfo)
{
- repatchBuffer.relink(stubInfo.hotPathBegin.jumpAtOffset(0), stubInfo.callReturnLocation.labelAtOffset(stubInfo.patch.deltaCallToSlowCase));
+ repatchBuffer.relink(stubInfo.callReturnLocation.jumpAtOffset(stubInfo.patch.deltaCallToJump), stubInfo.callReturnLocation.labelAtOffset(stubInfo.patch.deltaCallToSlowCase));
}
} // namespace JSC
