Closed Bug 898461 Opened 7 years ago Closed 7 years ago

div and mod optimizations

Categories

(Core :: JavaScript Engine, enhancement)

x86_64
All
enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: sunfish, Unassigned)

Details

Attachments

(2 files)

The following are two div/mod optimization patches for x86/x64.
Whiteboard: [leave open]
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 :-).
Attachment #781737 - Flags: review?(bhackett1024)
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?
Attachment #781737 - Flags: review?(bhackett1024)
> 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?
(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.
Attachment #781738 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/mozilla-central/rev/505ad1c506cc
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.