Skip to content

Commit

Permalink
IndexingType should not have a bit for each type
Browse files Browse the repository at this point in the history 
https://bugs.webkit.org/show_bug.cgi?id=98997

Reviewed by Oliver Hunt.

Somewhat incidentally, the introduction of butterflies led to each indexing
type being represented by a unique bit. This is superficially nice since it
allows you to test if a structure corresponds to a particular indexing type
by saying !!(structure->indexingType() & TheType). But the downside is that
given the 8 bits we have for the m_indexingType field, that leaves only a
small number of possible indexing types if we have one per bit.
        
This changeset changes the indexing type to be:
        
Bit #1: Tells you if you're an array.
        
Bits #2 - #5: 16 possible indexing types, including the blank type for
    objects that don't have indexed properties.
        
Bits #6-8: Auxiliary bits that we could use for other things. Currently we
    just use one of those bits, for MayHaveIndexedAccessors.
        
This is performance-neutral, and is primarily intended to give us more
breathing room for introducing new inferred array modes.

* assembler/AbstractMacroAssembler.h:
(JSC::AbstractMacroAssembler::JumpList::jumps):
* assembler/MacroAssembler.h:
(MacroAssembler):
(JSC::MacroAssembler::patchableBranch32):
* assembler/MacroAssemblerARMv7.h:
(JSC::MacroAssemblerARMv7::patchableBranch32):
(MacroAssemblerARMv7):
* dfg/DFGArrayMode.cpp:
(JSC::DFG::modeAlreadyChecked):
* dfg/DFGRepatch.cpp:
(JSC::DFG::tryCacheGetByID):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::speculationCheck):
(JSC::DFG::SpeculativeJIT::forwardSpeculationCheck):
(JSC::DFG::SpeculativeJIT::jumpSlowForUnwantedArrayMode):
(DFG):
(JSC::DFG::SpeculativeJIT::checkArray):
(JSC::DFG::SpeculativeJIT::arrayify):
* dfg/DFGSpeculativeJIT.h:
(SpeculativeJIT):
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* jit/JITInlineMethods.h:
(JSC::JIT::emitAllocateJSArray):
(JSC::JIT::chooseArrayMode):
* jit/JITPropertyAccess.cpp:
(JSC::JIT::emit_op_get_by_val):
(JSC::JIT::emitContiguousGetByVal):
(JSC::JIT::emitArrayStorageGetByVal):
(JSC::JIT::emit_op_put_by_val):
(JSC::JIT::emitContiguousPutByVal):
(JSC::JIT::emitArrayStoragePutByVal):
(JSC::JIT::privateCompilePatchGetArrayLength):
* jit/JITPropertyAccess32_64.cpp:
(JSC::JIT::emit_op_get_by_val):
(JSC::JIT::emitContiguousGetByVal):
(JSC::JIT::emitArrayStorageGetByVal):
(JSC::JIT::emit_op_put_by_val):
(JSC::JIT::emitContiguousPutByVal):
(JSC::JIT::emitArrayStoragePutByVal):
(JSC::JIT::privateCompilePatchGetArrayLength):
* llint/LowLevelInterpreter.asm:
* llint/LowLevelInterpreter32_64.asm:
* llint/LowLevelInterpreter64.asm:
* runtime/IndexingType.h:
(JSC):
(JSC::hasIndexedProperties):
(JSC::hasContiguous):
(JSC::hasFastArrayStorage):
(JSC::hasArrayStorage):
(JSC::shouldUseSlowPut):
* runtime/JSGlobalObject.cpp:
(JSC):
* runtime/StructureTransitionTable.h:
(JSC::newIndexingType):



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@131276 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
Filip Pizlo committed Oct 14, 2012
1 parent 13c9ce2 commit f4afbc5
Show file tree
Hide file tree
Showing 19 changed files with 293 additions and 174 deletions.
86 changes: 86 additions & 0 deletions Source/JavaScriptCore/ChangeLog
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,89 @@
2012-10-13 Filip Pizlo <fpizlo@apple.com>

IndexingType should not have a bit for each type
https://bugs.webkit.org/show_bug.cgi?id=98997

Reviewed by Oliver Hunt.

Somewhat incidentally, the introduction of butterflies led to each indexing
type being represented by a unique bit. This is superficially nice since it
allows you to test if a structure corresponds to a particular indexing type
by saying !!(structure->indexingType() & TheType). But the downside is that
given the 8 bits we have for the m_indexingType field, that leaves only a
small number of possible indexing types if we have one per bit.

