Summary: | [JSC] Put the x86 Assembler on a binary diet | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Benjamin Poulain <benjamin> | ||||||||||
Component: | New Bugs | Assignee: | Benjamin Poulain <benjamin> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | barraclough, commit-queue, darin, keith_miller, mark.lam, msaboff, saam | ||||||||||
Priority: | P2 | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Benjamin Poulain
2016-03-19 12:59:42 PDT
Created attachment 274520 [details]
Patch
Created attachment 274523 [details]
Patch
Created attachment 274627 [details]
Patch
Comment on attachment 274627 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=274627&action=review > Source/JavaScriptCore/ChangeLog:68 > + This patch reduces the binary size by 66k. It is a small > + speedup on Sunspider. Speed-up!? Wow. > Source/JavaScriptCore/assembler/AssemblerBuffer.h:173 > + // LocalWriter *CANNOT* be mixed with other types access to AssemblerBuffer. "types access" -> "types of access" > Source/JavaScriptCore/assembler/AssemblerBuffer.h:174 > + // AssemblerBuffer cannot be used until its LocalWritter goes out of scope. "LocalWritter" -> "LocalWriter" > Source/JavaScriptCore/assembler/AssemblerBuffer.h:210 > + char* m_storrageBuffer; "m_storrageBuffer" -> "m_storageBuffer" > Source/JavaScriptCore/assembler/AssemblerBuffer.h:255 > + NEVER_INLINE void outOfLineGrow() > + { > + m_storage.grow(); > + } Could make this private instead of protected. > Source/JavaScriptCore/assembler/X86Assembler.h:2906 > +#if CPU(X86_64) > + // Byte operand register spl & above require a REX prefix (to prevent the 'H' registers be accessed). > + inline static bool byteRegRequiresRex(int reg) > + { > + static_assert(X86Registers::esp == 4, "Necessary condition for OR-masking"); > + return (reg >= X86Registers::esp); > + } > + inline static bool byteRegRequiresRex(int a, int b) > + { > + return byteRegRequiresRex(a | b); > + } > + > + // Registers r8 & above require a REX prefixe. > + inline static bool regRequiresRex(int reg) > + { > + static_assert(X86Registers::r8 == 8, "Necessary condition for OR-masking"); > + return (reg >= X86Registers::r8); > + } > + inline static bool regRequiresRex(int a, int b) > + { > + return regRequiresRex(a | b); > + } > + inline static bool regRequiresRex(int a, int b, int c) > + { > + return regRequiresRex(a | b | c); > + } > +#else > + inline static bool byteRegRequiresRex(int) { return false; } > + inline static bool byteRegRequiresRex(int, int) { return false; } > + inline static bool regRequiresRex(int) { return false; } > + inline static bool regRequiresRex(int, int) { return false; } > + inline static bool regRequiresRex(int, int, int) { return false; } > +#endif Since we are inside a class, all these "inline" can and should be omitted; they have no effect. > Source/JavaScriptCore/assembler/X86Assembler.h:2919 > + static const RegisterID noBase = X86Registers::ebp; > + static const RegisterID hasSib = X86Registers::esp; > + static const RegisterID noIndex = X86Registers::esp; Can/should we use constexpr here instead of just const? Might be a good habit for the future. Created attachment 274965 [details]
Patch
Comment on attachment 274965 [details] Patch Clearing flags on attachment: 274965 Committed r198708: <http://trac.webkit.org/changeset/198708> All reviewed patches have been landed. Closing bug. |