Attachment 357253 Details for Bug 192680 – Patch v1

[patch] Patch v1

bug-192680-20181213142152.patch (text/plain), 11.97 KB, created by David Kilzer (:ddkilzer) on 2018-12-13 14:21:53 PST

(hide)

Description:

Filename:

MIME Type:

Creator: David Kilzer (:ddkilzer)

Created: 2018-12-13 14:21:53 PST

Size: 11.97 KB

patch

obsolete

>Subversion Revision: 239112
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 31522434d77a3ffa3b1ef47ca56d57c52db43d66..dfa8c2fb4732c6bec52f609180e93f274ae89805 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,40 @@
>+2018-12-13  David Kilzer  <ddkilzer@apple.com>
>+
>+        clang-tidy: Fix unnecessary object copies in JavaScriptCore
>+        <https://webkit.org/b/192680>
>+        <rdar://problem/46708767>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * assembler/testmasm.cpp:
>+        (JSC::invoke):
>+        - Make MacroAssemblerCodeRef<JSEntryPtrTag> argument a const
>+          reference.
>+
>+        * b3/testb3.cpp:
>+        (JSC::B3::checkDisassembly):
>+        - Make CString argument a const reference.
>+
>+        * dfg/DFGSpeculativeJIT.cpp:
>+        (JSC::DFG::SpeculativeJIT::compileStringEquality):
>+        * dfg/DFGSpeculativeJIT.h:
>+        * dfg/DFGSpeculativeJIT32_64.cpp:
>+        (JSC::DFG::SpeculativeJIT::cachedGetByIdWithThis):
>+        * dfg/DFGSpeculativeJIT64.cpp:
>+        (JSC::DFG::SpeculativeJIT::cachedGetByIdWithThis):
>+        - Make JITCompiler::JumpList arguments a const reference.
>+
>+        * ftl/FTLLowerDFGToB3.cpp:
>+        (JSC::FTL::DFG::LowerDFGToB3::checkStructure):
>+        - Make RegisteredStructureSet argument a const reference.
>+
>+        * jsc.cpp:
>+        (GlobalObject::moduleLoaderImportModule): Make local auto
>+        variables const references.
>+        (Workers::report): Make String argument a const reference.
>+        (addOption): Make Identifier argument a const reference.
>+        (runJSC): Make CString loop variable a const reference.
>+
> 2018-12-11  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
> 
>         [BigInt] Simplify boolean context evaluation by leveraging JSString::offsetOfLength() == JSBigInt::offsetOfLength()
>diff --git a/Source/JavaScriptCore/assembler/testmasm.cpp b/Source/JavaScriptCore/assembler/testmasm.cpp
>index 3e75574b21d0e0d72fe8c87a65d56728e5482408..8239a943dabaf42d709c7b019111ed4c2c755a08 100644
>--- a/Source/JavaScriptCore/assembler/testmasm.cpp
>+++ b/Source/JavaScriptCore/assembler/testmasm.cpp
>@@ -153,7 +153,7 @@ MacroAssemblerCodeRef<JSEntryPtrTag> compile(Generator&& generate)
> }
> 
> template<typename T, typename... Arguments>
>-T invoke(MacroAssemblerCodeRef<JSEntryPtrTag> code, Arguments... arguments)
>+T invoke(const MacroAssemblerCodeRef<JSEntryPtrTag>& code, Arguments... arguments)
> {
>     void* executableAddress = untagCFunctionPtr<JSEntryPtrTag>(code.code().executableAddress());
>     T (*function)(Arguments...) = bitwise_cast<T(*)(Arguments...)>(executableAddress);
>diff --git a/Source/JavaScriptCore/b3/testb3.cpp b/Source/JavaScriptCore/b3/testb3.cpp
>index b338979ba512ae2ddab609679a0842acff642c05..f90431989be0d9929e03b90baa60a5035cc0526d 100644
>--- a/Source/JavaScriptCore/b3/testb3.cpp
>+++ b/Source/JavaScriptCore/b3/testb3.cpp
>@@ -164,7 +164,7 @@ void lowerToAirForTesting(Procedure& proc)
> }
> 
> template<typename Func>
>-void checkDisassembly(Compilation& compilation, const Func& func, CString failText)
>+void checkDisassembly(Compilation& compilation, const Func& func, const CString& failText)
> {
>     CString disassembly = compilation.disassembly();
>     if (func(disassembly.data()))
>diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
>index 465bf3fb0fd87c4a08e5f5d96fa98f3338a4ab62..bb6363f622ec09a68dfaa8504466b93cac5d94b5 100644
>--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
>@@ -6293,7 +6293,7 @@ void SpeculativeJIT::compilePeepHoleSymbolEquality(Node* node, Node* branchNode)
> void SpeculativeJIT::compileStringEquality(
>     Node* node, GPRReg leftGPR, GPRReg rightGPR, GPRReg lengthGPR, GPRReg leftTempGPR,
>     GPRReg rightTempGPR, GPRReg leftTemp2GPR, GPRReg rightTemp2GPR,
>-    JITCompiler::JumpList fastTrue, JITCompiler::JumpList fastFalse)
>+    const JITCompiler::JumpList& fastTrue, const JITCompiler::JumpList& fastFalse)
> {
>     JITCompiler::JumpList trueCase;
>     JITCompiler::JumpList falseCase;
>diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
>index a20e39a83afe8cbb2929271d3ce25afe33e7ea87..086e29b43e507409c61ba5a053d63203a44597dd 100644
>--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
>+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
>@@ -724,10 +724,10 @@ public:
> 
> #if USE(JSVALUE64)
>     void cachedGetById(CodeOrigin, GPRReg baseGPR, GPRReg resultGPR, unsigned identifierNumber, JITCompiler::Jump slowPathTarget, SpillRegistersMode, AccessType);
>-    void cachedGetByIdWithThis(CodeOrigin, GPRReg baseGPR, GPRReg thisGPR, GPRReg resultGPR, unsigned identifierNumber, JITCompiler::JumpList slowPathTarget = JITCompiler::JumpList());
>+    void cachedGetByIdWithThis(CodeOrigin, GPRReg baseGPR, GPRReg thisGPR, GPRReg resultGPR, unsigned identifierNumber, const JITCompiler::JumpList& slowPathTarget = JITCompiler::JumpList());
> #elif USE(JSVALUE32_64)
>     void cachedGetById(CodeOrigin, GPRReg baseTagGPROrNone, GPRReg basePayloadGPR, GPRReg resultTagGPR, GPRReg resultPayloadGPR, unsigned identifierNumber, JITCompiler::Jump slowPathTarget, SpillRegistersMode, AccessType);
>-    void cachedGetByIdWithThis(CodeOrigin, GPRReg baseTagGPROrNone, GPRReg basePayloadGPR, GPRReg thisTagGPROrNone, GPRReg thisPayloadGPR, GPRReg resultTagGPR, GPRReg resultPayloadGPR, unsigned identifierNumber, JITCompiler::JumpList slowPathTarget = JITCompiler::JumpList());
>+    void cachedGetByIdWithThis(CodeOrigin, GPRReg baseTagGPROrNone, GPRReg basePayloadGPR, GPRReg thisTagGPROrNone, GPRReg thisPayloadGPR, GPRReg resultTagGPR, GPRReg resultPayloadGPR, unsigned identifierNumber, const JITCompiler::JumpList& slowPathTarget = JITCompiler::JumpList());
> #endif
> 
>     void compileDeleteById(Node*);
>@@ -1199,8 +1199,8 @@ public:
>     void compileStringEquality(
>         Node*, GPRReg leftGPR, GPRReg rightGPR, GPRReg lengthGPR,
>         GPRReg leftTempGPR, GPRReg rightTempGPR, GPRReg leftTemp2GPR,
>-        GPRReg rightTemp2GPR, JITCompiler::JumpList fastTrue,
>-        JITCompiler::JumpList fastSlow);
>+        GPRReg rightTemp2GPR, const JITCompiler::JumpList& fastTrue,
>+        const JITCompiler::JumpList& fastSlow);
>     void compileStringEquality(Node*);
>     void compileStringIdentEquality(Node*);
>     void compileStringToUntypedEquality(Node*, Edge stringEdge, Edge untypedEdge);
>diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
>index bcf7c72488e87650af6bcf42ab57fcc17325893b..c1229050b281f427e87ac35313aa5bd2c11a4874 100644
>--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
>@@ -228,7 +228,7 @@ void SpeculativeJIT::cachedGetById(
> 
> void SpeculativeJIT::cachedGetByIdWithThis(
>     CodeOrigin codeOrigin, GPRReg baseTagGPROrNone, GPRReg basePayloadGPR, GPRReg thisTagGPR, GPRReg thisPayloadGPR, GPRReg resultTagGPR, GPRReg resultPayloadGPR,
>-    unsigned identifierNumber, JITCompiler::JumpList slowPathTarget)
>+    unsigned identifierNumber, const JITCompiler::JumpList& slowPathTarget)
> {
>     RegisterSet usedRegisters = this->usedRegisters();
>     
>diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
>index 066274490093fec2b0f2f886372adc041d058782..c1093c68b594f31b1e8dca300d7f1e1c98cbf59d 100644
>--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
>@@ -187,7 +187,7 @@ void SpeculativeJIT::cachedGetById(CodeOrigin codeOrigin, GPRReg baseGPR, GPRReg
>     addSlowPathGenerator(WTFMove(slowPath));
> }
> 
>-void SpeculativeJIT::cachedGetByIdWithThis(CodeOrigin codeOrigin, GPRReg baseGPR, GPRReg thisGPR, GPRReg resultGPR, unsigned identifierNumber, JITCompiler::JumpList slowPathTarget)
>+void SpeculativeJIT::cachedGetByIdWithThis(CodeOrigin codeOrigin, GPRReg baseGPR, GPRReg thisGPR, GPRReg resultGPR, unsigned identifierNumber, const JITCompiler::JumpList& slowPathTarget)
> {
>     CallSiteIndex callSite = m_jit.recordCallSiteAndGenerateExceptionHandlingOSRExitIfNeeded(codeOrigin, m_stream->size());
>     RegisterSet usedRegisters = this->usedRegisters();
>diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>index 72eb9363edbbe575dbbedc6cc8d5a31f053e748b..c75674da459d2091e642b2a84270e7bc52a99f0c 100644
>--- a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>+++ b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>@@ -11288,7 +11288,7 @@ private:
>     template<typename Functor>
>     void checkStructure(
>         LValue structureDiscriminant, const FormattedValue& formattedValue, ExitKind exitKind,
>-        RegisteredStructureSet set, const Functor& weakStructureDiscriminant)
>+        const RegisteredStructureSet& set, const Functor& weakStructureDiscriminant)
>     {
>         if (set.isEmpty()) {
>             terminate(exitKind);
>diff --git a/Source/JavaScriptCore/jsc.cpp b/Source/JavaScriptCore/jsc.cpp
>index 6c5a638beb37c1fb16156ef1d30cace13cfb6891..a4a57f8ddc2e4c3bb5744d9549a3739aa67d8376 100644
>--- a/Source/JavaScriptCore/jsc.cpp
>+++ b/Source/JavaScriptCore/jsc.cpp
>@@ -216,7 +216,7 @@ class GlobalObject;
> class Workers;
> 
> template<typename Func>
>-int runJSC(CommandLine, bool isWorker, const Func&);
>+int runJSC(const CommandLine&, bool isWorker, const Func&);
> static void checkException(ExecState*, GlobalObject*, bool isLastFile, bool hasException, JSValue, CommandLine&, bool& success);
> 
> class Message : public ThreadSafeRefCounted<Message> {
>@@ -257,7 +257,7 @@ public:
>     template<typename Func>
>     void broadcast(const Func&);
>     
>-    void report(String);
>+    void report(const String&);
>     String tryGetReport();
>     String getReport();
>     
>@@ -817,8 +817,8 @@ JSInternalPromise* GlobalObject::moduleLoaderImportModule(JSGlobalObject* global
>     if (sourceOrigin.isNull())
>         return reject(createError(exec, "Could not resolve the module specifier."_s));
> 
>-    auto referrer = sourceOrigin.string();
>-    auto moduleName = moduleNameValue->value(exec);
>+    const auto& referrer = sourceOrigin.string();
>+    const auto& moduleName = moduleNameValue->value(exec);
>     if (UNLIKELY(catchScope.exception()))
>         return reject(catchScope.exception());
> 
>@@ -1643,7 +1643,7 @@ void Workers::broadcast(const Func& func)
>     m_condition.notifyAll();
> }
> 
>-void Workers::report(String string)
>+void Workers::report(const String& string)
> {
>     auto locker = holdLock(m_lock);
>     m_reports.append(string.isolatedCopy());
>@@ -1923,10 +1923,10 @@ EncodedJSValue JSC_HOST_CALL functionTotalCompileTime(ExecState*)
> }
> 
> template<typename ValueType>
>-typename std::enable_if<!std::is_fundamental<ValueType>::value>::type addOption(VM&, JSObject*, Identifier, ValueType) { }
>+typename std::enable_if<!std::is_fundamental<ValueType>::value>::type addOption(VM&, JSObject*, const Identifier&, ValueType) { }
> 
> template<typename ValueType>
>-typename std::enable_if<std::is_fundamental<ValueType>::value>::type addOption(VM& vm, JSObject* optionsObject, Identifier identifier, ValueType value)
>+typename std::enable_if<std::is_fundamental<ValueType>::value>::type addOption(VM& vm, JSObject* optionsObject, const Identifier& identifier, ValueType value)
> {
>     optionsObject->putDirect(vm, identifier, JSValue(value));
> }
>@@ -2759,7 +2759,7 @@ void CommandLine::parseArguments(int argc, char** argv)
> }
> 
> template<typename Func>
>-int runJSC(CommandLine options, bool isWorker, const Func& func)
>+int runJSC(const CommandLine& options, bool isWorker, const Func& func)
> {
>     Worker worker(Workers::singleton());
>     
>@@ -2820,7 +2820,7 @@ int runJSC(CommandLine options, bool isWorker, const Func& func)
>         for (auto& entry : compileTimeStats)
>             compileTimeKeys.append(entry.key);
>         std::sort(compileTimeKeys.begin(), compileTimeKeys.end());
>-        for (CString key : compileTimeKeys)
>+        for (const CString& key : compileTimeKeys)
>             printf("%40s: %.3lf ms\n", key.data(), compileTimeStats.get(key).milliseconds());
>     }
> #endif

Actions: View | Formatted Diff | Diff

Attachments on bug 192680: 357253