Revision: 12721
Author:   [email protected]
Date:     Mon Oct 15 00:25:20 2012
Log:      Consistently make the bounds check for AccessArgumentsAt explicit.

This has the advantage that AccessArgumentsAt itself can't deopt anymore and the
bounds check is visible for the elimination phase. Furthermore, things are
simply more consistent now, a good thing in itself. :-)

Review URL: https://codereview.chromium.org/11106012
http://code.google.com/p/v8/source/detail?r=12721

Modified:
 /branches/bleeding_edge/src/arm/lithium-arm.cc
 /branches/bleeding_edge/src/arm/lithium-codegen-arm.cc
 /branches/bleeding_edge/src/hydrogen.cc
 /branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc
 /branches/bleeding_edge/src/ia32/lithium-ia32.cc
 /branches/bleeding_edge/src/mips/lithium-codegen-mips.cc
 /branches/bleeding_edge/src/mips/lithium-mips.cc
 /branches/bleeding_edge/src/x64/lithium-codegen-x64.cc
 /branches/bleeding_edge/src/x64/lithium-x64.cc

=======================================
--- /branches/bleeding_edge/src/arm/lithium-arm.cc      Mon Oct  8 05:50:15 2012
+++ /branches/bleeding_edge/src/arm/lithium-arm.cc      Mon Oct 15 00:25:20 2012
@@ -2164,12 +2164,10 @@


LInstruction* LChunkBuilder::DoAccessArgumentsAt(HAccessArgumentsAt* instr) {
-  LOperand* arguments = UseRegister(instr->arguments());
+  LOperand* args = UseRegister(instr->arguments());
   LOperand* length = UseTempRegister(instr->length());
   LOperand* index = UseRegister(instr->index());
-  LAccessArgumentsAt* result =
-      new(zone()) LAccessArgumentsAt(arguments, length, index);
-  return AssignEnvironment(DefineAsRegister(result));
+ return DefineAsRegister(new(zone()) LAccessArgumentsAt(args, length, index));
 }


=======================================
--- /branches/bleeding_edge/src/arm/lithium-codegen-arm.cc Fri Oct 12 04:09:14 2012 +++ /branches/bleeding_edge/src/arm/lithium-codegen-arm.cc Mon Oct 15 00:25:20 2012
@@ -2904,14 +2904,9 @@
   Register length = ToRegister(instr->length());
   Register index = ToRegister(instr->index());
   Register result = ToRegister(instr->result());
-
-  // Bailout index is not a valid argument index. Use unsigned check to get
-  // negative check for free.
-  __ sub(length, length, index, SetCC);
-  DeoptimizeIf(ls, instr->environment());
-
   // There are two words between the frame pointer and the last argument.
   // Subtracting from length accounts for one of them add one more.
+  __ sub(length, length, index);
   __ add(length, length, Operand(1));
   __ ldr(result, MemOperand(arguments, length, LSL, kPointerSizeLog2));
 }
=======================================
--- /branches/bleeding_edge/src/hydrogen.cc     Thu Oct 11 03:52:58 2012
+++ /branches/bleeding_edge/src/hydrogen.cc     Mon Oct 15 00:25:20 2012
@@ -9016,8 +9016,10 @@
   HInstruction* elements = AddInstruction(
       new(zone()) HArgumentsElements(false));
HInstruction* length = AddInstruction(new(zone()) HArgumentsLength(elements));
+  HInstruction* checked_index =
+      AddInstruction(new(zone()) HBoundsCheck(index, length));
   HAccessArgumentsAt* result =
-      new(zone()) HAccessArgumentsAt(elements, length, index);
+      new(zone()) HAccessArgumentsAt(elements, length, checked_index);
   return ast_context()->ReturnInstruction(result, call->id());
 }

=======================================
--- /branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc Fri Oct 12 04:09:14 2012 +++ /branches/bleeding_edge/src/ia32/lithium-codegen-ia32.cc Mon Oct 15 00:25:20 2012
@@ -2742,12 +2742,9 @@
   Register length = ToRegister(instr->length());
   Operand index = ToOperand(instr->index());
   Register result = ToRegister(instr->result());
-
-  __ sub(length, index);
-  DeoptimizeIf(below_equal, instr->environment());
-
   // There are two words between the frame pointer and the last argument.
   // Subtracting from length accounts for one of them add one more.
+  __ sub(length, index);
   __ mov(result, Operand(arguments, length, times_4, kPointerSize));
 }

