MIPS: Fix performance regression on sunspider string-unpack-code.

MIPS: Fix performance regression on sunspider string-unpack-code.

Regression surfaced with r19009, but latent error was present since
r17767.

BUG=
R=dusan.milosavljevic@rt-rk.com

Committed: https://code.google.com/p/v8/source/detail?r=19500

[Paul Lind, 2014-02-19 08:49:15]

PTAL. On my test board, I got back to baseline performance on this test.

[kisg, 2014-02-19 11:25:26]

LGTM

[dusmil, 2014-02-19 13:31:46]

Good catch.

The problem here is that other architectures have compare operation separated
from jumps, and branches.
The fix is good, but maybe we add some comments in code for clarity.

[dusmil, 2014-02-19 13:47:35]

Oops, not good.

The at register will be clobbered after jump is taken in long branch mode.

I suggest using same scratch register for smi check and map compare to
achieve same effect as it is on other arches.

Like this:

diff --git a/src/mips/stub-cache-mips.cc b/src/mips/stub-cache-mips.cc
index 10813eb..7e116f2 100644
--- a/src/mips/stub-cache-mips.cc
+++ b/src/mips/stub-cache-mips.cc
@@ -1472,7 +1472,9 @@ Handle<Code>
BaseLoadStoreStubCompiler::CompilePolymorphicIC(

   Label number_case;
   Label* smi_target = IncludesNumberType(types) ? &number_case : &miss;
-  __ JumpIfSmi(receiver(), smi_target);
+
+  __ SmiTst(receiver(), scratch2());
+  __ Branch(smi_target, eq, scratch2(), Operand(zero_reg));

   Register map_reg = scratch1();

@@ -1484,12 +1486,13 @@ Handle<Code>
BaseLoadStoreStubCompiler::CompilePolymorphicIC(
     Handle<Map> map = IC::TypeToMap(*type, isolate());
     if (!map->is_deprecated()) {
       number_of_handled_maps++;
+      __ Subu(scratch2(), map_reg, Operand(map)); //Compare maps.
       if (type->Is(HeapType::Number())) {
         ASSERT(!number_case.is_unused());
         __ bind(&number_case);
       }
       __ Jump(handlers->at(current), RelocInfo::CODE_TARGET,
-          eq, map_reg, Operand(map));
+          eq, scratch2(), Operand(zero_reg));
     }
   }
   ASSERT(number_of_handled_maps != 0);

[kisg, 2014-02-19 16:26:49]

On 2014/02/19 13:47:35, dusmil wrote:
> Oops, not good.
> 
> The at register will be clobbered after jump is taken in long branch mode.
> 
> I suggest using same scratch register for smi check and map compare to
> achieve same effect as it is on other arches.
> 
> Like this:
> 
> diff --git a/src/mips/stub-cache-mips.cc b/src/mips/stub-cache-mips.cc
> index 10813eb..7e116f2 100644
> --- a/src/mips/stub-cache-mips.cc
> +++ b/src/mips/stub-cache-mips.cc
> @@ -1472,7 +1472,9 @@ Handle<Code>
> BaseLoadStoreStubCompiler::CompilePolymorphicIC(
> 
>    Label number_case;
>    Label* smi_target = IncludesNumberType(types) ? &number_case : &miss;
> -  __ JumpIfSmi(receiver(), smi_target);
> +
> +  __ SmiTst(receiver(), scratch2());
> +  __ Branch(smi_target, eq, scratch2(), Operand(zero_reg));
> 
>    Register map_reg = scratch1();
> 
> @@ -1484,12 +1486,13 @@ Handle<Code>
> BaseLoadStoreStubCompiler::CompilePolymorphicIC(
>      Handle<Map> map = IC::TypeToMap(*type, isolate());
>      if (!map->is_deprecated()) {
>        number_of_handled_maps++;
> +      __ Subu(scratch2(), map_reg, Operand(map)); //Compare maps.
>        if (type->Is(HeapType::Number())) {
>          ASSERT(!number_case.is_unused());
>          __ bind(&number_case);
>        }
>        __ Jump(handlers->at(current), RelocInfo::CODE_TARGET,
> -          eq, map_reg, Operand(map));
> +          eq, scratch2(), Operand(zero_reg));
>      }
>    }
>    ASSERT(number_of_handled_maps != 0);

Oops, good catch Dusan.

[Paul Lind, 2014-02-19 17:26:45]

Thanks Dusan, I forgot the long-branch issue. New patch, PTAL.

This also includes some additional comments. I stayed with existing JumpIfSmi(),
since I did not see a reason to separate, since we can specify the register
used.

[dusmil, 2014-02-19 17:32:32]

LGTM

[Paul Lind, 2014-02-19 18:30:26]

Committed patchset #3 manually as r19500 (presubmit successful).