IonMonkey: Remove some unnecessary register moves

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jandem, Assigned: jandem)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
Created attachment 635229 [details] [diff] [review]
Patch

On x86, many common instructions use useRegisterAtStart + defineReuseInput. Due to the way defineReuseInput now works (allocate a new register with some hints, insert a move from the input to this new register if needed), we sometimes generate code like this:

mov stack -> eax
mov eax -> esi
add 1, esi

With useAtStart instead of useRegisterAtStart we emit

mov stack -> eax
add 1, eax

This avoids a second register + move in some cases and is a small win on my am3 micro-benchmark, v8-crypto and Kraken.
Attachment #635229 - Flags: review?(dvander)
(Assignee)

Comment 1

5 years ago
Created attachment 635256 [details] [diff] [review]
Patch

Another approach. useAtStart instead of useRegisterAtStart is not very intuitive and the way reuseInput is handled is a regalloc implementation detail, so this seems a bit cleaner.
Attachment #635229 - Attachment is obsolete: true
Attachment #635229 - Flags: review?(dvander)
Attachment #635256 - Flags: review?(dvander)
Comment on attachment 635256 [details] [diff] [review]
Patch

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

::: js/src/ion/LinearScan.cpp
@@ +588,5 @@
> +                        // input.
> +                        LUse *inputUse = ins->getOperand(def->getReusedInput())->toUse();
> +                        JS_ASSERT(inputUse->policy() == LUse::REGISTER);
> +                        JS_ASSERT(inputUse->usedAtStart());
> +                        *inputUse = LUse(inputUse->virtualRegister(), LUse::ANY, /* usedAtStart = */ true);

Just making sure: if this does get an Operand, do we fill in a Register during reification (since I think consumers will expect that)?
Attachment #635256 - Flags: review?(dvander) → review+
(Assignee)

Comment 3

5 years ago
https://hg.mozilla.org/projects/ionmonkey/rev/0b892385c0a5

(In reply to David Anderson [:dvander] from comment #2)
> Comment on attachment 635256 [details] [diff] [review]
> 
> Just making sure: if this does get an Operand, do we fill in a Register
> during reification (since I think consumers will expect that)?

Yeah the input and output are set to the same (register) LAllocation.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.