Reviewers: mvstanton, danno, paul.l..., gergely.kis.imgtec, balazs.kilvady, dusmil.imgtec,

Message:
PTAL.

Description:
MIPS64: VectorICs: keyed element loads were kicking out non-smi keys
unnecessarily

Port 6689cc27ebe60685c025de9ae1f09919093f8213

Original commit message:
Handlers should be in charge of this work. The change uncovered a bug in
vector-ics related to keyed loads into strings. It's important for
StringCharCodeAtGenerator, a helper used in full code and in
LoadIndexedStringStub (a handler) to protect the vector and slot registers
when it makes a runtime call to convert a HeapNumber to a Smi.

It's still possible for the handler to MISS after this call, perhaps due
to out of bounds access. In that case, the vector and slot registers need
to be delivered safely to the MISS handler.

BUG=

Please review this at https://codereview.chromium.org/1025303005/

Base URL: https://chromium.googlesource.com/v8/v8.git@master

Affected files (+17, -11 lines):
  M src/mips64/code-stubs-mips64.cc
  M src/mips64/full-codegen-mips64.cc


Index: src/mips64/code-stubs-mips64.cc
diff --git a/src/mips64/code-stubs-mips64.cc b/src/mips64/code-stubs-mips64.cc index 643ff5cd01e55af03e694f0e39d431438f5fbd77..74477b557bff04bb9cee26c9f98a1be43ea6b605 100644
--- a/src/mips64/code-stubs-mips64.cc
+++ b/src/mips64/code-stubs-mips64.cc
@@ -1380,12 +1380,8 @@ void LoadIndexedStringStub::Generate(MacroAssembler* masm) {
   Register result = v0;
   DCHECK(!scratch.is(receiver) && !scratch.is(index));
   DCHECK(!FLAG_vector_ics ||
-         (!scratch.is(VectorLoadICDescriptor::VectorRegister()) &&
-          result.is(VectorLoadICDescriptor::SlotRegister())));
+         !scratch.is(VectorLoadICDescriptor::VectorRegister()));

- // StringCharAtGenerator doesn't use the result register until it's passed
-  // the different miss possibilities. If it did, we would have a conflict
-  // when FLAG_vector_ics is true.
   StringCharAtGenerator char_at_generator(receiver, index, scratch, result,
                                           &miss,  // When not a string.
                                           &miss,  // When not a number.
@@ -1396,7 +1392,7 @@ void LoadIndexedStringStub::Generate(MacroAssembler* masm) {
   __ Ret();

   StubRuntimeCallHelper call_helper;
-  char_at_generator.GenerateSlow(masm, call_helper);
+  char_at_generator.GenerateSlow(masm, PART_OF_IC_HANDLER, call_helper);

   __ bind(&miss);
   PropertyAccessCompiler::TailCallBuiltin(
@@ -3089,7 +3085,7 @@ void CallICStub::GenerateMiss(MacroAssembler* masm) {


 void StringCharCodeAtGenerator::GenerateSlow(
-    MacroAssembler* masm,
+    MacroAssembler* masm, EmbedMode embed_mode,
     const RuntimeCallHelper& call_helper) {
   __ Abort(kUnexpectedFallthroughToCharCodeAtSlowCase);

@@ -3103,7 +3099,12 @@ void StringCharCodeAtGenerator::GenerateSlow(
               DONT_DO_SMI_CHECK);
   call_helper.BeforeCall(masm);
   // Consumed by runtime conversion function:
-  __ Push(object_, index_);
+  if (FLAG_vector_ics && embed_mode == PART_OF_IC_HANDLER) {
+    __ Push(VectorLoadICDescriptor::VectorRegister(),
+            VectorLoadICDescriptor::SlotRegister(), object_, index_);
+  } else {
+    __ Push(object_, index_);
+  }
   if (index_flags_ == STRING_INDEX_IS_NUMBER) {
     __ CallRuntime(Runtime::kNumberToIntegerMapMinusZero, 1);
   } else {
@@ -3116,7 +3117,12 @@ void StringCharCodeAtGenerator::GenerateSlow(
   // have a chance to overwrite it.

   __ Move(index_, v0);
-  __ pop(object_);
+  if (FLAG_vector_ics && embed_mode == PART_OF_IC_HANDLER) {
+    __ Pop(VectorLoadICDescriptor::SlotRegister(),
+           VectorLoadICDescriptor::VectorRegister(), object_);
+  } else {
+    __ pop(object_);
+  }
   // Reload the instance type.
   __ ld(result_, FieldMemOperand(object_, HeapObject::kMapOffset));
   __ lbu(result_, FieldMemOperand(result_, Map::kInstanceTypeOffset));
Index: src/mips64/full-codegen-mips64.cc
diff --git a/src/mips64/full-codegen-mips64.cc b/src/mips64/full-codegen-mips64.cc index 7d1261f1c1c031f441f6607567c283c99f430669..0f157ec1a331e6f6fea91bd24e5c6a9e70bc45dc 100644
--- a/src/mips64/full-codegen-mips64.cc
+++ b/src/mips64/full-codegen-mips64.cc
@@ -4111,7 +4111,7 @@ void FullCodeGenerator::EmitStringCharCodeAt(CallRuntime* expr) {
   __ jmp(&done);

   NopRuntimeCallHelper call_helper;
-  generator.GenerateSlow(masm_, call_helper);
+  generator.GenerateSlow(masm_, NOT_PART_OF_IC_HANDLER, call_helper);

   __ bind(&done);
   context()->Plug(result);
@@ -4160,7 +4160,7 @@ void FullCodeGenerator::EmitStringCharAt(CallRuntime* expr) {
   __ jmp(&done);

   NopRuntimeCallHelper call_helper;
-  generator.GenerateSlow(masm_, call_helper);
+  generator.GenerateSlow(masm_, NOT_PART_OF_IC_HANDLER, call_helper);

   __ bind(&done);
   context()->Plug(result);


--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to