This changeset changes the indexing type to be:

Bit #1: Tells you if you're an array.

Bits #2 - #5: 16 possible indexing types, including the blank type for
objects that don't have indexed properties.

Bits #6-8: Auxiliary bits that we could use for other things. Currently we
just use one of those bits, for MayHaveIndexedAccessors.

This is performance-neutral, and is primarily intended to give us more
breathing room for introducing new inferred array modes.

* assembler/AbstractMacroAssembler.h:
(JSC::AbstractMacroAssembler::JumpList::jumps):
* assembler/MacroAssembler.h:
(MacroAssembler):
(JSC::MacroAssembler::patchableBranch32):
* assembler/MacroAssemblerARMv7.h:
(JSC::MacroAssemblerARMv7::patchableBranch32):
(MacroAssemblerARMv7):
* dfg/DFGArrayMode.cpp:
(JSC::DFG::modeAlreadyChecked):
* dfg/DFGRepatch.cpp:
(JSC::DFG::tryCacheGetByID):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::speculationCheck):
(JSC::DFG::SpeculativeJIT::forwardSpeculationCheck):
(JSC::DFG::SpeculativeJIT::jumpSlowForUnwantedArrayMode):
(DFG):
(JSC::DFG::SpeculativeJIT::checkArray):
(JSC::DFG::SpeculativeJIT::arrayify):
* dfg/DFGSpeculativeJIT.h:
(SpeculativeJIT):
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* jit/JITInlineMethods.h:
(JSC::JIT::emitAllocateJSArray):
(JSC::JIT::chooseArrayMode):
* jit/JITPropertyAccess.cpp:
(JSC::JIT::emit_op_get_by_val):
(JSC::JIT::emitContiguousGetByVal):
(JSC::JIT::emitArrayStorageGetByVal):
(JSC::JIT::emit_op_put_by_val):
(JSC::JIT::emitContiguousPutByVal):
(JSC::JIT::emitArrayStoragePutByVal):
(JSC::JIT::privateCompilePatchGetArrayLength):
* jit/JITPropertyAccess32_64.cpp:
(JSC::JIT::emit_op_get_by_val):
(JSC::JIT::emitContiguousGetByVal):
(JSC::JIT::emitArrayStorageGetByVal):
(JSC::JIT::emit_op_put_by_val):
(JSC::JIT::emitContiguousPutByVal):
(JSC::JIT::emitArrayStoragePutByVal):
(JSC::JIT::privateCompilePatchGetArrayLength):
* llint/LowLevelInterpreter.asm:
* llint/LowLevelInterpreter32_64.asm:
* llint/LowLevelInterpreter64.asm:
* runtime/IndexingType.h:
(JSC):
(JSC::hasIndexedProperties):
(JSC::hasContiguous):
(JSC::hasFastArrayStorage):
(JSC::hasArrayStorage):
(JSC::shouldUseSlowPut):
* runtime/JSGlobalObject.cpp:
(JSC):
* runtime/StructureTransitionTable.h:
(JSC::newIndexingType):

2012-10-14 Filip Pizlo <fpizlo@apple.com>

DFG structure check hoisting should attempt to ignore side effects and make transformations that are sound even in their presence
Expand Down
2 changes: 1 addition & 1 deletion Source/JavaScriptCore/assembler/AbstractMacroAssembler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@ class AbstractMacroAssembler {
m_jumps.clear();
}

const JumpVector& jumps() { return m_jumps; }
const JumpVector& jumps() const { return m_jumps; }

