Closed Bug 895465 Opened 6 years ago Closed 6 years ago

IonMonkey: optimize codegen for test(and(x, y))

Categories

(Core :: JavaScript Engine, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: sunfish, Unassigned)

Details

Attachments

(2 files)

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.
Attached patch a proposed fixSplinter Review
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)
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 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+
> 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.
(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.
Whiteboard: [leave open]
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 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+
https://hg.mozilla.org/mozilla-central/rev/0cd7bded61f6
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.