http://codereview.chromium.org/6366003/diff/8001/src/ia32/lithium-gap-resolver-ia32.cc
File src/ia32/lithium-gap-resolver-ia32.cc (right):
http://codereview.chromium.org/6366003/diff/8001/src/ia32/lithium-gap-resolver-ia32.cc#newcode35
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.h#newcode44
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-x64.cc
File src/x64/lithium-gap-resolver-x64.cc (right):
http://codereview.chromium.org/6366003/diff/8001/src/x64/lithium-gap-resolver-x64.cc#newcode248
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-x64.cc#newcode258
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-x64.cc#newcode276
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-x64.cc#newcode289
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.
http://codereview.chromium.org/6366003/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
