Don't set up the callsite to operationGetByValDefault when the optimization is alread...

https://bugs.webkit.org/show_bug.cgi?id=147577

Reviewed by Filip Pizlo.

operationGetByValDefault should be called only when the IC is not set.
operationGetByValString breaks this invariant and `ASSERT(!byValInfo.stubRoutine)` in
operationGetByValDefault raises the assertion failure.
In this patch, we change the callsite setting up code in operationGetByValString when
the IC is already set. And to make the operation's meaning explicitly, we changed the
name operationGetByValDefault to operationGetByValOptimize, that is aligned to the
GetById case.

* jit/JITOperations.cpp:
* jit/JITOperations.h:
* jit/JITPropertyAccess.cpp:
(JSC::JIT::emitSlow_op_get_by_val):
* jit/JITPropertyAccess32_64.cpp:
(JSC::JIT::emitSlow_op_get_by_val):
* tests/stress/operation-get-by-val-default-should-not-called-for-already-optimized-site.js: Added.
(hello):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@187750 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 93e2983..77e96f4 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,27 @@
+2015-08-03  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        Don't set up the callsite to operationGetByValDefault when the optimization is already done
+        https://bugs.webkit.org/show_bug.cgi?id=147577
+
+        Reviewed by Filip Pizlo.
+
+        operationGetByValDefault should be called only when the IC is not set.
+        operationGetByValString breaks this invariant and `ASSERT(!byValInfo.stubRoutine)` in
+        operationGetByValDefault raises the assertion failure.
+        In this patch, we change the callsite setting up code in operationGetByValString when
+        the IC is already set. And to make the operation's meaning explicitly, we changed the
+        name operationGetByValDefault to operationGetByValOptimize, that is aligned to the
+        GetById case.
+
+        * jit/JITOperations.cpp:
+        * jit/JITOperations.h:
+        * jit/JITPropertyAccess.cpp:
+        (JSC::JIT::emitSlow_op_get_by_val):
+        * jit/JITPropertyAccess32_64.cpp:
+        (JSC::JIT::emitSlow_op_get_by_val):
+        * tests/stress/operation-get-by-val-default-should-not-called-for-already-optimized-site.js: Added.
+        (hello):
+
 2015-08-03  Csaba Osztrogonác  <ossy@webkit.org>
 
         [FTL] Remove unused scripts related to native call inlining

diff --git a/Source/JavaScriptCore/jit/JITOperations.cpp b/Source/JavaScriptCore/jit/JITOperations.cpp
index 4b58b44..8a6975c 100644 (file)
--- a/Source/JavaScriptCore/jit/JITOperations.cpp
+++ b/Source/JavaScriptCore/jit/JITOperations.cpp
@@ -1509,7 +1509,7 @@ EncodedJSValue JIT_OPERATION operationGetByValGeneric(ExecState* exec, EncodedJS
     return JSValue::encode(result);
 }
 