=======================================
--- /branches/bleeding_edge/src/ia32/lithium-ia32.cc Wed Sep 19 01:13:46 2012 +++ /branches/bleeding_edge/src/ia32/lithium-ia32.cc Mon Oct 15 00:25:20 2012
@@ -2279,12 +2279,10 @@


LInstruction* LChunkBuilder::DoAccessArgumentsAt(HAccessArgumentsAt* instr) {
-  LOperand* arguments = UseRegister(instr->arguments());
+  LOperand* args = UseRegister(instr->arguments());
   LOperand* length = UseTempRegister(instr->length());
   LOperand* index = Use(instr->index());
-  LAccessArgumentsAt* result =
-      new(zone()) LAccessArgumentsAt(arguments, length, index);
-  return AssignEnvironment(DefineAsRegister(result));
+ return DefineAsRegister(new(zone()) LAccessArgumentsAt(args, length, index));
 }


=======================================
--- /branches/bleeding_edge/src/mips/lithium-codegen-mips.cc Fri Oct 12 04:09:14 2012 +++ /branches/bleeding_edge/src/mips/lithium-codegen-mips.cc Mon Oct 15 00:25:20 2012
@@ -2612,14 +2612,6 @@
   Register length = ToRegister(instr->length());
   Register index = ToRegister(instr->index());
   Register result = ToRegister(instr->result());
-
-  // Bailout index is not a valid argument index. Use unsigned check to get
-  // negative check for free.
-
- // TODO(plind): Shoud be optimized to do the sub before the DeoptimizeIf(),
-  // as they do in Arm. It will save us an instruction.
-  DeoptimizeIf(ls, instr->environment(), length, Operand(index));
-
   // There are two words between the frame pointer and the last argument.
   // Subtracting from length accounts for one of them, add one more.
   __ subu(length, length, index);
=======================================
--- /branches/bleeding_edge/src/mips/lithium-mips.cc Fri Oct 12 00:05:00 2012 +++ /branches/bleeding_edge/src/mips/lithium-mips.cc Mon Oct 15 00:25:20 2012
@@ -2106,12 +2106,10 @@


LInstruction* LChunkBuilder::DoAccessArgumentsAt(HAccessArgumentsAt* instr) {
-  LOperand* arguments = UseRegister(instr->arguments());
+  LOperand* args = UseRegister(instr->arguments());
   LOperand* length = UseTempRegister(instr->length());
   LOperand* index = UseRegister(instr->index());
-  LAccessArgumentsAt* result =
-      new(zone()) LAccessArgumentsAt(arguments, length, index);
-  return AssignEnvironment(DefineAsRegister(result));
+ return DefineAsRegister(new(zone()) LAccessArgumentsAt(args, length, index));
 }


=======================================
--- /branches/bleeding_edge/src/x64/lithium-codegen-x64.cc Fri Oct 12 04:09:14 2012 +++ /branches/bleeding_edge/src/x64/lithium-codegen-x64.cc Mon Oct 15 00:25:20 2012
@@ -2601,16 +2601,13 @@
   Register arguments = ToRegister(instr->arguments());
   Register length = ToRegister(instr->length());
   Register result = ToRegister(instr->result());
-
+  // There are two words between the frame pointer and the last argument.
+  // Subtracting from length accounts for one of them add one more.
   if (instr->index()->IsRegister()) {
     __ subl(length, ToRegister(instr->index()));
   } else {
     __ subl(length, ToOperand(instr->index()));
   }
-  DeoptimizeIf(below_equal, instr->environment());
-
-  // There are two words between the frame pointer and the last argument.
-  // Subtracting from length accounts for one of them add one more.
__ movq(result, Operand(arguments, length, times_pointer_size, kPointerSize));
 }

=======================================
--- /branches/bleeding_edge/src/x64/lithium-x64.cc      Wed Sep 19 01:13:46 2012
+++ /branches/bleeding_edge/src/x64/lithium-x64.cc      Mon Oct 15 00:25:20 2012
@@ -2167,12 +2167,10 @@


LInstruction* LChunkBuilder::DoAccessArgumentsAt(HAccessArgumentsAt* instr) {
-  LOperand* arguments = UseRegister(instr->arguments());
+  LOperand* args = UseRegister(instr->arguments());
   LOperand* length = UseTempRegister(instr->length());
   LOperand* index = Use(instr->index());
-  LAccessArgumentsAt* result =
-      new(zone()) LAccessArgumentsAt(arguments, length, index);
-  return AssignEnvironment(DefineAsRegister(result));
+ return DefineAsRegister(new(zone()) LAccessArgumentsAt(args, length, index));
 }


--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to