Last Comment Bug 766899 - IonMonkey: Remove some unnecessary register moves
: IonMonkey: Remove some unnecessary register moves
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Jan de Mooij [:jandem]
:
Mentors:
Depends on:
Blocks: IonSpeed
  Show dependency treegraph
 
Reported: 2012-06-21 02:52 PDT by Jan de Mooij [:jandem]
Modified: 2012-06-22 05:35 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (12.90 KB, patch)
2012-06-21 02:52 PDT, Jan de Mooij [:jandem]
no flags Details | Diff | Review
Patch (8.16 KB, patch)
2012-06-21 04:40 PDT, Jan de Mooij [:jandem]
dvander: review+
Details | Diff | Review

Description Jan de Mooij [:jandem] 2012-06-21 02:52:22 PDT
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.
Comment 1 Jan de Mooij [:jandem] 2012-06-21 04:40:52 PDT
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.
Comment 2 David Anderson [:dvander] 2012-06-21 18:10:30 PDT
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)?
Comment 3 Jan de Mooij [:jandem] 2012-06-22 05:35:24 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.