Attachment 339000 Details for Bug 185081 – patch

[patch] patch

0001-Make-the-first-64-bits-of-JSString-look-like-a-doubl.patch (text/plain), 9.78 KB, created by JF Bastien on 2018-04-27 10:32:24 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: JF Bastien

Created: 2018-04-27 10:32:24 PDT

Size: 9.78 KB

patch

obsolete

>From c6eba50d3c0097cb663fe50deedde6caf7439fd2 Mon Sep 17 00:00:00 2001
>From: JF Bastien <jfbastien@apple.com>
>Date: Fri, 27 Apr 2018 10:32:03 -0700
>Subject: [PATCH] Make the first 64 bits of JSString look like a double JSValue
>
>---
> Source/JavaScriptCore/ChangeLog                    | 24 ++++++++++++++++++++++
> .../assembler/MacroAssemblerX86Common.h            | 10 +++++++++
> Source/JavaScriptCore/assembler/X86Assembler.h     | 14 ++++++++++++-
> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp    |  6 +++---
> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp      |  6 +++---
> Source/JavaScriptCore/ftl/FTLOutput.h              |  2 ++
> Source/JavaScriptCore/runtime/JSString.h           | 12 +++++------
> 7 files changed, 61 insertions(+), 13 deletions(-)
>
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 41d8c24..e835acd 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,27 @@
>+2018-04-27  JF Bastien  <jfbastien@apple.com>
>+
>+        Make the first 64 bits of JSString look like a double JSValue
>+        https://bugs.webkit.org/show_bug.cgi?id=185081
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        We can be clever about how we lay out JSString so that, were it
>+        reinterpreted as a JSValue, it would look like a double.
>+
>+        * assembler/MacroAssemblerX86Common.h:
>+        (JSC::MacroAssemblerX86Common::and16):
>+        * assembler/X86Assembler.h:
>+        (JSC::X86Assembler::andw_mr):
>+        * dfg/DFGSpeculativeJIT.cpp:
>+        (JSC::DFG::SpeculativeJIT::compileMakeRope):
>+        * ftl/FTLLowerDFGToB3.cpp:
>+        (JSC::FTL::DFG::LowerDFGToB3::compileMakeRope):
>+        * ftl/FTLOutput.h:
>+        (JSC::FTL::Output::store32As8):
>+        (JSC::FTL::Output::store32As16):
>+        * runtime/JSString.h:
>+        (JSC::JSString::JSString):
>+
> 2018-04-26  Jer Noble  <jer.noble@apple.com>
> 
>         WK_COCOA_TOUCH all the things.
>diff --git a/Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h b/Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h
>index e8c8599..a8986a4 100644
>--- a/Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h
>+++ b/Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h
>@@ -290,6 +290,16 @@ public:
>         m_assembler.andl_mr(src.offset, src.base, src.index, src.scale, dest);
>     }
> 
>+    void and16(Address src, RegisterID dest)
>+    {
>+        m_assembler.andw_mr(src.offset, src.base, dest);
>+    }
>+
>+    void and16(BaseIndex src, RegisterID dest)
>+    {
>+        m_assembler.andw_mr(src.offset, src.base, src.index, src.scale, dest);
>+    }
>+
>     void and32(TrustedImm32 imm, Address address)
>     {
>         m_assembler.andl_im(imm.m_value, address.offset, address.base);
>diff --git a/Source/JavaScriptCore/assembler/X86Assembler.h b/Source/JavaScriptCore/assembler/X86Assembler.h
>index 4e60266..51fb333 100644
>--- a/Source/JavaScriptCore/assembler/X86Assembler.h
>+++ b/Source/JavaScriptCore/assembler/X86Assembler.h
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2008-2017 Apple Inc. All rights reserved.
>+ * Copyright (C) 2008-2018 Apple Inc. All rights reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -685,6 +685,18 @@ public:
>         m_formatter.oneByteOp(OP_AND_GvEv, dst, base, index, scale, offset);
>     }
> 
>+    void andw_mr(int offset, RegisterID base, RegisterID dst)
>+    {
>+        m_formatter.prefix(PRE_OPERAND_SIZE);
>+        andl_mr(offset, base, dst);
>+    }
>+
>+    void andw_mr(int offset, RegisterID base, RegisterID index, int scale, RegisterID dst)
>+    {
>+        m_formatter.prefix(PRE_OPERAND_SIZE);
>+        andl_mr(offset, base, index, scale, dst);
>+    }
>+
>     void andl_rm(RegisterID src, int offset, RegisterID base)
>     {
>         m_formatter.oneByteOp(OP_AND_EvGv, src, base, offset);
>diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
>index df69227..b095268 100644
>--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
>@@ -4268,7 +4268,7 @@ void SpeculativeJIT::compileMakeRope(Node* node)
>         m_jit.storePtr(opGPRs[i], JITCompiler::Address(resultGPR, JSRopeString::offsetOfFibers() + sizeof(WriteBarrier<JSString>) * i));
>     for (unsigned i = numOpGPRs; i < JSRopeString::s_maxInternalRopeLength; ++i)
>         m_jit.storePtr(TrustedImmPtr(nullptr), JITCompiler::Address(resultGPR, JSRopeString::offsetOfFibers() + sizeof(WriteBarrier<JSString>) * i));
>-    m_jit.load32(JITCompiler::Address(opGPRs[0], JSString::offsetOfFlags()), scratchGPR);
>+    m_jit.load16(JITCompiler::Address(opGPRs[0], JSString::offsetOfFlags()), scratchGPR);
>     m_jit.load32(JITCompiler::Address(opGPRs[0], JSString::offsetOfLength()), allocatorGPR);
>     if (!ASSERT_DISABLED) {
>         JITCompiler::Jump ok = m_jit.branch32(
>@@ -4277,7 +4277,7 @@ void SpeculativeJIT::compileMakeRope(Node* node)
>         ok.link(&m_jit);
>     }
>     for (unsigned i = 1; i < numOpGPRs; ++i) {
>-        m_jit.and32(JITCompiler::Address(opGPRs[i], JSString::offsetOfFlags()), scratchGPR);
>+        m_jit.and16(JITCompiler::Address(opGPRs[i], JSString::offsetOfFlags()), scratchGPR);
>         speculationCheck(
>             Uncountable, JSValueSource(), nullptr,
>             m_jit.branchAdd32(
>@@ -4285,7 +4285,7 @@ void SpeculativeJIT::compileMakeRope(Node* node)
>                 JITCompiler::Address(opGPRs[i], JSString::offsetOfLength()), allocatorGPR));
>     }
>     m_jit.and32(JITCompiler::TrustedImm32(JSString::Is8Bit), scratchGPR);
>-    m_jit.store32(scratchGPR, JITCompiler::Address(resultGPR, JSString::offsetOfFlags()));
>+    m_jit.store16(scratchGPR, JITCompiler::Address(resultGPR, JSString::offsetOfFlags()));
>     if (!ASSERT_DISABLED) {
>         JITCompiler::Jump ok = m_jit.branch32(
>             JITCompiler::GreaterThanOrEqual, allocatorGPR, TrustedImm32(0));
>diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>index cc3ee98..3372051 100644
>--- a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>+++ b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>@@ -6165,16 +6165,16 @@ private:
>             m_out.storePtr(kids[i], result, m_heaps.JSRopeString_fibers[i]);
>         for (unsigned i = numKids; i < JSRopeString::s_maxInternalRopeLength; ++i)
>             m_out.storePtr(m_out.intPtrZero, result, m_heaps.JSRopeString_fibers[i]);
>-        LValue flags = m_out.load32(kids[0], m_heaps.JSString_flags);
>+        LValue flags = m_out.load16ZeroExt32(kids[0], m_heaps.JSString_flags);
>         LValue length = m_out.load32(kids[0], m_heaps.JSString_length);
>         for (unsigned i = 1; i < numKids; ++i) {
>-            flags = m_out.bitAnd(flags, m_out.load32(kids[i], m_heaps.JSString_flags));
>+            flags = m_out.bitAnd(flags, m_out.load16ZeroExt32(kids[i], m_heaps.JSString_flags));
>             CheckValue* lengthCheck = m_out.speculateAdd(
>                 length, m_out.load32(kids[i], m_heaps.JSString_length));
>             blessSpeculation(lengthCheck, Uncountable, noValue(), nullptr, m_origin);
>             length = lengthCheck;
>         }
>-        m_out.store32(
>+        m_out.store32As16(
>             m_out.bitAnd(m_out.constInt32(JSString::Is8Bit), flags),
>             result, m_heaps.JSString_flags);
>         m_out.store32(length, result, m_heaps.JSString_length);
>diff --git a/Source/JavaScriptCore/ftl/FTLOutput.h b/Source/JavaScriptCore/ftl/FTLOutput.h
>index efabdb1..588edd8 100644
>--- a/Source/JavaScriptCore/ftl/FTLOutput.h
>+++ b/Source/JavaScriptCore/ftl/FTLOutput.h
>@@ -322,6 +322,8 @@ public:
>     LValue load64(LValue base, const AbstractHeap& field) { return load64(address(base, field)); }
>     LValue loadPtr(LValue base, const AbstractHeap& field) { return loadPtr(address(base, field)); }
>     LValue loadDouble(LValue base, const AbstractHeap& field) { return loadDouble(address(base, field)); }
>+    void store32As8(LValue value, LValue base, const AbstractHeap& field) { store32As8(value, address(base, field)); }
>+    void store32As16(LValue value, LValue base, const AbstractHeap& field) { store32As16(value, address(base, field)); }
>     void store32(LValue value, LValue base, const AbstractHeap& field) { store32(value, address(base, field)); }
>     void store64(LValue value, LValue base, const AbstractHeap& field) { store64(value, address(base, field)); }
>     void storePtr(LValue value, LValue base, const AbstractHeap& field) { storePtr(value, address(base, field)); }
>diff --git a/Source/JavaScriptCore/runtime/JSString.h b/Source/JavaScriptCore/runtime/JSString.h
>index 144d9f6..6da7bfd 100644
>--- a/Source/JavaScriptCore/runtime/JSString.h
>+++ b/Source/JavaScriptCore/runtime/JSString.h
>@@ -1,7 +1,7 @@
> /*
>  *  Copyright (C) 1999-2001 Harri Porten (porten@kde.org)
>  *  Copyright (C) 2001 Peter Kelly (pmk@post.com)
>- *  Copyright (C) 2003-2017 Apple Inc. All rights reserved.
>+ *  Copyright (C) 2003-2018 Apple Inc. All rights reserved.
>  *
>  *  This library is free software; you can redistribute it and/or
>  *  modify it under the terms of the GNU Library General Public
>@@ -101,14 +101,12 @@ public:
> private:
>     JSString(VM& vm, Ref<StringImpl>&& value)
>         : JSCell(vm, vm.stringStructure.get())
>-        , m_flags(0)
>         , m_value(WTFMove(value))
>     {
>     }
> 
>     JSString(VM& vm)
>         : JSCell(vm, vm.stringStructure.get())
>-        , m_flags(0)
>     {
>     }
> 
>@@ -218,10 +216,12 @@ protected:
>     }
> 
> private:
>-    mutable unsigned m_flags;
>-
>     // A string is represented either by a String or a rope of fibers.
>-    unsigned m_length;
>+    unsigned m_length { 0 };
>+    mutable uint16_t m_flags { 0 };
>+    // The poison is strategically placed and holds a value such that the first
>+    // 64 bits of JSString look like a double JSValue.
>+    uint16_t m_poison { 1 };
>     mutable String m_value;
> 
>     friend class LLIntOffsetsExtractor;
>-- 
>2.9.3
>

Flags:

fpizlo: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 185081: 339000 | 339024