Closed
Bug 895465
Opened 11 years ago
Closed 11 years ago
IonMonkey: optimize codegen for test(and(x, y))
Categories
(Core :: JavaScript Engine, enhancement)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla25
People
(Reporter: sunfish, Unassigned)
Details
Attachments
(2 files)
15.33 KB,
patch
|
mjrosenb
:
review+
|
Details | Diff | Splinter Review |
10.43 KB,
patch
|
sstangl
:
review+
|
Details | Diff | Splinter Review |
x86, x64, and ARM all have instructions which fold a bitwise and with a zero comparison -- test on x86 and x64, and tst on ARM. This corresponds to a fairly common code idiom of "if (x & y)", and in particular bit tests like "if (x & 16)". IonMonkey currently emits a separate and and comparison for such code.
Reporter | ||
Comment 1•11 years ago
|
||
This patch introduces target-independent lowering, and target-specific codegen support for x86/x64 and ARM. I'm not very familiar with ARM, so I'd especially appreciate any ARM-specific advice here.
Attachment #777870 -
Flags: review?(mrosenberg)
Reporter | ||
Comment 2•11 years ago
|
||
This patch introduces an x86/x64-specific micro-optimization which works well with the tests introduced in the other patch. It turns code like this: if (x & 256) into code like testb $0x1, %ah assuming x is in %eax, instead of testl $0x100, %eax so it saves 2 bytes of encoding. However, I also recognize that H registers are fairly obscure, so I'll understand if you don't want this patch.
Attachment #777876 -
Flags: review?(sstangl)
Comment 3•11 years ago
|
||
Comment on attachment 777870 [details] [diff] [review] a proposed fix Review of attachment 777870 [details] [diff] [review]: ----------------------------------------------------------------- The ARM parts look correct to me. Do you happen to know how common slight variations of if (a&b) are? e.g. if (!a&b), or if (a&!b).
Attachment #777870 -
Flags: review?(mrosenberg) → review+
Comment 4•11 years ago
|
||
> However, I also recognize that H registers are fairly obscure, so I'll
> understand if you don't want this patch.
Isn't it incredibly slow to access the h registers? Of course, you would know much better than I would.
Reporter | ||
Comment 5•11 years ago
|
||
(In reply to Marty Rosenberg [:mjrosenb] from comment #3) > The ARM parts look correct to me. Do you happen to know how common slight > variations of if (a&b) are? e.g. if (!a&b), or if (a&!b). Emscripten effectively converts many short-circuit operators (&&, ||) into bitwise (&, |) when it is safe and "cheap" (according to target-independent heuristics), so it's pretty common to see bitand tests of all forms coming out of compiled C++ code. (In reply to Marty Rosenberg [:mjrosenb] from comment #4) > Isn't it incredibly slow to access the h registers? Of course, you would > know much better than I would. It is generally slow to write to the h registers, but reading them is generally fine.
Reporter | ||
Updated•11 years ago
|
Whiteboard: [leave open]
Reporter | ||
Comment 6•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/190563720411
Comment 7•11 years ago
|
||
Backed out for Linux32 jit-test failures. https://hg.mozilla.org/integration/mozilla-inbound/rev/b6e1a6d7f622 https://tbpl.mozilla.org/php/getParsedLog.php?id=25570903&tree=Mozilla-Inbound
Reporter | ||
Comment 8•11 years ago
|
||
Reapplied with a check to only perform this optimization when the operands of the BitAnd are Int32. https://hg.mozilla.org/integration/mozilla-inbound/rev/89a4ff6bdc93
Comment 10•11 years ago
|
||
Comment on attachment 777876 [details] [diff] [review] an accompanying micro-optimization Review of attachment 777876 [details] [diff] [review]: ----------------------------------------------------------------- This looks completely reasonable to me.
Attachment #777876 -
Flags: review?(sstangl) → review+
Reporter | ||
Comment 11•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/0cd7bded61f6
Whiteboard: [leave open]
Comment 12•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/0cd7bded61f6
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in
before you can comment on or make changes to this bug.
Description
•