Looks good to me too, then. On Thu, Sep 9, 2010 at 12:43 PM, <[email protected]> wrote:
> Thanks. I will enable the other cases separately. > > > > http://codereview.chromium.org/3295022/diff/7001/8001 > File src/arm/full-codegen-arm.cc (right): > > http://codereview.chromium.org/3295022/diff/7001/8001#newcode888 > src/arm/full-codegen-arm.cc:888: Register context = cp; > > On 2010/09/09 09:59:26, Kevin Millikin wrote: > >> This code might be clearer with some renaming: >> > > Register current = cp; >> Register next = r1; >> Register temp = r2; >> ... >> > > > Done. > > > http://codereview.chromium.org/3295022/diff/7001/8001#newcode897 > src/arm/full-codegen-arm.cc:897: __ ldr(tmp2, > CodeGenerator::ContextOperand(context, > On 2010/09/09 10:02:19, Kasper Lund wrote: > >> If the full codegen had a ContextOperand (maybe just wrapping the one >> > in > >> CodeGenerator) it would look nicer. >> > > Done. > > http://codereview.chromium.org/3295022/diff/7001/8001#newcode906 > src/arm/full-codegen-arm.cc:906: context = tmp; > > On 2010/09/09 09:59:26, Kevin Millikin wrote: > >> // Walk the rest of the chain using a single register (without >> > clobbering cp). > >> current = next; >> > > Done. > > http://codereview.chromium.org/3295022/diff/7001/8001#newcode916 > src/arm/full-codegen-arm.cc:916: __ Move(tmp, context); > > On 2010/09/09 09:59:26, Kevin Millikin wrote: > >> Don't you know statically whether you've walked up the context chain? >> > Could you > >> do >> > > if (!tmp.is(context)) { >> __ Move(tmp, context); >> } >> > > Yes I do and I have exactly that conditional on the other platforms. > Done. > > > http://codereview.chromium.org/3295022/show > -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
