Separated smi check from HBoundsCheck.

src/hydrogen-instructions.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 797 matching lines @@
 
 void HBinaryCall::PrintDataTo(StringStream* stream) {
   first()->PrintNameTo(stream);
   stream->Add(" ");
   second()->PrintNameTo(stream);
   stream->Add(" ");
   stream->Add("#%d", argument_count());
 }
 
 
+HBoundsCheck* HBoundsCheck::AddAfter(

[Jakob Kummerow, 2013/02/06 13:43:59]

I don't see this used anywhere.

[Massi, 2013/02/06 19:12:31]

Removed.

+    HInstruction* insertion_point, HValue* index, HValue* length,
+    BoundsCheckKeyMode key_mode, Representation r) {
+  Zone* zone = insertion_point->block()->graph()->zone();
+  HDeoptimizeIfTaggedIsNotSmi* checked_index =
+      new(zone) HDeoptimizeIfTaggedIsNotSmi(index, r);
+  checked_index->InsertAfter(insertion_point);
+  HDeoptimizeIfTaggedIsNotSmi* checked_length =
+      new(zone) HDeoptimizeIfTaggedIsNotSmi(length, r);
+  checked_length->InsertAfter(checked_index);
+  HBoundsCheck* result = new(zone) HBoundsCheck(
+        checked_index, checked_length, key_mode, r);
+  result->InsertAfter(checked_length);
+  return result;
+}
+
+
+HBoundsCheck* HBoundsCheck::AddToGraph(
+    HGraphBuilder* graph_builder, HValue* index, HValue* length,
+    BoundsCheckKeyMode key_mode, Representation r) {
+  Zone* zone = graph_builder->graph()->zone();
+  HDeoptimizeIfTaggedIsNotSmi* checked_index =
+      new(zone) HDeoptimizeIfTaggedIsNotSmi(index, r);
+  graph_builder->AddInstruction(checked_index);
+  HDeoptimizeIfTaggedIsNotSmi* checked_length =

[Jakob Kummerow, 2013/02/06 13:43:59]

Didn't we agree that HBoundsCheck is only ever used for arrays with FAST
elements, and those always have a Smi as their length? So I don't think we need
this check.

[Massi, 2013/02/06 19:12:31]

Done.

+      new(zone) HDeoptimizeIfTaggedIsNotSmi(length, r);
+  graph_builder->AddInstruction(checked_length);
+  HBoundsCheck* result = new(zone) HBoundsCheck(
+        checked_index, checked_length, key_mode, r);
+  graph_builder->AddInstruction(result);
+  return result;
+}
+
+
 void HBoundsCheck::PrintDataTo(StringStream* stream) {
   index()->PrintNameTo(stream);
   stream->Add(" ");
   length()->PrintNameTo(stream);
 }
 
 
 void HBoundsCheck::InferRepresentation(HInferRepresentation* h_infer) {
   ASSERT(CheckFlag(kFlexibleRepresentation));
   Representation r;
   if (key_mode_ == DONT_ALLOW_SMI_KEY ||
       !length()->representation().IsTagged()) {
     r = Representation::Integer32();
   } else if (index()->representation().IsTagged() ||
-      (index()->IsConstant() &&
-       HConstant::cast(index())->HasInteger32Value() &&
-       Smi::IsValid(HConstant::cast(index())->Integer32Value()))) {
+      (index()->ActualValue()->IsConstant() &&
+       HConstant::cast(index()->ActualValue())->HasSmiValue())) {
     // If the index is tagged, or a constant that holds a Smi, allow the length
     // to be tagged, since it is usually already tagged from loading it out of
     // the length field of a JSArray. This allows for direct comparison without
     // untagging.
     r = Representation::Tagged();
   } else {
     r = Representation::Integer32();
   }
   UpdateRepresentation(r, h_infer, "boundscheck");
 }
@@ skipping 1540 matching lines @@
   // TODO(kasperl): Is there any way to signal that this isn't a smi?
   return HType::Tagged();
 }
 
 
 HType HCheckSmi::CalculateInferredType() {
   return HType::Smi();
 }
 
 
+void HDeoptimizeIfTaggedIsNotSmi::InferRepresentation(
+    HInferRepresentation* h_infer) {
+  ASSERT(CheckFlag(kFlexibleRepresentation));
+  Representation r = value()->representation().IsTagged()
+      ? Representation::Tagged() : Representation::Integer32();
+  UpdateRepresentation(r, h_infer, "checksmiorint32");
+}
+
+
 HType HPhi::CalculateInferredType() {
   HType result = HType::Uninitialized();
   for (int i = 0; i < OperandCount(); ++i) {
     HType current = OperandAt(i)->type();
     result = result.Combine(current);
   }
   return result;
 }
 
 
@@ skipping 515 matching lines @@
   ASSERT(HasAstId());
 }
 
 
 void HCheckSmi::Verify() {
   HInstruction::Verify();
   ASSERT(HasNoUses());
 }
 
 
+void HDeoptimizeIfTaggedIsNotSmi::Verify() {
+  HInstruction::Verify();

[Jakob Kummerow, 2013/02/06 13:43:59]

This is the default implementation, no need to override it.

[Massi, 2013/02/06 19:12:31]

The linker complains if I don't override it (and implement it).

[Jakob Kummerow, 2013/02/07 16:17:14]

Not for me. Maybe you've forgotten to remove the declaration from the class
definition in hydrogen-instructions.h?

[Massi, 2013/02/11 10:35:55]

I was sure I had deleted, but I did not :-/
Done :-)

+}
+
+
 void HCheckNonSmi::Verify() {
   HInstruction::Verify();
   ASSERT(HasNoUses());
 }
 
 
 void HCheckFunction::Verify() {
   HInstruction::Verify();
   ASSERT(HasNoUses());
 }
 
 #endif
 
 } }  // namespace v8::internal