-EncodedJSValue JIT_OPERATION operationGetByValDefault(ExecState* exec, EncodedJSValue encodedBase, EncodedJSValue encodedSubscript, ArrayProfile* arrayProfile)
+EncodedJSValue JIT_OPERATION operationGetByValOptimize(ExecState* exec, EncodedJSValue encodedBase, EncodedJSValue encodedSubscript, ArrayProfile* arrayProfile)
 {
     VM& vm = exec->vm();
     NativeCallFrameTracer tracer(&vm, exec);
@@ -1643,8 +1643,12 @@ EncodedJSValue JIT_OPERATION operationGetByValString(ExecState* exec, EncodedJSV
             result = asString(baseValue)->getIndex(exec, i);
         else {
             result = baseValue.get(exec, i);
-            if (!isJSString(baseValue))
-                ctiPatchCallByReturnAddress(exec->codeBlock(), ReturnAddressPtr(OUR_RETURN_ADDRESS), FunctionPtr(operationGetByValDefault));
+            if (!isJSString(baseValue)) {
+                unsigned bytecodeOffset = exec->locationAsBytecodeOffset();
+                ASSERT(bytecodeOffset);
+                ByValInfo& byValInfo = exec->codeBlock()->getByValInfo(bytecodeOffset - 1);
+                ctiPatchCallByReturnAddress(exec->codeBlock(), ReturnAddressPtr(OUR_RETURN_ADDRESS), FunctionPtr(byValInfo.stubRoutine ? operationGetByValGeneric : operationGetByValOptimize));
+            }
         }
     } else {
         baseValue.requireObjectCoercible(exec);

diff --git a/Source/JavaScriptCore/jit/JITOperations.h b/Source/JavaScriptCore/jit/JITOperations.h
index 1709d25..cacff2c 100644 (file)
--- a/Source/JavaScriptCore/jit/JITOperations.h
+++ b/Source/JavaScriptCore/jit/JITOperations.h
@@ -310,7 +310,7 @@ void JIT_OPERATION operationPopScope(ExecState*, int32_t) WTF_INTERNAL;
 void JIT_OPERATION operationProfileDidCall(ExecState*, EncodedJSValue) WTF_INTERNAL;
 void JIT_OPERATION operationProfileWillCall(ExecState*, EncodedJSValue) WTF_INTERNAL;
 EncodedJSValue JIT_OPERATION operationCheckHasInstance(ExecState*, EncodedJSValue, EncodedJSValue baseVal) WTF_INTERNAL;
-EncodedJSValue JIT_OPERATION operationGetByValDefault(ExecState*, EncodedJSValue encodedBase, EncodedJSValue encodedSubscript, ArrayProfile*) WTF_INTERNAL;
+EncodedJSValue JIT_OPERATION operationGetByValOptimize(ExecState*, EncodedJSValue encodedBase, EncodedJSValue encodedSubscript, ArrayProfile*) WTF_INTERNAL;
 EncodedJSValue JIT_OPERATION operationGetByValGeneric(ExecState*, EncodedJSValue encodedBase, EncodedJSValue encodedSubscript, ArrayProfile*) WTF_INTERNAL;
 EncodedJSValue JIT_OPERATION operationGetByValString(ExecState*, EncodedJSValue encodedBase, EncodedJSValue encodedSubscript) WTF_INTERNAL;
 EncodedJSValue JIT_OPERATION operationHasIndexedPropertyDefault(ExecState*, EncodedJSValue encodedBase, EncodedJSValue encodedSubscript, ArrayProfile*) WTF_INTERNAL;

diff --git a/Source/JavaScriptCore/jit/JITPropertyAccess.cpp b/Source/JavaScriptCore/jit/JITPropertyAccess.cpp
index c0a54b7..4ef06d9 100644 (file)
--- a/Source/JavaScriptCore/jit/JITPropertyAccess.cpp
+++ b/Source/JavaScriptCore/jit/JITPropertyAccess.cpp
@@ -224,7 +224,7 @@ void JIT::emitSlow_op_get_by_val(Instruction* currentInstruction, Vector<SlowCas
     
     emitGetVirtualRegister(base, regT0);
     emitGetVirtualRegister(property, regT1);
-    Call call = callOperation(operationGetByValDefault, dst, regT0, regT1, profile);
+    Call call = callOperation(operationGetByValOptimize, dst, regT0, regT1, profile);
 
     m_byValCompilationInfo[m_byValInstructionIndex].slowPathTarget = slowPath;
     m_byValCompilationInfo[m_byValInstructionIndex].returnAddress = call;

diff --git a/Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp b/Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp
index 148d76c..609ca13 100644 (file)
--- a/Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp
+++ b/Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp
@@ -265,7 +265,7 @@ void JIT::emitSlow_op_get_by_val(Instruction* currentInstruction, Vector<SlowCas
     
     emitLoad(base, regT1, regT0);
     emitLoad(property, regT3, regT2);
-    Call call = callOperation(operationGetByValDefault, dst, regT1, regT0, regT3, regT2, profile);
+    Call call = callOperation(operationGetByValOptimize, dst, regT1, regT0, regT3, regT2, profile);
 
     m_byValCompilationInfo[m_byValInstructionIndex].slowPathTarget = slowPath;
     m_byValCompilationInfo[m_byValInstructionIndex].returnAddress = call;

diff --git a/Source/JavaScriptCore/tests/stress/operation-get-by-val-default-should-not-called-for-already-optimized-site.js b/Source/JavaScriptCore/tests/stress/operation-get-by-val-default-should-not-called-for-already-optimized-site.js
new file mode 100644 (file)
index 0000000..d9e24c3
--- /dev/null
+++ b/Source/JavaScriptCore/tests/stress/operation-get-by-val-default-should-not-called-for-already-optimized-site.js
@@ -0,0 +1,12 @@
+function hello(object, name)
+{
+    return object[name];
+}
+noInline(hello);
+for (var i = 0; i < 100; ++i)
+    hello([0,1,2,3], 1);
+hello([0.1,0.2,0.3,0.4], 1);
+hello('string', 1);
+hello('string', 1);
+hello([true, false, true, false], 1);
+hello([true, false, true, false], 1);