Port new version of ParallelMove's LGapResolver to X64.

Port new version of ParallelMove's LGapResolver to X64.

Committed: http://code.google.com/p/v8/source/detail?r=6452

[Kevin Millikin (Chromium), 2011-01-24 14:57:12]

LGTM.  Some small comments, mostly comments that need to be updated.

http://codereview.chromium.org/6366003/diff/8001/src/ia32/lithium-gap-resolve...
File src/ia32/lithium-gap-resolver-ia32.cc (right):

http://codereview.chromium.org/6366003/diff/8001/src/ia32/lithium-gap-resolve...
src/ia32/lithium-gap-resolver-ia32.cc:35: : cgen_(owner), moves_(32),
source_uses_(), destination_uses_(),
I prefer treating the initializers like arguments in declarations (breaking one
per line if they don't all fit on one line):

    : cgen_(owner),
      moves_(32),
      source_uses_(),
      // ...

http://codereview.chromium.org/6366003/diff/8001/src/x64/lithium-codegen-x64.h
File src/x64/lithium-codegen-x64.h (right):

http://codereview.chromium.org/6366003/diff/8001/src/x64/lithium-codegen-x64....
src/x64/lithium-codegen-x64.h:44: class LGapNode;
No need for this forward declaration anymore.

http://codereview.chromium.org/6366003/diff/8001/src/x64/lithium-gap-resolver...
File src/x64/lithium-gap-resolver-x64.cc (right):

http://codereview.chromium.org/6366003/diff/8001/src/x64/lithium-gap-resolver...
src/x64/lithium-gap-resolver-x64.cc:208: }
I guess there should be a blank line after this one to match the other cases.

http://codereview.chromium.org/6366003/diff/8001/src/x64/lithium-gap-resolver...
src/x64/lithium-gap-resolver-x64.cc:248: // Register-memory.  Use a free
register as a temp if possible.  Do not
Update the comment so it doesn't mention the spill on demand trick of IA32. 
Just say you'll use on kScratchRegister.

http://codereview.chromium.org/6366003/diff/8001/src/x64/lithium-gap-resolver...
src/x64/lithium-gap-resolver-x64.cc:258: } else if (source->IsStackSlot() &&
destination->IsStackSlot()) {
This case could be

} else if ((source->IsStackSlot() && destination->IsStackSlot()) ||
           (source->IsDoubleStackSlot() && destination->IsDoubleStackSlot())) {

http://codereview.chromium.org/6366003/diff/8001/src/x64/lithium-gap-resolver...
src/x64/lithium-gap-resolver-x64.cc:276: if (other->IsDoubleRegister()) {
I think this case (double register/double register) would be clearer if you had
before this:

} else if (source->IsDoubleRegister() && destination->IsDoubleRegister()) {
  ... 
} else if ...  // double register/memory cases.

http://codereview.chromium.org/6366003/diff/8001/src/x64/lithium-gap-resolver...
src/x64/lithium-gap-resolver-x64.cc:289: // Double-width memory-to-memory. 
Spill on demand to use a general
If you don't move this to share the other memory/memory case, update the comment
here because you don't spill on demand.

[William Hesse, 2011-01-26 10:05:57]

http://codereview.chromium.org/6366003/diff/8001/src/ia32/lithium-gap-resolve...
File src/ia32/lithium-gap-resolver-ia32.cc (right):

http://codereview.chromium.org/6366003/diff/8001/src/ia32/lithium-gap-resolve...
src/ia32/lithium-gap-resolver-ia32.cc:35: : cgen_(owner), moves_(32),
source_uses_(), destination_uses_(),
On 2011/01/24 14:57:12, Kevin Millikin wrote:
> I prefer treating the initializers like arguments in declarations (breaking
one
> per line if they don't all fit on one line):
> 
>     : cgen_(owner),
>       moves_(32),
>       source_uses_(),
>       // ...

Done.

http://codereview.chromium.org/6366003/diff/8001/src/x64/lithium-codegen-x64.h
File src/x64/lithium-codegen-x64.h (right):

http://codereview.chromium.org/6366003/diff/8001/src/x64/lithium-codegen-x64....
src/x64/lithium-codegen-x64.h:44: class LGapNode;
On 2011/01/24 14:57:12, Kevin Millikin wrote:
> No need for this forward declaration anymore.

Done.

http://codereview.chromium.org/6366003/diff/8001/src/x64/lithium-gap-resolver...
File src/x64/lithium-gap-resolver-x64.cc (right):

http://codereview.chromium.org/6366003/diff/8001/src/x64/lithium-gap-resolver...
src/x64/lithium-gap-resolver-x64.cc:248: // Register-memory.  Use a free
register as a temp if possible.  Do not
On 2011/01/24 14:57:12, Kevin Millikin wrote:
> Update the comment so it doesn't mention the spill on demand trick of IA32. 
> Just say you'll use on kScratchRegister.

Done.

http://codereview.chromium.org/6366003/diff/8001/src/x64/lithium-gap-resolver...
src/x64/lithium-gap-resolver-x64.cc:258: } else if (source->IsStackSlot() &&
destination->IsStackSlot()) {
On 2011/01/24 14:57:12, Kevin Millikin wrote:
> This case could be
> 
> } else if ((source->IsStackSlot() && destination->IsStackSlot()) ||
>            (source->IsDoubleStackSlot() && destination->IsDoubleStackSlot()))
{

Done.

http://codereview.chromium.org/6366003/diff/8001/src/x64/lithium-gap-resolver...
src/x64/lithium-gap-resolver-x64.cc:276: if (other->IsDoubleRegister()) {
On 2011/01/24 14:57:12, Kevin Millikin wrote:
> I think this case (double register/double register) would be clearer if you
had
> before this:
> 
> } else if (source->IsDoubleRegister() && destination->IsDoubleRegister()) {
>   ... 
> } else if ...  // double register/memory cases.

Done.

http://codereview.chromium.org/6366003/diff/8001/src/x64/lithium-gap-resolver...
src/x64/lithium-gap-resolver-x64.cc:289: // Double-width memory-to-memory. 
Spill on demand to use a general
On 2011/01/24 14:57:12, Kevin Millikin wrote:
> If you don't move this to share the other memory/memory case, update the
comment
> here because you don't spill on demand.

Done.