Take advantage of pre-decrement and post-increment opcodes for sh4 base JIT.
Created attachment 200892 [details] Take advantage of pre-decrement and post-increment opcodes for sh4 base JIT
Attachment 200892 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/assembler/MacroAssemblerSH4.h', u'Source/JavaScriptCore/assembler/SH4Assembler.h']" exit_code: 1 Source/JavaScriptCore/assembler/SH4Assembler.h:120: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] Total errors found: 1 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 200892 [details] Take advantage of pre-decrement and post-increment opcodes for sh4 base JIT View in context: https://bugs.webkit.org/attachment.cgi?id=200892&action=review >> Source/JavaScriptCore/assembler/SH4Assembler.h:120 >> + MOVW_READ_RMINC_OPCODE = 0x6005, > > enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] All other 100+ entries in this enum are in capital letters, so I think I'll leave it as it is.
I didn't see any regression when testing this patch with: - Tools/Scripts/run-javascriptcore-tests - Tools/Scripts/run-fast-jsc Very little speed improvement according to SunSpider with this patch: Rev 149664 without patch: ============================================ RESULTS (means and 95% confidence intervals) -------------------------------------------- Total: 13461.8ms +/- 0.3% -------------------------------------------- 3d: 2751.5ms +/- 0.2% cube: 838.5ms +/- 0.7% morph: 500.5ms +/- 0.2% raytrace: 1412.5ms +/- 0.4% access: 1948.0ms +/- 0.1% binary-trees: 136.5ms +/- 0.7% fannkuch: 1176.8ms +/- 0.4% nbody: 454.8ms +/- 1.2% nsieve: 180.0ms +/- 1.8% bitops: 1051.8ms +/- 0.5% 3bit-bits-in-byte: 94.8ms +/- 0.8% bits-in-byte: 122.0ms +/- 0.0% bitwise-and: 202.8ms +/- 2.7% nsieve-bits: 632.3ms +/- 0.8% controlflow: 105.5ms +/- 4.5% recursive: 105.5ms +/- 4.5% crypto: 1236.0ms +/- 0.2% aes: 725.0ms +/- 0.8% md5: 328.3ms +/- 1.0% sha1: 182.8ms +/- 0.4% date: 1983.0ms +/- 0.7% format-tofte: 927.3ms +/- 0.6% format-xparb: 1055.8ms +/- 1.8% math: 988.5ms +/- 2.1% cordic: 289.8ms +/- 2.0% partial-sums: 395.5ms +/- 1.5% spectral-norm: 303.3ms +/- 6.0% regexp: 563.3ms +/- 0.1% dna: 563.3ms +/- 0.1% string: 2834.3ms +/- 0.8% base64: 224.5ms +/- 3.3% fasta: 459.0ms +/- 9.8% tagcloud: 568.3ms +/- 0.9% unpack-code: 1242.5ms +/- 4.2% validate-input: 340.0ms +/- 2.5% Rev 149664 with patch: ============================================ RESULTS (means and 95% confidence intervals) -------------------------------------------- Total: 13311.0ms +/- 0.5% -------------------------------------------- 3d: 2599.8ms +/- 0.4% cube: 834.5ms +/- 0.4% morph: 446.3ms +/- 0.9% raytrace: 1319.0ms +/- 1.0% access: 1957.3ms +/- 0.5% binary-trees: 137.3ms +/- 3.7% fannkuch: 1180.0ms +/- 1.1% nbody: 456.5ms +/- 0.9% nsieve: 183.5ms +/- 0.5% bitops: 1059.3ms +/- 0.4% 3bit-bits-in-byte: 94.5ms +/- 1.7% bits-in-byte: 121.5ms +/- 0.8% bitwise-and: 202.0ms +/- 0.0% nsieve-bits: 641.3ms +/- 0.8% controlflow: 103.3ms +/- 0.8% recursive: 103.3ms +/- 0.8% crypto: 1236.3ms +/- 0.9% aes: 739.8ms +/- 1.1% md5: 313.5ms +/- 0.5% sha1: 183.0ms +/- 3.0% date: 2020.0ms +/- 1.5% format-tofte: 949.0ms +/- 1.6% format-xparb: 1071.0ms +/- 3.4% math: 987.0ms +/- 2.7% cordic: 293.0ms +/- 2.3% partial-sums: 410.8ms +/- 7.9% spectral-norm: 283.3ms +/- 0.3% regexp: 532.5ms +/- 1.2% dna: 532.5ms +/- 1.2% string: 2815.8ms +/- 1.1% base64: 213.8ms +/- 2.0% fasta: 452.5ms +/- 9.2% tagcloud: 588.8ms +/- 3.3% unpack-code: 1189.3ms +/- 4.8% validate-input: 371.5ms +/- 2.9%
Comment on attachment 200892 [details] Take advantage of pre-decrement and post-increment opcodes for sh4 base JIT View in context: https://bugs.webkit.org/attachment.cgi?id=200892&action=review I had some war moments while reading this, so i'll hold off on r+ pending replies > Source/JavaScriptCore/assembler/MacroAssemblerSH4.h:1043 > - m_assembler.loadConstant(address.offset, scr); > + m_assembler.loadConstant(address.offset + 8, scr); wat? > Source/JavaScriptCore/assembler/MacroAssemblerSH4.h:1057 > + add32(TrustedImm32(address.offset + 8), scr); again wat? > Source/JavaScriptCore/assembler/MacroAssemblerSH4.h:1170 > - m_assembler.ensureSpace(m_assembler.maxInstructionSize + 68, sizeof(uint32_t)); > + m_assembler.ensureSpace(m_assembler.maxInstructionSize + 58, sizeof(uint32_t)); This magic constant scares me, what is it? >>> Source/JavaScriptCore/assembler/SH4Assembler.h:120 >>> + MOVW_READ_RMINC_OPCODE = 0x6005, >> >> enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] > > All other 100+ entries in this enum are in capital letters, so I think I'll leave it as it is. I think we just need to modify the style bot to ignore these
Comment on attachment 200892 [details] Take advantage of pre-decrement and post-increment opcodes for sh4 base JIT View in context: https://bugs.webkit.org/attachment.cgi?id=200892&action=review >> Source/JavaScriptCore/assembler/MacroAssemblerSH4.h:1043 >> + m_assembler.loadConstant(address.offset + 8, scr); > > wat? Before patch we had something like: 1. load effective address in scr 2. store first part of the double at address scr 3. add 4 to scr 4. store second part of the double at address scr With patch, I do this as it is in LLINT: 1. load effective address + 8 in scr 2. store the second part of the double at address scr with predecrement of scr (-4) 3. store the first part of the double at address scr with predecrement of scr (-4) Result is the same but we save one opcode. >> Source/JavaScriptCore/assembler/MacroAssemblerSH4.h:1057 >> + add32(TrustedImm32(address.offset + 8), scr); > > again wat? Same idea (and I removed the if, because this case is already checked in add32() function). >> Source/JavaScriptCore/assembler/MacroAssemblerSH4.h:1170 >> + m_assembler.ensureSpace(m_assembler.maxInstructionSize + 58, sizeof(uint32_t)); > > This magic constant scares me, what is it? It scares me too :/ From what I understand, it ensures that we'll have enough contiguous space in the instruction buffer (to be able to do relative short jumps for instance). I reduced the constant size from 10 because I removed 10 bytes of instructions, but I'm not very proud of this... >>>> Source/JavaScriptCore/assembler/SH4Assembler.h:120 >>>> + MOVW_READ_RMINC_OPCODE = 0x6005, >>> >>> enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] >> >> All other 100+ entries in this enum are in capital letters, so I think I'll leave it as it is. > > I think we just need to modify the style bot to ignore these Ok
Comment on attachment 200892 [details] Take advantage of pre-decrement and post-increment opcodes for sh4 base JIT Clearing flags on attachment: 200892 Committed r149676: <http://trac.webkit.org/changeset/149676>
All reviewed patches have been landed. Closing bug.