[JSC] Put the x86 Assembler on a binary diet
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.