Diff
Modified: trunk/JSTests/ChangeLog (234330 => 234331)
--- trunk/JSTests/ChangeLog 2018-07-27 22:20:26 UTC (rev 234330)
+++ trunk/JSTests/ChangeLog 2018-07-27 22:27:10 UTC (rev 234331)
@@ -1,3 +1,16 @@
+2018-07-25 Yusuke Suzuki <[email protected]>
+
+ [JSC] Record CoW status in ArrayProfile correctly
+ https://bugs.webkit.org/show_bug.cgi?id=187949
+
+ Reviewed by Saam Barati.
+
+ * stress/array-profile-should-record-copy-on-write.js: Added.
+ (shouldBe):
+ (test1):
+ (test2):
+ (test3):
+
2018-07-26 Mark Lam <[email protected]>
arrayProtoPrivateFuncConcatMemcpy() should handle copying from an Undecided type array.
Added: trunk/JSTests/stress/array-profile-should-record-copy-on-write.js (0 => 234331)
--- trunk/JSTests/stress/array-profile-should-record-copy-on-write.js (rev 0)
+++ trunk/JSTests/stress/array-profile-should-record-copy-on-write.js 2018-07-27 22:27:10 UTC (rev 234331)
@@ -0,0 +1,39 @@
+function shouldBe(actual, expected)
+{
+ if (actual !== expected)
+ throw new Error('bad value: ' + actual);
+}
+noInline(shouldBe);
+
+function test1(array)
+{
+ for (var i = 0; i < 5; ++i) {
+ array[0] = array[0] + 1;
+ }
+ return array;
+}
+noInline(test1);
+
+function test2(array)
+{
+ for (var i = 0; i < 5; ++i) {
+ array[0] = array[0] + 1;
+ }
+ return array;
+}
+noInline(test2);
+
+function test3(array)
+{
+ for (var i = 0; i < 5; ++i) {
+ array[0] = array[0] + 1;
+ }
+ return array;
+}
+noInline(test3);
+
+for (var i = 0; i < 1e5; ++i) {
+ shouldBe(String(test1([0, 1, 2, 3, 4])), `5,1,2,3,4`);
+ shouldBe(String(test2([0.1, 1.1, 2.1, 3.1, 4.1])), `5.1,1.1,2.1,3.1,4.1`);
+ shouldBe(String(test3(['C', 'o', 'c', 'o', 'a'])), `C11111,o,c,o,a`);
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (234330 => 234331)
--- trunk/Source/_javascript_Core/ChangeLog 2018-07-27 22:20:26 UTC (rev 234330)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-07-27 22:27:10 UTC (rev 234331)
@@ -1,3 +1,25 @@
+2018-07-25 Yusuke Suzuki <[email protected]>
+
+ [JSC] Record CoW status in ArrayProfile correctly
+ https://bugs.webkit.org/show_bug.cgi?id=187949
+
+ Reviewed by Saam Barati.
+
+ In this patch, we simplify asArrayModes: just shifting the value with IndexingMode.
+ This is important since our OSR exit compiler records m_observedArrayModes by calculating
+ ArrayModes with shifting. Since ArrayModes for CoW arrays are incorrectly calculated,
+ our OSR exit compiler records incorrect results in ArrayProfile. And it leads to
+ Array::Generic DFG nodes.
+
+ * bytecode/ArrayProfile.h:
+ (JSC::asArrayModes):
+ (JSC::ArrayProfile::ArrayProfile):
+ * dfg/DFGOSRExit.cpp:
+ (JSC::DFG::OSRExit::compileExit):
+ * ftl/FTLOSRExitCompiler.cpp:
+ (JSC::FTL::compileStub):
+ * runtime/IndexingType.h:
+
2018-07-26 Andy VanWagoner <[email protected]>
[INTL] Remove INTL sub-feature compile flags
Modified: trunk/Source/_javascript_Core/bytecode/ArrayProfile.h (234330 => 234331)
--- trunk/Source/_javascript_Core/bytecode/ArrayProfile.h 2018-07-27 22:20:26 UTC (rev 234330)
+++ trunk/Source/_javascript_Core/bytecode/ArrayProfile.h 2018-07-27 22:27:10 UTC (rev 234331)
@@ -39,36 +39,27 @@
// There are 9 typed array types taking the bits 16 to 25.
typedef unsigned ArrayModes;
-const ArrayModes CopyOnWriteArrayWithInt32ArrayMode = 1 << 16;
-const ArrayModes CopyOnWriteArrayWithDoubleArrayMode = 1 << 17;
-const ArrayModes CopyOnWriteArrayWithContiguousArrayMode = 1 << 18;
+// The possible IndexingTypes are limited within (0 - 16, 21, 23, 25).
+// This is because CoW types only appear for JSArrays.
+static_assert(CopyOnWriteArrayWithInt32 == 21, "");
+static_assert(CopyOnWriteArrayWithDouble == 23, "");
+static_assert(CopyOnWriteArrayWithContiguous == 25, "");
+const ArrayModes CopyOnWriteArrayWithInt32ArrayMode = 1 << CopyOnWriteArrayWithInt32;
+const ArrayModes CopyOnWriteArrayWithDoubleArrayMode = 1 << CopyOnWriteArrayWithDouble;
+const ArrayModes CopyOnWriteArrayWithContiguousArrayMode = 1 << CopyOnWriteArrayWithContiguous;
-const ArrayModes Int8ArrayMode = 1 << 19;
-const ArrayModes Int16ArrayMode = 1 << 20;
-const ArrayModes Int32ArrayMode = 1 << 21;
-const ArrayModes Uint8ArrayMode = 1 << 22;
-const ArrayModes Uint8ClampedArrayMode = 1 << 23;
-const ArrayModes Uint16ArrayMode = 1 << 24;
-const ArrayModes Uint32ArrayMode = 1 << 25;
-const ArrayModes Float32ArrayMode = 1 << 26;
-const ArrayModes Float64ArrayMode = 1 << 27;
+const ArrayModes Int8ArrayMode = 1 << 16;
+const ArrayModes Int16ArrayMode = 1 << 17;
+const ArrayModes Int32ArrayMode = 1 << 18;
+const ArrayModes Uint8ArrayMode = 1 << 19;
+const ArrayModes Uint8ClampedArrayMode = 1 << 20; // 21 - 25 are used for CoW arrays.
+const ArrayModes Uint16ArrayMode = 1 << 26;
+const ArrayModes Uint32ArrayMode = 1 << 27;
+const ArrayModes Float32ArrayMode = 1 << 28;
+const ArrayModes Float64ArrayMode = 1 << 29;
inline constexpr ArrayModes asArrayModes(IndexingType indexingMode)
{
- if (isCopyOnWrite(indexingMode)) {
- switch (indexingMode) {
- case CopyOnWriteArrayWithInt32:
- return CopyOnWriteArrayWithInt32ArrayMode;
- case CopyOnWriteArrayWithDouble:
- return CopyOnWriteArrayWithDoubleArrayMode;
- case CopyOnWriteArrayWithContiguous:
- return CopyOnWriteArrayWithContiguousArrayMode;
- default:
- UNREACHABLE_FOR_PLATFORM();
- return 0;
- }
- }
-
return static_cast<unsigned>(1) << static_cast<unsigned>(indexingMode);
}
@@ -221,26 +212,15 @@
class ArrayProfile {
public:
ArrayProfile()
- : m_bytecodeOffset(std::numeric_limits<unsigned>::max())
- , m_lastSeenStructureID(0)
- , m_mayStoreToHole(false)
- , m_outOfBounds(false)
- , m_mayInterceptIndexedAccesses(false)
- , m_usesOriginalArrayStructures(true)
- , m_didPerformFirstRunPruning(false)
- , m_observedArrayModes(0)
+ : ArrayProfile(std::numeric_limits<unsigned>::max())
{
}
- ArrayProfile(unsigned bytecodeOffset)
+ explicit ArrayProfile(unsigned bytecodeOffset)
: m_bytecodeOffset(bytecodeOffset)
- , m_lastSeenStructureID(0)
- , m_mayStoreToHole(false)
- , m_outOfBounds(false)
, m_mayInterceptIndexedAccesses(false)
, m_usesOriginalArrayStructures(true)
, m_didPerformFirstRunPruning(false)
- , m_observedArrayModes(0)
{
}
@@ -281,13 +261,13 @@
static Structure* polymorphicStructure() { return static_cast<Structure*>(reinterpret_cast<void*>(1)); }
unsigned m_bytecodeOffset;
- StructureID m_lastSeenStructureID;
- bool m_mayStoreToHole; // This flag may become overloaded to indicate other special cases that were encountered during array access, as it depends on indexing type. Since we currently have basically just one indexing type (two variants of ArrayStorage), this flag for now just means exactly what its name implies.
- bool m_outOfBounds;
+ StructureID m_lastSeenStructureID { 0 };
+ bool m_mayStoreToHole { false }; // This flag may become overloaded to indicate other special cases that were encountered during array access, as it depends on indexing type. Since we currently have basically just one indexing type (two variants of ArrayStorage), this flag for now just means exactly what its name implies.
+ bool m_outOfBounds { false };
bool m_mayInterceptIndexedAccesses : 1;
bool m_usesOriginalArrayStructures : 1;
bool m_didPerformFirstRunPruning : 1;
- ArrayModes m_observedArrayModes;
+ ArrayModes m_observedArrayModes { 0 };
};
typedef SegmentedVector<ArrayProfile, 4> ArrayProfileVector;
Modified: trunk/Source/_javascript_Core/dfg/DFGOSRExit.cpp (234330 => 234331)
--- trunk/Source/_javascript_Core/dfg/DFGOSRExit.cpp 2018-07-27 22:20:26 UTC (rev 234330)
+++ trunk/Source/_javascript_Core/dfg/DFGOSRExit.cpp 2018-07-27 22:27:10 UTC (rev 234331)
@@ -1199,6 +1199,7 @@
#else
jit.load8(AssemblyHelpers::Address(scratch1, Structure::indexingModeIncludingHistoryOffset()), scratch1);
#endif
+ jit.and32(AssemblyHelpers::TrustedImm32(IndexingModeMask), scratch1);
jit.move(AssemblyHelpers::TrustedImm32(1), scratch2);
jit.lshift32(scratch1, scratch2);
jit.or32(scratch2, AssemblyHelpers::AbsoluteAddress(arrayProfile->addressOfArrayModes()));
Modified: trunk/Source/_javascript_Core/ftl/FTLOSRExitCompiler.cpp (234330 => 234331)
--- trunk/Source/_javascript_Core/ftl/FTLOSRExitCompiler.cpp 2018-07-27 22:20:26 UTC (rev 234330)
+++ trunk/Source/_javascript_Core/ftl/FTLOSRExitCompiler.cpp 2018-07-27 22:27:10 UTC (rev 234331)
@@ -278,6 +278,7 @@
jit.load32(MacroAssembler::Address(GPRInfo::regT0, JSCell::structureIDOffset()), GPRInfo::regT1);
jit.store32(GPRInfo::regT1, arrayProfile->addressOfLastSeenStructureID());
jit.load8(MacroAssembler::Address(GPRInfo::regT0, JSCell::indexingTypeAndMiscOffset()), GPRInfo::regT1);
+ jit.and32(MacroAssembler::TrustedImm32(IndexingModeMask), GPRInfo::regT1);
jit.move(MacroAssembler::TrustedImm32(1), GPRInfo::regT2);
jit.lshift32(GPRInfo::regT1, GPRInfo::regT2);
jit.or32(GPRInfo::regT2, MacroAssembler::AbsoluteAddress(arrayProfile->addressOfArrayModes()));
Modified: trunk/Source/_javascript_Core/runtime/IndexingType.h (234330 => 234331)
--- trunk/Source/_javascript_Core/runtime/IndexingType.h 2018-07-27 22:20:26 UTC (rev 234330)
+++ trunk/Source/_javascript_Core/runtime/IndexingType.h 2018-07-27 22:27:10 UTC (rev 234331)
@@ -79,6 +79,7 @@
// Whether or not the butterfly is copy on write. If it is copy on write then the butterfly is actually a JSImmutableButterfly. This should only ever be set if there are no named properties.
static const IndexingType CopyOnWrite = 0x10;
static const IndexingType IndexingShapeAndWritabilityMask = CopyOnWrite | IndexingShapeMask;
+static const IndexingType IndexingModeMask = CopyOnWrite | IndexingTypeMask;
static const IndexingType NumberOfCopyOnWriteIndexingModes = 3; // We only have copy on write for int32, double, and contiguous shapes.
static const IndexingType NumberOfArrayIndexingModes = NumberOfIndexingShapes + NumberOfCopyOnWriteIndexingModes;
