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

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Dan Gohman [:sunfish]]

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.

Comment 1

[Dan Gohman [:sunfish]]

Attachment: a proposed fix

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.

Comment 2

[Dan Gohman [:sunfish]]

Attachment: an accompanying micro-optimization

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.

Comment 3

[Marty Rosenberg [:mjrosenb]]

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).

Comment 4

[Marty Rosenberg [:mjrosenb]]

> 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.

Comment 5

[Dan Gohman [:sunfish]]

(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.

Comment 6

[Dan Gohman [:sunfish]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/190563720411

Comment 7

[Ryan VanderMeulen [:RyanVM]]

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

Comment 8

[Dan Gohman [:sunfish]]

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 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/89a4ff6bdc93

Comment 10

[Sean Stangl [:sstangl]]

Comment on attachment 777876 [details] [diff] [review]
an accompanying micro-optimization

Review of attachment 777876 [details] [diff] [review]:
-----------------------------------------------------------------

This looks completely reasonable to me.

Comment 11

[Dan Gohman [:sunfish]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0cd7bded61f6

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0cd7bded61f6