private:
JumpVector m_jumps;
Expand Down
5 changes: 5 additions & 0 deletions Source/JavaScriptCore/assembler/MacroAssembler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,11 @@ class MacroAssembler : public MacroAssemblerBase {
{
return PatchableJump(branchTest32(cond, reg, mask));
}

PatchableJump patchableBranch32(RelationalCondition cond, RegisterID reg, TrustedImm32 imm)
{
return PatchableJump(branch32(cond, reg, imm));
}
#endif

void jump(Label target)
Expand Down
8 changes: 8 additions & 0 deletions Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1701,6 +1701,14 @@ class MacroAssemblerARMv7 : public AbstractMacroAssembler<ARMv7Assembler> {
return PatchableJump(result);
}

PatchableJump patchableBranch32(RelationalCondition cond, RegisterID reg, TrustedImm32 imm)
{
m_makeJumpPatchable = true;
Jump result = branch32(cond, reg, imm);
m_makeJumpPatchable = false;
return PatchableJump(result);
}

PatchableJump patchableBranchPtrWithPatch(RelationalCondition cond, Address left, DataLabelPtr& dataLabel, TrustedImmPtr initialRightValue = TrustedImmPtr(0))
{
m_makeJumpPatchable = true;
Expand Down
12 changes: 6 additions & 6 deletions Source/JavaScriptCore/dfg/DFGArrayMode.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,13 +171,13 @@ bool modeAlreadyChecked(AbstractValue& value, Array::Mode arrayMode)
case Array::PossiblyArrayWithContiguousToTail:
case Array::PossiblyArrayWithContiguousOutOfBounds:
return value.m_currentKnownStructure.hasSingleton()
&& (value.m_currentKnownStructure.singleton()->indexingType() & HasContiguous);
&& hasContiguous(value.m_currentKnownStructure.singleton()->indexingType());

case Array::ArrayWithContiguous:
case Array::ArrayWithContiguousToTail:
case Array::ArrayWithContiguousOutOfBounds:
return value.m_currentKnownStructure.hasSingleton()
&& (value.m_currentKnownStructure.singleton()->indexingType() & HasContiguous)
&& hasContiguous(value.m_currentKnownStructure.singleton()->indexingType())
&& (value.m_currentKnownStructure.singleton()->indexingType() & IsArray);

case Array::ArrayStorage:
Expand All @@ -187,23 +187,23 @@ bool modeAlreadyChecked(AbstractValue& value, Array::Mode arrayMode)
case Array::PossiblyArrayWithArrayStorageToHole:
case Array::PossiblyArrayWithArrayStorageOutOfBounds:
return value.m_currentKnownStructure.hasSingleton()
&& (value.m_currentKnownStructure.singleton()->indexingType() & HasArrayStorage);
&& hasFastArrayStorage(value.m_currentKnownStructure.singleton()->indexingType());

case Array::SlowPutArrayStorage:
case Array::PossiblyArrayWithSlowPutArrayStorage:
return value.m_currentKnownStructure.hasSingleton()
&& (value.m_currentKnownStructure.singleton()->indexingType() & (HasArrayStorage | HasSlowPutArrayStorage));
&& hasArrayStorage(value.m_currentKnownStructure.singleton()->indexingType());

case Array::ArrayWithArrayStorage:
case Array::ArrayWithArrayStorageToHole:
case Array::ArrayWithArrayStorageOutOfBounds:
return value.m_currentKnownStructure.hasSingleton()
&& (value.m_currentKnownStructure.singleton()->indexingType() & HasArrayStorage)
&& hasFastArrayStorage(value.m_currentKnownStructure.singleton()->indexingType())
&& (value.m_currentKnownStructure.singleton()->indexingType() & IsArray);

case Array::ArrayWithSlowPutArrayStorage:
return value.m_currentKnownStructure.hasSingleton()
&& (value.m_currentKnownStructure.singleton()->indexingType() & (HasArrayStorage | HasSlowPutArrayStorage))
&& hasArrayStorage(value.m_currentKnownStructure.singleton()->indexingType())
&& (value.m_currentKnownStructure.singleton()->indexingType() & IsArray);

case ALL_EFFECTFUL_MODES:
Expand Down
2 changes: 1 addition & 1 deletion Source/JavaScriptCore/dfg/DFGRepatch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ static bool tryCacheGetByID(ExecState* exec, JSValue baseValue, const Identifier
stubJit.loadPtr(MacroAssembler::Address(baseGPR, JSCell::structureOffset()), scratchGPR);
stubJit.load8(MacroAssembler::Address(scratchGPR, Structure::indexingTypeOffset()), scratchGPR);
failureCases.append(stubJit.branchTest32(MacroAssembler::Zero, scratchGPR, MacroAssembler::TrustedImm32(IsArray)));
failureCases.append(stubJit.branchTest32(MacroAssembler::Zero, scratchGPR, MacroAssembler::TrustedImm32(HasArrayStorage | HasContiguous | HasSlowPutArrayStorage)));
failureCases.append(stubJit.branchTest32(MacroAssembler::Zero, scratchGPR, MacroAssembler::TrustedImm32(IndexingShapeMask)));

stubJit.loadPtr(MacroAssembler::Address(baseGPR, JSObject::butterflyOffset()), scratchGPR);
stubJit.load32(MacroAssembler::Address(scratchGPR, ArrayStorage::lengthOffset()), scratchGPR);
Expand Down
Loading

0 comments on commit f4afbc5

Please sign in to comment.