Changeset 132757 in webkit

Timestamp:

Oct 28, 2012 7:16:27 PM (11 years ago)

Author:

fpizlo@apple.com

Message:

There should not be blind spots in array length array profiling
​https://bugs.webkit.org/show_bug.cgi?id=100620

Reviewed by Oliver Hunt.

I don't think this has any performance impact. But it's good to not have random
programs occasionally emit a GetById for array length accesses.

• jit/JITPropertyAccess.cpp:

(JSC::JIT::compileGetByIdHotPath):
(JSC::JIT::privateCompilePatchGetArrayLength):

• jit/JITPropertyAccess32_64.cpp:

(JSC::JIT::compileGetByIdHotPath):
(JSC::JIT::privateCompilePatchGetArrayLength):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• jit/JIT.h (modified) (1 diff)
• jit/JITPropertyAccess.cpp (modified) (3 diffs)
• jit/JITPropertyAccess32_64.cpp (modified) (6 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2012-10-28  Filip Pizlo  <fpizlo@apple.com>
+
+        There should not be blind spots in array length array profiling
+        https://bugs.webkit.org/show_bug.cgi?id=100620
+
+        Reviewed by Oliver Hunt.
+
+        I don't think this has any performance impact. But it's good to not have random
+        programs occasionally emit a GetById for array length accesses.
+
+        * jit/JITPropertyAccess.cpp:
+        (JSC::JIT::compileGetByIdHotPath):
+        (JSC::JIT::privateCompilePatchGetArrayLength):
+        * jit/JITPropertyAccess32_64.cpp:
+        (JSC::JIT::compileGetByIdHotPath):
+        (JSC::JIT::privateCompilePatchGetArrayLength):
+
 2012-10-28  Filip Pizlo  <fpizlo@apple.com>
 

trunk/Source/JavaScriptCore/jit/JIT.h

@@ -544 +544 @@
         void emitJumpSlowCaseIfNotJSCell(int virtualRegisterIndex, RegisterID tag);
 
-        void compileGetByIdHotPath();
+        void compileGetByIdHotPath(Identifier*);
         void compileGetByIdSlowCase(int resultVReg, int baseVReg, Identifier* ident, Vector<SlowCaseEntry>::iterator& iter, bool isMethodCheck = false);
         void compileGetDirectOffset(RegisterID base, RegisterID resultTag, RegisterID resultPayload, PropertyOffset cachedOffset);

trunk/Source/JavaScriptCore/jit/JITPropertyAccess.cpp

@@ -518 +518 @@
 }
 
-void JIT::compileGetByIdHotPath(int baseVReg, Identifier*)
+void JIT::compileGetByIdHotPath(int baseVReg, Identifier* ident)
 {
     // As for put_by_id, get_by_id requires the offset of the Structure and the offset of the access to be patched.
@@ -526 +526 @@
 
     emitJumpSlowCaseIfNotJSCell(regT0, baseVReg);
+    
+    if (*ident == m_globalData->propertyNames->length && canBeOptimized()) {
+        loadPtr(Address(regT0, JSCell::structureOffset()), regT1);
+        emitArrayProfilingSiteForBytecodeIndex(regT1, regT2, m_bytecodeOffset);
+    }
 
     BEGIN_UNINTERRUPTED_SEQUENCE(sequenceGetByIdHotPath);
@@ -789 +794 @@
     // Check eax is an array
     loadPtr(Address(regT0, JSCell::structureOffset()), regT2);
-    emitArrayProfilingSiteForBytecodeIndex(regT2, regT1, stubInfo->bytecodeIndex);
     Jump failureCases1 = branchTest32(Zero, regT2, TrustedImm32(IsArray));
     Jump failureCases2 = branchTest32(Zero, regT2, TrustedImm32(IndexingShapeMask));

trunk/Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp

@@ -106 +106 @@
     int dst = currentInstruction[1].u.operand;
     int base = currentInstruction[2].u.operand;
+    Identifier* ident = &(m_codeBlock->identifier(currentInstruction[3].u.operand));
     
     emitLoad(base, regT1, regT0);
@@ -130 +131 @@
     // Do a regular(ish) get_by_id (the slow case will be link to
     // cti_op_get_by_id_method_check instead of cti_op_get_by_id.
-    compileGetByIdHotPath();
+    compileGetByIdHotPath(ident);
     
     match.link(this);
@@ -454 +455 @@
     int dst = currentInstruction[1].u.operand;
     int base = currentInstruction[2].u.operand;
+    Identifier* ident = &(m_codeBlock->identifier(currentInstruction[3].u.operand));
     
     emitLoad(base, regT1, regT0);
     emitJumpSlowCaseIfNotJSCell(base, regT1);
-    compileGetByIdHotPath();
+    compileGetByIdHotPath(ident);
     emitValueProfilingSite();
     emitStore(dst, regT1, regT0);
@@ -463 +465 @@
 }
 
-void JIT::compileGetByIdHotPath()
+void JIT::compileGetByIdHotPath(Identifier* ident)
 {
     // As for put_by_id, get_by_id requires the offset of the Structure and the offset of the access to be patched.
@@ -470 +472 @@
     // to jump back to if one of these trampolies finds a match.
     
+    if (*ident == m_globalData->propertyNames->length && canBeOptimized()) {
+        loadPtr(Address(regT0, JSCell::structureOffset()), regT2);
+        emitArrayProfilingSiteForBytecodeIndex(regT2, regT3, m_bytecodeOffset);
+    }
+
     BEGIN_UNINTERRUPTED_SEQUENCE(sequenceGetByIdHotPath);
     
@@ -752 +759 @@
     // Check for array
     loadPtr(Address(regT0, JSCell::structureOffset()), regT2);
-    emitArrayProfilingSiteForBytecodeIndex(regT2, regT3, stubInfo->bytecodeIndex);
     Jump failureCases1 = branchTest32(Zero, regT2, TrustedImm32(IsArray));
     Jump failureCases2 = branchTest32(Zero, regT2, TrustedImm32(IndexingShapeMask));