LGTM. I don't know how it would look if you introduced a helper function for constructing these Operands? At least it would eliminate the repeated assertions.
On Sat, Nov 8, 2008 at 2:58 AM, <[EMAIL PROTECTED]> wrote: > Reviewers: Kasper Lund, > > Message: > Originally I had changed Operand to make this optimization when scale = > times_2, but I don't really like the idea of having lots of > optimizations in the assembler, so you can't get specific encodings if > you wanted them. > > Adam pointed out that lea eax,[eax+eax*1] should still be shorter as shl > eax, 1, which just happens because kSmiTag is 0. I don't know if it's > worth trying to handle that case somehow. > > > > Description: > Use shorter SIB encoding. For example, the previous: > 8d044500000000 lea eax,[eax*2+0x0] > Will be encoded with the much shorter: > 8d0400 lea eax,[eax+eax*1] > > > > Please review this at http://codereview.chromium.org/9723 > > Affected files: > M src/codegen-ia32.cc > > > Index: src/codegen-ia32.cc > diff --git a/src/codegen-ia32.cc b/src/codegen-ia32.cc > index > 22f35232bf95c86243a9cd33deecadc4d3af7c46..19f64b026927fb3b51503f13f31dbc9961d38213 > 100644 > --- a/src/codegen-ia32.cc > +++ b/src/codegen-ia32.cc > @@ -1095,7 +1095,7 @@ void CodeGenerator::SmiOperation(Token::Value op, > __ j(not_zero, deferred->enter(), not_taken); > // tag result and store it in TOS (eax) > ASSERT(kSmiTagSize == times_2); // adjust code if not the case > - __ lea(eax, Operand(ebx, times_2, kSmiTag)); > + __ lea(eax, Operand(ebx, ebx, times_1, kSmiTag)); > __ bind(deferred->exit()); > frame_->Push(eax); > } > @@ -1124,7 +1124,7 @@ void CodeGenerator::SmiOperation(Token::Value op, > __ j(not_zero, deferred->enter(), not_taken); > // tag result and store it in TOS (eax) > ASSERT(kSmiTagSize == times_2); // adjust code if not the case > - __ lea(eax, Operand(ebx, times_2, kSmiTag)); > + __ lea(eax, Operand(ebx, ebx, times_1, kSmiTag)); > __ bind(deferred->exit()); > frame_->Push(eax); > } > @@ -1651,7 +1651,7 @@ void CodeGenerator::GenerateFastCaseSwitchJumpTable( > __ j(greater_equal, fail_label, not_taken); > > // 0 is placeholder. > - __ jmp(Operand(eax, times_2, 0x0, RelocInfo::INTERNAL_REFERENCE)); > + __ jmp(Operand(eax, eax, times_1, 0x0, RelocInfo::INTERNAL_REFERENCE)); > // calculate address to overwrite later with actual address of table. > int32_t jump_table_ref = __ pc_offset() - sizeof(int32_t); > > @@ -4062,7 +4062,7 @@ void > GenericBinaryOpStub::GenerateSmiCode(MacroAssembler* masm, Label* slow) { > __ j(not_zero, slow); > // Tag the result and store it in register eax. > ASSERT(kSmiTagSize == times_2); // adjust code if not the case > - __ lea(eax, Operand(eax, times_2, kSmiTag)); > + __ lea(eax, Operand(eax, eax, times_1, kSmiTag)); > break; > > case Token::MOD: > @@ -4123,7 +4123,7 @@ void > GenericBinaryOpStub::GenerateSmiCode(MacroAssembler* masm, Label* slow) { > } > // Tag the result and store it in register eax. > ASSERT(kSmiTagSize == times_2); // adjust code if not the case > - __ lea(eax, Operand(eax, times_2, kSmiTag)); > + __ lea(eax, Operand(eax, eax, times_1, kSmiTag)); > break; > > default: > @@ -4250,7 +4250,7 @@ void GenericBinaryOpStub::Generate(MacroAssembler* > masm) { > > // Tag smi result and return. > ASSERT(kSmiTagSize == times_2); // adjust code if not the case > - __ lea(eax, Operand(eax, times_2, kSmiTag)); > + __ lea(eax, Operand(eax, eax, times_1, kSmiTag)); > __ ret(2 * kPointerSize); > > // All ops except SHR return a signed int32 that we load in a > HeapNumber. > > > --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
