Attachment 361536 Details for Bug 194452 – proposed patch.

[patch] proposed patch.

bug-194452.patch (text/plain), 5.17 KB, created by Mark Lam on 2019-02-08 14:44:39 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Mark Lam

Created: 2019-02-08 14:44:39 PST

Size: 5.17 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 241210)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,23 @@
>+2019-02-08  Mark Lam  <mark.lam@apple.com>
>+
>+        Use maxSingleCharacterString in comparisons instead of literal constants.
>+        https://bugs.webkit.org/show_bug.cgi?id=194452
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This way, if we ever change maxSingleCharacterString, it won't break all this code
>+        that relies on it being 0xff implicitly.
>+
>+        * dfg/DFGSpeculativeJIT.cpp:
>+        (JSC::DFG::SpeculativeJIT::compileStringSlice):
>+        (JSC::DFG::SpeculativeJIT::compileGetByValOnString):
>+        * ftl/FTLLowerDFGToB3.cpp:
>+        (JSC::FTL::DFG::LowerDFGToB3::compileStringCharAt):
>+        (JSC::FTL::DFG::LowerDFGToB3::compileStringSlice):
>+        * jit/ThunkGenerators.cpp:
>+        (JSC::stringGetByValGenerator):
>+        (JSC::charToString):
>+
> 2019-02-08  Mark Lam  <mark.lam@apple.com>
> 
>         Fix DFG's doesGC() for CheckTierUp*, GetByVal, PutByVal*, and StringCharAt nodes.
>Index: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp	(revision 241209)
>+++ Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp	(working copy)
>@@ -1577,7 +1577,7 @@ void SpeculativeJIT::compileStringSlice(
>     is16Bit.link(&m_jit);
>     m_jit.load16(MacroAssembler::BaseIndex(tempGPR, startIndexGPR, MacroAssembler::TimesTwo, 0), tempGPR);
> 
>-    auto bigCharacter = m_jit.branch32(MacroAssembler::AboveOrEqual, tempGPR, TrustedImm32(0x100));
>+    auto bigCharacter = m_jit.branch32(MacroAssembler::Above, tempGPR, TrustedImm32(maxSingleCharacterString));
> 
>     // 8 bit string values don't need the isASCII check.
>     cont8Bit.link(&m_jit);
>@@ -2194,7 +2194,7 @@ void SpeculativeJIT::compileGetByValOnSt
>     m_jit.load16(MacroAssembler::BaseIndex(storageReg, propertyReg, MacroAssembler::TimesTwo, 0), scratchReg);
> 
>     JITCompiler::Jump bigCharacter =
>-        m_jit.branch32(MacroAssembler::AboveOrEqual, scratchReg, TrustedImm32(0x100));
>+        m_jit.branch32(MacroAssembler::Above, scratchReg, TrustedImm32(maxSingleCharacterString));
> 
>     // 8 bit string values don't need the isASCII check.
>     cont8Bit.link(&m_jit);
>Index: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>===================================================================
>--- Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp	(revision 241210)
>+++ Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp	(working copy)
>@@ -6612,7 +6612,7 @@ private:
>                 provenValue(m_graph.child(m_node, 1))));
>         ValueFromBlock char16Bit = m_out.anchor(char16BitValue);
>         m_out.branch(
>-            m_out.aboveOrEqual(char16BitValue, m_out.constInt32(0x100)),
>+            m_out.above(char16BitValue, m_out.constInt32(maxSingleCharacterString)),
>             rarely(bigCharacter), usually(bitsContinuation));
>             
>         m_out.appendTo(bigCharacter, bitsContinuation);
>@@ -11973,7 +11973,7 @@ private:
>         LValue char16BitValue = m_out.load16ZeroExt32(m_out.baseIndex(m_heaps.characters16, storage, m_out.zeroExtPtr(from)));
>         ValueFromBlock char16Bit = m_out.anchor(char16BitValue);
>         m_out.branch(
>-            m_out.aboveOrEqual(char16BitValue, m_out.constInt32(0x100)),
>+            m_out.above(char16BitValue, m_out.constInt32(maxSingleCharacterString)),
>             rarely(bigCharacter), usually(bitsContinuation));
> 
>         m_out.appendTo(bigCharacter, bitsContinuation);
>Index: Source/JavaScriptCore/jit/ThunkGenerators.cpp
>===================================================================
>--- Source/JavaScriptCore/jit/ThunkGenerators.cpp	(revision 241209)
>+++ Source/JavaScriptCore/jit/ThunkGenerators.cpp	(working copy)
>@@ -656,7 +656,7 @@ MacroAssemblerCodeRef<JITThunkPtrTag> st
>     jit.load16(JSInterfaceJIT::BaseIndex(stringGPR, indexGPR, JSInterfaceJIT::TimesTwo, 0), stringGPR);
>     cont8Bit.link(&jit);
> 
>-    failures.append(jit.branch32(JSInterfaceJIT::AboveOrEqual, stringGPR, JSInterfaceJIT::TrustedImm32(0x100)));
>+    failures.append(jit.branch32(JSInterfaceJIT::Above, stringGPR, JSInterfaceJIT::TrustedImm32(maxSingleCharacterString)));
>     jit.move(JSInterfaceJIT::TrustedImmPtr(vm->smallStrings.singleCharacterStrings()), indexGPR);
>     jit.loadPtr(JSInterfaceJIT::BaseIndex(indexGPR, stringGPR, JSInterfaceJIT::ScalePtr, 0), stringGPR);
>     jit.ret();
>@@ -701,7 +701,7 @@ static void stringCharLoad(SpecializedTh
> 
> static void charToString(SpecializedThunkJIT& jit, VM* vm, MacroAssembler::RegisterID src, MacroAssembler::RegisterID dst, MacroAssembler::RegisterID scratch)
> {
>-    jit.appendFailure(jit.branch32(MacroAssembler::AboveOrEqual, src, MacroAssembler::TrustedImm32(0x100)));
>+    jit.appendFailure(jit.branch32(MacroAssembler::Above, src, MacroAssembler::TrustedImm32(maxSingleCharacterString)));
>     jit.move(MacroAssembler::TrustedImmPtr(vm->smallStrings.singleCharacterStrings()), scratch);
>     jit.loadPtr(MacroAssembler::BaseIndex(scratch, src, MacroAssembler::ScalePtr, 0), dst);
>     jit.appendFailure(jit.branchTestPtr(MacroAssembler::Zero, dst));

Flags:

ysuzuki: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 194452: 361536