898461 - div and mod optimizations

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Dan Gohman [:sunfish]]

The following are two div/mod optimization patches for x86/x64.

Comment 1

[Dan Gohman [:sunfish]]

Attachment: optimize-modi.patch

Make LModI require its lhs to be in eax, instead of requiring a temp and copying lhs into it. This theoretically gives the register allocator the flexibility to avoid a copy in some cases :-).

Comment 2

[Dan Gohman [:sunfish]]

Attachment: move overflow and divide-by-zero code out of line

Comment 3

[Brian Hackett [Laid off!]]

Comment on attachment 781737 [details] [diff] [review]
optimize-modi.patch

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

The idiv instruction in ModI can clobber eax, right?  Removing the useFixed temp seems to mean the regalloc is allowed to reuse eax in future instructions which have the mod lhs as an input, without reloading it.  Is there another mechanism preventing this?

Comment 4

[Dan Gohman [:sunfish]]

> The idiv instruction in ModI can clobber eax, right?  Removing the useFixed
> temp seems to mean the regalloc is allowed to reuse eax in future
> instructions which have the mod lhs as an input, without reloading it.  Is
> there another mechanism preventing this?

Oh, you're right. And I can even reproduce the problem with --ion-regalloc=backtracking. Would it be valid to do useFixed(mod->lhs(), eax) and tempFixed(eax) at the same time instead?

Comment 5

[Brian Hackett [Laid off!]]

(In reply to Dan Gohman [:sunfish] from comment #4)
> > The idiv instruction in ModI can clobber eax, right?  Removing the useFixed
> > temp seems to mean the regalloc is allowed to reuse eax in future
> > instructions which have the mod lhs as an input, without reloading it.  Is
> > there another mechanism preventing this?
> 
> Oh, you're right. And I can even reproduce the problem with
> --ion-regalloc=backtracking. Would it be valid to do useFixed(mod->lhs(),
> eax) and tempFixed(eax) at the same time instead?

Unfortunately it doesn't look like this will work, looking at the intervals generated by the regalloc for uses/defs/temps.  When these are fixed their ranges all cover the instruction's input.  Not sure why this holds with lsra for defs, as the backtracking allocator doesn't have this restriction.

Comment 6

[Dan Gohman [:sunfish]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/505ad1c506cc

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/505ad1c506cc