DFG OSR exit doesn't know which virtual register to use for the last result register for post_inc and post_dec
https://bugs.webkit.org/show_bug.cgi?id=109036
<rdar://problem/13292139>
Source/JavaScriptCore:
Reviewed by Gavin Barraclough.
This was a two-fold problem:
1) post_inc/dec has two results - the new value of the variable, and the old value of the variable. DFG OSR exit
assumed that the "last result" used for the Baseline JIT's register allocation would be the new value. It was
wrong in this assumption.
2) The Baseline JIT knew to disable its last result optimization in cases where it might confuse the DFG. But it
was doing this only for code blocks that could be totally optimized, but not code blocks that could only be
optimized when inlined.
This patch introduces a more rigorous notion of when the Baseline JIT emits profiling, when it does extra work
to account for the possibility of OSR exit, and when it does extra work to account for the possibility of OSR
entry. These notions are called shouldEmitProfiling(), canBeOptimizedOrInlined(), and canBeOptimized(),
respectively.
This is performance-neutral and fixes the reported bug. It probably fixes other bugs as well, since previously
we for example weren't doing the more conservative implementation of op_mov in the Baseline JIT for code blocks
that could be inlined but not optimized. So, if such a code block OSR exited at just the right point, you'd get
symptoms similar to this bug.
* dfg/DFGCapabilities.h:
(JSC::DFG::canCompileOpcode):
* dfg/DFGCommon.h:
* jit/JIT.cpp:
(JSC::JIT::privateCompile):
* jit/JIT.h:
(JSC::JIT::compilePatchGetArrayLength):
(JSC::JIT::canBeOptimizedOrInlined):
(JIT):
* jit/JITArithmetic.cpp:
(JSC::JIT::emit_op_post_inc):
(JSC::JIT::emit_op_post_dec):
* jit/JITArithmetic32_64.cpp:
(JSC::JIT::emit_op_post_inc):
(JSC::JIT::emit_op_post_dec):
* jit/JITCall.cpp:
(JSC::JIT::emit_op_call_put_result):
(JSC::JIT::compileOpCall):
* jit/JITCall32_64.cpp:
(JSC::JIT::compileOpCall):
* jit/JITInlines.h:
(JSC::JIT::emitArrayProfilingSite):
(JSC::JIT::map):
* jit/JITOpcodes.cpp:
(JSC::JIT::emit_op_mov):
* jit/JITPropertyAccess.cpp:
(JSC::JIT::compileGetByIdHotPath):
(JSC::JIT::privateCompilePutByIdTransition):
* jit/JITPropertyAccess32_64.cpp:
(JSC::JIT::compileGetByIdHotPath):
(JSC::JIT::privateCompilePutByIdTransition):
LayoutTests:
Reviewed by Gavin Barraclough.
* fast/js/dfg-post-inc-then-exit-expected.txt: Added.
* fast/js/dfg-post-inc-then-exit.html: Added.
* fast/js/jsc-test-list:
* fast/js/script-tests/dfg-post-inc-then-exit.js: Added.
(foo):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@144137 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 480a1f9..34b5ccc 100644
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,63 @@
+2013-02-26 Filip Pizlo <fpizlo@apple.com>
+
+ DFG OSR exit doesn't know which virtual register to use for the last result register for post_inc and post_dec
+ https://bugs.webkit.org/show_bug.cgi?id=109036
+ <rdar://problem/13292139>
+
+ Reviewed by Gavin Barraclough.
+
+ This was a two-fold problem:
+
+ 1) post_inc/dec has two results - the new value of the variable, and the old value of the variable. DFG OSR exit
+ assumed that the "last result" used for the Baseline JIT's register allocation would be the new value. It was
+ wrong in this assumption.
+
+ 2) The Baseline JIT knew to disable its last result optimization in cases where it might confuse the DFG. But it
+ was doing this only for code blocks that could be totally optimized, but not code blocks that could only be
+ optimized when inlined.
+
+ This patch introduces a more rigorous notion of when the Baseline JIT emits profiling, when it does extra work
+ to account for the possibility of OSR exit, and when it does extra work to account for the possibility of OSR
+ entry. These notions are called shouldEmitProfiling(), canBeOptimizedOrInlined(), and canBeOptimized(),
+ respectively.
+
+ This is performance-neutral and fixes the reported bug. It probably fixes other bugs as well, since previously
+ we for example weren't doing the more conservative implementation of op_mov in the Baseline JIT for code blocks
+ that could be inlined but not optimized. So, if such a code block OSR exited at just the right point, you'd get
+ symptoms similar to this bug.
+
+ * dfg/DFGCapabilities.h:
+ (JSC::DFG::canCompileOpcode):
+ * dfg/DFGCommon.h:
+ * jit/JIT.cpp:
+ (JSC::JIT::privateCompile):
+ * jit/JIT.h:
+ (JSC::JIT::compilePatchGetArrayLength):
+ (JSC::JIT::canBeOptimizedOrInlined):
+ (JIT):
+ * jit/JITArithmetic.cpp:
+ (JSC::JIT::emit_op_post_inc):
+ (JSC::JIT::emit_op_post_dec):
+ * jit/JITArithmetic32_64.cpp:
+ (JSC::JIT::emit_op_post_inc):
+ (JSC::JIT::emit_op_post_dec):
+ * jit/JITCall.cpp:
+ (JSC::JIT::emit_op_call_put_result):
+ (JSC::JIT::compileOpCall):
+ * jit/JITCall32_64.cpp:
+ (JSC::JIT::compileOpCall):
+ * jit/JITInlines.h:
+ (JSC::JIT::emitArrayProfilingSite):
+ (JSC::JIT::map):
+ * jit/JITOpcodes.cpp:
+ (JSC::JIT::emit_op_mov):
+ * jit/JITPropertyAccess.cpp:
+ (JSC::JIT::compileGetByIdHotPath):
+ (JSC::JIT::privateCompilePutByIdTransition):
+ * jit/JITPropertyAccess32_64.cpp:
+ (JSC::JIT::compileGetByIdHotPath):
+ (JSC::JIT::privateCompilePutByIdTransition):
+
2013-02-26 Roger Fong <roger_fong@apple.com>
Unreviewed. AppleWin VS2010 build fix.
diff --git a/Source/JavaScriptCore/dfg/DFGCapabilities.h b/Source/JavaScriptCore/dfg/DFGCapabilities.h
index 44fca9c..6064e3e 100644
--- a/Source/JavaScriptCore/dfg/DFGCapabilities.h
+++ b/Source/JavaScriptCore/dfg/DFGCapabilities.h
@@ -193,7 +193,7 @@
return CanCompile;
case op_call_varargs:
- return ShouldProfile;
+ return MayInline;
case op_resolve:
case op_resolve_global_property:
diff --git a/Source/JavaScriptCore/dfg/DFGCommon.h b/Source/JavaScriptCore/dfg/DFGCommon.h
index 05c1de1..a832487 100644
--- a/Source/JavaScriptCore/dfg/DFGCommon.h
+++ b/Source/JavaScriptCore/dfg/DFGCommon.h
@@ -246,7 +246,7 @@
// Put things here that must be defined even if ENABLE(DFG_JIT) is false.
-enum CapabilityLevel { CannotCompile, ShouldProfile, CanCompile, CapabilityLevelNotSet };
+enum CapabilityLevel { CannotCompile, MayInline, CanCompile, CapabilityLevelNotSet };
// Unconditionally disable DFG disassembly support if the DFG is not compiled in.
inline bool shouldShowDisassembly()
diff --git a/Source/JavaScriptCore/jit/JIT.cpp b/Source/JavaScriptCore/jit/JIT.cpp
index ba78bc8..2e8fced 100644
--- a/Source/JavaScriptCore/jit/JIT.cpp
+++ b/Source/JavaScriptCore/jit/JIT.cpp
@@ -627,12 +627,14 @@
m_canBeOptimized = false;
m_shouldEmitProfiling = false;
break;
- case DFG::ShouldProfile:
+ case DFG::MayInline:
m_canBeOptimized = false;
+ m_canBeOptimizedOrInlined = true;
m_shouldEmitProfiling = true;
break;
case DFG::CanCompile:
m_canBeOptimized = true;
+ m_canBeOptimizedOrInlined = true;
m_shouldEmitProfiling = true;
break;
default:
@@ -815,7 +817,7 @@
}
#if ENABLE(DFG_JIT) || ENABLE(LLINT)
- if (canBeOptimized()
+ if (canBeOptimizedOrInlined()
#if ENABLE(LLINT)
|| true
#endif
diff --git a/Source/JavaScriptCore/jit/JIT.h b/Source/JavaScriptCore/jit/JIT.h
index 1b32c9d..d21ccce 100644
--- a/Source/JavaScriptCore/jit/JIT.h
+++ b/Source/JavaScriptCore/jit/JIT.h
@@ -392,6 +392,8 @@
#if ENABLE(DFG_JIT)
// Force profiling to be enabled during stub generation.
jit.m_canBeOptimized = true;
+ jit.m_canBeOptimizedOrInlined = true;
+ jit.m_shouldEmitProfiling = true;
#endif // ENABLE(DFG_JIT)
return jit.privateCompilePatchGetArrayLength(returnAddress);
}
@@ -895,6 +897,7 @@
#if ENABLE(DFG_JIT)
bool canBeOptimized() { return m_canBeOptimized; }
+ bool canBeOptimizedOrInlined() { return m_canBeOptimizedOrInlined; }
bool shouldEmitProfiling() { return m_shouldEmitProfiling; }
#else
bool canBeOptimized() { return false; }
@@ -947,6 +950,7 @@
#if ENABLE(VALUE_PROFILER)
bool m_canBeOptimized;
+ bool m_canBeOptimizedOrInlined;
bool m_shouldEmitProfiling;
#endif
} JIT_CLASS_ALIGNMENT;
diff --git a/Source/JavaScriptCore/jit/JITArithmetic.cpp b/Source/JavaScriptCore/jit/JITArithmetic.cpp
index dfd0099..11123f7 100644
--- a/Source/JavaScriptCore/jit/JITArithmetic.cpp
+++ b/Source/JavaScriptCore/jit/JITArithmetic.cpp
@@ -641,6 +641,8 @@
emitFastArithIntToImmNoCheck(regT1, regT1);
emitPutVirtualRegister(srcDst, regT1);
emitPutVirtualRegister(result);
+ if (canBeOptimizedOrInlined())
+ killLastResultRegister();
}
void JIT::emitSlow_op_post_inc(Instruction* currentInstruction, Vector<SlowCaseEntry>::iterator& iter)
@@ -668,6 +670,8 @@
emitFastArithIntToImmNoCheck(regT1, regT1);
emitPutVirtualRegister(srcDst, regT1);
emitPutVirtualRegister(result);
+ if (canBeOptimizedOrInlined())
+ killLastResultRegister();
}
void JIT::emitSlow_op_post_dec(Instruction* currentInstruction, Vector<SlowCaseEntry>::iterator& iter)
diff --git a/Source/JavaScriptCore/jit/JITArithmetic32_64.cpp b/Source/JavaScriptCore/jit/JITArithmetic32_64.cpp
index 0955dd6..af5770e 100644
--- a/Source/JavaScriptCore/jit/JITArithmetic32_64.cpp
+++ b/Source/JavaScriptCore/jit/JITArithmetic32_64.cpp
@@ -467,6 +467,8 @@
emitStoreInt32(srcDst, regT2, true);
emitStoreAndMapInt32(dst, regT1, regT0, false, OPCODE_LENGTH(op_post_inc));
+ if (canBeOptimizedOrInlined())
+ unmap();
}
void JIT::emitSlow_op_post_inc(Instruction* currentInstruction, Vector<SlowCaseEntry>::iterator& iter)
@@ -502,6 +504,8 @@
emitStoreInt32(srcDst, regT2, true);
emitStoreAndMapInt32(dst, regT1, regT0, false, OPCODE_LENGTH(op_post_dec));
+ if (canBeOptimizedOrInlined())
+ unmap();
}
void JIT::emitSlow_op_post_dec(Instruction* currentInstruction, Vector<SlowCaseEntry>::iterator& iter)
diff --git a/Source/JavaScriptCore/jit/JITCall.cpp b/Source/JavaScriptCore/jit/JITCall.cpp
index cdd7528..fdd429b 100644
--- a/Source/JavaScriptCore/jit/JITCall.cpp
+++ b/Source/JavaScriptCore/jit/JITCall.cpp
@@ -56,7 +56,7 @@
int dst = instruction[1].u.operand;
emitValueProfilingSite();
emitPutVirtualRegister(dst);
- if (canBeOptimized())
+ if (canBeOptimizedOrInlined())
killLastResultRegister(); // Make lastResultRegister tracking simpler in the DFG.
}
@@ -168,7 +168,7 @@
int argCount = instruction[2].u.operand;
int registerOffset = instruction[3].u.operand;
- if (opcodeID == op_call && canBeOptimized()) {
+ if (opcodeID == op_call && shouldEmitProfiling()) {
emitGetVirtualRegister(registerOffset + CallFrame::argumentOffsetIncludingThis(0), regT0);
Jump done = emitJumpIfNotJSCell(regT0);
loadPtr(Address(regT0, JSCell::structureOffset()), regT0);
diff --git a/Source/JavaScriptCore/jit/JITCall32_64.cpp b/Source/JavaScriptCore/jit/JITCall32_64.cpp
index 90395b2..1c1f5ce 100644
--- a/Source/JavaScriptCore/jit/JITCall32_64.cpp
+++ b/Source/JavaScriptCore/jit/JITCall32_64.cpp
@@ -244,7 +244,7 @@
int argCount = instruction[2].u.operand;
int registerOffset = instruction[3].u.operand;
- if (opcodeID == op_call && canBeOptimized()) {
+ if (opcodeID == op_call && shouldEmitProfiling()) {
emitLoad(registerOffset + CallFrame::argumentOffsetIncludingThis(0), regT0, regT1);
Jump done = branch32(NotEqual, regT0, TrustedImm32(JSValue::CellTag));
loadPtr(Address(regT1, JSCell::structureOffset()), regT1);
diff --git a/Source/JavaScriptCore/jit/JITInlines.h b/Source/JavaScriptCore/jit/JITInlines.h
index 14b5b1e..b9e5152 100644
--- a/Source/JavaScriptCore/jit/JITInlines.h
+++ b/Source/JavaScriptCore/jit/JITInlines.h
@@ -386,7 +386,7 @@
RegisterID structure = structureAndIndexingType;
RegisterID indexingType = structureAndIndexingType;
- if (canBeOptimized())
+ if (shouldEmitProfiling())
storePtr(structure, arrayProfile->addressOfLastSeenStructure());
load8(Address(structure, Structure::indexingTypeOffset()), indexingType);
@@ -624,8 +624,8 @@
m_mappedTag = tag;
m_mappedPayload = payload;
- ASSERT(!canBeOptimized() || m_mappedPayload == regT0);
- ASSERT(!canBeOptimized() || m_mappedTag == regT1);
+ ASSERT(!canBeOptimizedOrInlined() || m_mappedPayload == regT0);
+ ASSERT(!canBeOptimizedOrInlined() || m_mappedTag == regT1);
}
inline void JIT::unmap(RegisterID registerID)
diff --git a/Source/JavaScriptCore/jit/JITOpcodes.cpp b/Source/JavaScriptCore/jit/JITOpcodes.cpp
index 259c6f2..c2ff959 100644
--- a/Source/JavaScriptCore/jit/JITOpcodes.cpp
+++ b/Source/JavaScriptCore/jit/JITOpcodes.cpp
@@ -53,7 +53,7 @@
int dst = currentInstruction[1].u.operand;
int src = currentInstruction[2].u.operand;
- if (canBeOptimized()) {
+ if (canBeOptimizedOrInlined()) {
// Use simpler approach, since the DFG thinks that the last result register
// is always set to the destination on every operation.
emitGetVirtualRegister(src, regT0);
diff --git a/Source/JavaScriptCore/jit/JITPropertyAccess.cpp b/Source/JavaScriptCore/jit/JITPropertyAccess.cpp
index ff6243e..df1efd8 100644
--- a/Source/JavaScriptCore/jit/JITPropertyAccess.cpp
+++ b/Source/JavaScriptCore/jit/JITPropertyAccess.cpp
@@ -535,7 +535,7 @@
emitJumpSlowCaseIfNotJSCell(regT0, baseVReg);
- if (*ident == m_globalData->propertyNames->length && canBeOptimized()) {
+ if (*ident == m_globalData->propertyNames->length && shouldEmitProfiling()) {
loadPtr(Address(regT0, JSCell::structureOffset()), regT1);
emitArrayProfilingSiteForBytecodeIndex(regT1, regT2, m_bytecodeOffset);
}
@@ -705,7 +705,7 @@
// If we succeed in all of our checks, and the code was optimizable, then make sure we
// decrement the rare case counter.
#if ENABLE(VALUE_PROFILER)
- if (m_codeBlock->canCompileWithDFG() >= DFG::ShouldProfile) {
+ if (m_codeBlock->canCompileWithDFG() >= DFG::MayInline) {
sub32(
TrustedImm32(1),
AbsoluteAddress(&m_codeBlock->rareCaseProfileForBytecodeOffset(stubInfo->bytecodeIndex)->m_counter));
diff --git a/Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp b/Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp
index bf02bcd..5d94599 100644
--- a/Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp
+++ b/Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp
@@ -477,7 +477,7 @@
// to array-length / prototype access tranpolines, and finally we also the the property-map access offset as a label
// to jump back to if one of these trampolies finds a match.
- if (*ident == m_globalData->propertyNames->length && canBeOptimized()) {
+ if (*ident == m_globalData->propertyNames->length && shouldEmitProfiling()) {
loadPtr(Address(regT0, JSCell::structureOffset()), regT2);
emitArrayProfilingSiteForBytecodeIndex(regT2, regT3, m_bytecodeOffset);
}
@@ -648,7 +648,7 @@
// If we succeed in all of our checks, and the code was optimizable, then make sure we
// decrement the rare case counter.
#if ENABLE(VALUE_PROFILER)
- if (m_codeBlock->canCompileWithDFG() >= DFG::ShouldProfile) {
+ if (m_codeBlock->canCompileWithDFG() >= DFG::MayInline) {
sub32(
TrustedImm32(1),
AbsoluteAddress(&m_codeBlock->rareCaseProfileForBytecodeOffset(stubInfo->bytecodeIndex)->m_counter));
