Closed Bug 642957 Opened 11 years ago Closed 10 years ago
TI+JM: remove x|0 and x << 0 optimization
In bug 592631 I added a fast-path for double|0 and double << 0. This was a nice optimization for Firefox 4 but AFAIK the TI branch now inlines double-to-int conversion for bitwise operators. This makes the fast path redundant. We should remove it and verify the benchmark in bug 592631 comment 0 does not regress.
Jan de Mooij: I should probably see if any changes to this regress the audio sample aliasing in some code of mine.
I've tested the benchmark from bug 592631 (within the web console), and it didn't regressed (actually it's better by some ms). However with Chromium I get better results (in Firefox without the patch 899 ms, with the patch 894 ms, in Chromium 37 ms).
I've tested a bit more with the benchmark, the results without the patch and with the patch are more or less the same (in a small range of more or less 10 ms). Grant Galitz, could you test the patch with your code? I'd like to be sure there isn't any regression.
The web console is running in a sandboxed environment with various stuff going through proxies, so running any sort of performance tests in it is useless. But yes, looks like the sandbox also doesn't enable the jit. Oh, and we're a bit slower than Chrome with TI here, comparing our 64-bit apples to their 32-bit oranges.
(In reply to Marco Castelluccio from comment #6) > In the jit-test there is a failure, that however happens with or without the > patch: > FAILURES: > -m -j -p sunspider/check-date-format-tofte.js That's bug 600522, the test depends on the time zone. I get the same jit-test/reftest failures, it's annoying... > Instead ubench results seem to have regressed a bit (I tested with > 'sunspider --args="-m -n" ...'). Results are attached. None of the ubench tests use bitwise operators, so it's probably safe to assume this is noise. It looks like we will now generate an extra "or 0, reg" instruction though. You can get rid of it by changing else masm.or32(Imm32(rhsInt), reg); to this: else if (rhsInt != 0) masm.or32(Imm32(rhsInt), reg); We already do something similar for <<. Can you update the patch with this change? Thanks!
I've done the change. Now the ubench results are better with the patch than without, probably as you said it's noise :)
Comment on attachment 568816 [details] [diff] [review] Remove optimization v2 Thanks! Pushed to inbound: http://hg.mozilla.org/integration/mozilla-inbound/rev/a955bd83e9df
Attachment #568816 - Flags: review?(jandemooij) → review+
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.