Sorry - I meant to say LGTM at least a few hours ago. On Tue, Nov 11, 2008 at 5:03 AM, Dean McNamee <[EMAIL PROTECTED]> wrote: > Talked to Kasper today, he brought up a good point of moving to > Immediate() could allow us to eventually move to having registers be > an enum, otherwise we have the problem of mov(reg, reg) and mov(reg, > int) being ambiguous. I can look into assembler cleanup someday, but > for now I think this patch makes sense. > > Kasper? > > On Mon, Nov 10, 2008 at 7:13 AM, Dean McNamee <[EMAIL PROTECTED]> wrote: >> I talked to Kasper, and I think the conclusion was to use Immediate(), >> but I'm not positive. I'll see what he thinks. Thanks for the >> review. >> >> -- dean >> >> On Mon, Nov 10, 2008 at 7:05 AM, <[EMAIL PROTECTED]> wrote: >>> I like it. Even if it doesn't result in a shorter encoding, I think it >>> reads better to get rid of Immediate in places where it isn't needed. >>> >>> >>> http://codereview.chromium.org/10002/diff/401/604 >>> File src/codegen-ia32.cc (right): >>> >>> http://codereview.chromium.org/10002/diff/401/604#newcode1044 >>> Line 1044: __ mov(eax, Immediate(value)); >>> Couldn't this just be __ mov(eax, value)? >>> >>> http://codereview.chromium.org/10002/diff/401/604#newcode4987 >>> Line 4987: __ mov(eax, Immediate(reinterpret_cast<int32_t>(failure))); >>> Similarly __ mov(eax, reinterpret_cast<int32_t>(failure))? >>> >>> http://codereview.chromium.org/10002/diff/401/604#newcode5005 >>> Line 5005: __ mov(eax, Immediate(reinterpret_cast<int32_t>(failure))); >>> Ditto. >>> >>> http://codereview.chromium.org/10002/diff/401/606 >>> File src/macro-assembler-ia32.cc (right): >>> >>> http://codereview.chromium.org/10002/diff/401/606#newcode698 >>> Line 698: mov(eax, Immediate(Factory::undefined_value())); >>> Just mov(eax, Factory::undefined_value())? >>> >>> http://codereview.chromium.org/10002/diff/401/606#newcode790 >>> Line 790: mov(edx, Immediate(code_constant)); >>> Ditto. >>> >>> http://codereview.chromium.org/10002/diff/401/607 >>> File src/stub-cache-ia32.cc (right): >>> >>> http://codereview.chromium.org/10002/diff/401/607#newcode398 >>> Line 398: __ mov(ecx, Immediate(Handle<Map>(transition))); >>> Ditto and below. >>> >>> http://codereview.chromium.org/10002 >>> >> >
--~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
