- Revision
- 242519
- Author
- sbar...@apple.com
- Date
- 2019-03-05 16:20:07 -0800 (Tue, 05 Mar 2019)
Log Message
op_switch_char broken for rope strings after JSRopeString layout rewrite
https://bugs.webkit.org/show_bug.cgi?id=195339
<rdar://problem/48592545>
Reviewed by Yusuke Suzuki.
JSTests:
* stress/switch-on-char-llint-rope.js: Added.
Source/_javascript_Core:
When we did the JSString rewrite, we accidentally broke LLInt's switch_char
for rope strings. That change made it so that we always go to the slow path
for ropes. That's wrong. The slow path should only be taken when the rope
is of length 1. For lengths other than 1, we need to fall through to the
default case. This patch fixes this.
* llint/LowLevelInterpreter32_64.asm:
* llint/LowLevelInterpreter64.asm:
* runtime/JSString.h:
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (242518 => 242519)
--- trunk/JSTests/ChangeLog 2019-03-05 23:40:08 UTC (rev 242518)
+++ trunk/JSTests/ChangeLog 2019-03-06 00:20:07 UTC (rev 242519)
@@ -1,3 +1,13 @@
+2019-03-05 Saam barati <sbar...@apple.com>
+
+ op_switch_char broken for rope strings after JSRopeString layout rewrite
+ https://bugs.webkit.org/show_bug.cgi?id=195339
+ <rdar://problem/48592545>
+
+ Reviewed by Yusuke Suzuki.
+
+ * stress/switch-on-char-llint-rope.js: Added.
+
2019-03-04 Yusuke Suzuki <ysuz...@apple.com>
[JSC] Store bits for JSRopeString in 3 stores
Added: trunk/JSTests/stress/switch-on-char-llint-rope.js (0 => 242519)
--- trunk/JSTests/stress/switch-on-char-llint-rope.js (rev 0)
+++ trunk/JSTests/stress/switch-on-char-llint-rope.js 2019-03-06 00:20:07 UTC (rev 242519)
@@ -0,0 +1,22 @@
+function constStr() {
+ return ' ZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ';
+}
+
+function foo(z) {
+ switch (z) {
+ case 'a':
+ case 'a':
+ case 'a':
+ return 1;
+ default:
+ return 2;
+ }
+}
+noInline(foo);
+
+let str = 'a' + constStr();
+for (let i = 0; i < 10000; ++i) {
+ let result = foo(str);
+ if (result !== 2)
+ throw new Error("Bad result");
+}
Modified: trunk/Source/_javascript_Core/ChangeLog (242518 => 242519)
--- trunk/Source/_javascript_Core/ChangeLog 2019-03-05 23:40:08 UTC (rev 242518)
+++ trunk/Source/_javascript_Core/ChangeLog 2019-03-06 00:20:07 UTC (rev 242519)
@@ -1,3 +1,21 @@
+2019-03-05 Saam barati <sbar...@apple.com>
+
+ op_switch_char broken for rope strings after JSRopeString layout rewrite
+ https://bugs.webkit.org/show_bug.cgi?id=195339
+ <rdar://problem/48592545>
+
+ Reviewed by Yusuke Suzuki.
+
+ When we did the JSString rewrite, we accidentally broke LLInt's switch_char
+ for rope strings. That change made it so that we always go to the slow path
+ for ropes. That's wrong. The slow path should only be taken when the rope
+ is of length 1. For lengths other than 1, we need to fall through to the
+ default case. This patch fixes this.
+
+ * llint/LowLevelInterpreter32_64.asm:
+ * llint/LowLevelInterpreter64.asm:
+ * runtime/JSString.h:
+
2019-03-05 Yusuke Suzuki <ysuz...@apple.com>
[JSC] Should check exception for JSString::toExistingAtomicString
Modified: trunk/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm (242518 => 242519)
--- trunk/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm 2019-03-05 23:40:08 UTC (rev 242518)
+++ trunk/Source/_javascript_Core/llint/LowLevelInterpreter32_64.asm 2019-03-06 00:20:07 UTC (rev 242519)
@@ -1788,15 +1788,15 @@
addp t3, t2
bineq t1, CellTag, .opSwitchCharFallThrough
bbneq JSCell::m_type[t0], StringType, .opSwitchCharFallThrough
- loadp JSString::m_fiber[t0], t0
- btpnz t0, isRopeInPointer, .opSwitchOnRope
- bineq StringImpl::m_length[t0], 1, .opSwitchCharFallThrough
- loadp StringImpl::m_data8[t0], t1
- btinz StringImpl::m_hashAndFlags[t0], HashFlags8BitBuffer, .opSwitchChar8Bit
- loadh [t1], t0
+ loadp JSString::m_fiber[t0], t1
+ btpnz t1, isRopeInPointer, .opSwitchOnRope
+ bineq StringImpl::m_length[t1], 1, .opSwitchCharFallThrough
+ loadp StringImpl::m_data8[t1], t0
+ btinz StringImpl::m_hashAndFlags[t1], HashFlags8BitBuffer, .opSwitchChar8Bit
+ loadh [t0], t0
jmp .opSwitchCharReady
.opSwitchChar8Bit:
- loadb [t1], t0
+ loadb [t0], t0
.opSwitchCharReady:
subi SimpleJumpTable::min[t2], t0
biaeq t0, SimpleJumpTable::branchOffsets + VectorSizeOffset[t2], .opSwitchCharFallThrough
@@ -1809,6 +1809,9 @@
jump(m_defaultOffset)
.opSwitchOnRope:
+ bineq JSRopeString::m_compactFibers + JSRopeString::CompactFibers::m_length[t0], 1, .opSwitchCharFallThrough
+
+.opSwitchOnRopeChar:
callSlowPath(_llint_slow_path_switch_char)
nextInstruction()
end)
Modified: trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm (242518 => 242519)
--- trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm 2019-03-05 23:40:08 UTC (rev 242518)
+++ trunk/Source/_javascript_Core/llint/LowLevelInterpreter64.asm 2019-03-06 00:20:07 UTC (rev 242519)
@@ -1898,6 +1898,9 @@
jump(m_defaultOffset)
.opSwitchOnRope:
+ bineq JSRopeString::m_compactFibers + JSRopeString::CompactFibers::m_length[t1], 1, .opSwitchCharFallThrough
+
+.opSwitchOnRopeChar:
callSlowPath(_llint_slow_path_switch_char)
nextInstruction()
end)
Modified: trunk/Source/_javascript_Core/runtime/JSString.h (242518 => 242519)
--- trunk/Source/_javascript_Core/runtime/JSString.h 2019-03-05 23:40:08 UTC (rev 242518)
+++ trunk/Source/_javascript_Core/runtime/JSString.h 2019-03-06 00:20:07 UTC (rev 242519)
@@ -311,6 +311,8 @@
static ptrdiff_t offsetOfFiber2() { return OBJECT_OFFSETOF(CompactFibers, m_fiber1Upper); }
private:
+ friend class LLIntOffsetsExtractor;
+
uint32_t m_length { 0 };
uint32_t m_fiber1Lower { 0 };
uint16_t m_fiber1Upper { 0 };
@@ -350,6 +352,8 @@
static ptrdiff_t offsetOfFiber2() { return OBJECT_OFFSETOF(CompactFibers, m_fiber2); }
private:
+ friend class LLIntOffsetsExtractor;
+
uint32_t m_length { 0 };
JSString* m_fiber1 { nullptr };
JSString* m_fiber2 { nullptr };
@@ -438,6 +442,8 @@
}
private:
+ friend class LLIntOffsetsExtractor;
+
void convertToNonRope(String&&) const;
void initializeIs8Bit(bool flag) const
