Separated smi check from HBoundsCheck.

src/hydrogen-instructions.h

 // 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 86 matching lines @@
   V(CompareIDAndBranch)                        \
   V(CompareGeneric)                            \
   V(CompareObjectEqAndBranch)                  \
   V(CompareMap)                                \
   V(CompareConstantEqAndBranch)                \
   V(Constant)                                  \
   V(Context)                                   \
   V(DeclareGlobals)                            \
   V(DeleteProperty)                            \
   V(Deoptimize)                                \
+  V(DeoptimizeIfTaggedIsNotSmi)                \
   V(Div)                                       \
   V(DummyUse)                                  \
   V(ElementsKind)                              \
   V(EnterInlined)                              \
   V(FastLiteral)                               \
   V(FixedArrayBaseLength)                      \
   V(ForceRepresentation)                       \
   V(FunctionLiteral)                           \
   V(GetCachedArrayIndex)                       \
   V(GlobalObject)                              \
@@ skipping 2488 matching lines @@
   virtual void Verify();
 #endif
 
   DECLARE_CONCRETE_INSTRUCTION(CheckSmi)
 
  protected:
   virtual bool DataEquals(HValue* other) { return true; }
 };
 
 
+class HDeoptimizeIfTaggedIsNotSmi: public HUnaryOperation {

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

For consistency with HCheckSmi, I'd call this HCheckSmiOrInt32.

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

Done.

+ public:
+  explicit HDeoptimizeIfTaggedIsNotSmi(

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

no "explicit" necessary (unless you remove the Representation argument, see
below).

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

Done.

+      HValue* value,
+      Representation r = Representation::None())
+      : HUnaryOperation(value) {
+    if (r.IsNone()) {
+      // In the normal compilation pipeline the representation is flexible
+      // (see InferRepresentation).
+      SetFlag(kFlexibleRepresentation);
+    } else {

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

I don't think we need this case. This instruction doesn't do anything when the
representation is Integer32, and in the code stubs case it is always Integer32.
So we can simply not insert it at all in that case.

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

Done.

+      // When compiling stubs we want to set the representation explicitly
+      // so the compilation pipeline can skip the HInferRepresentation phase.
+      set_representation(r);
+    }
+    SetFlag(kUseGVN);
+  }
+
+  virtual int RedefinedOperandIndex() { return 0; }
+  virtual Representation RequiredInputRepresentation(int index) {
+    return representation();
+  }
+  virtual void InferRepresentation(HInferRepresentation* h_infer);
+
+#ifdef DEBUG
+  virtual void Verify();
+#endif
+
+  DECLARE_CONCRETE_INSTRUCTION(DeoptimizeIfTaggedIsNotSmi)
+
+ protected:
+  virtual bool DataEquals(HValue* other) { return true; }
+};
+
+
 class HPhi: public HValue {
  public:
   HPhi(int merged_index, Zone* zone)
       : inputs_(2, zone),
         merged_index_(merged_index),
         phi_id_(-1),
         is_live_(false),
         is_convertible_to_integer_(true) {
     for (int i = 0; i < Representation::kNumRepresentations; i++) {
       non_phi_uses_[i] = 0;
@@ skipping 175 matching lines @@
   virtual void PrintDataTo(StringStream* stream);
   virtual HType CalculateInferredType();
   bool IsInteger() { return handle()->IsSmi(); }
   HConstant* CopyToRepresentation(Representation r, Zone* zone) const;
   HConstant* CopyToTruncatedInt32(Zone* zone) const;
   bool HasInteger32Value() const { return has_int32_value_; }
   int32_t Integer32Value() const {
     ASSERT(HasInteger32Value());
     return int32_value_;
   }
+  bool HasSmiValue() const {
+    return HasInteger32Value() && Smi::IsValid(Integer32Value());
+  }
   bool HasDoubleValue() const { return has_double_value_; }
   double DoubleValue() const {
     ASSERT(HasDoubleValue());
     return double_value_;
   }
   bool HasNumberValue() const { return has_double_value_; }
   int32_t NumberValueAsInteger32() const {
     ASSERT(HasNumberValue());
     // Irrespective of whether a numeric HConstant can be safely
     // represented as an int32, we store the (in some cases lossy)
@@ skipping 259 matching lines @@
   virtual bool DataEquals(HValue* other) { return true; }
 };
 
 
 enum BoundsCheckKeyMode {
   DONT_ALLOW_SMI_KEY,
   ALLOW_SMI_KEY
 };
 
 
+class HGraphBuilder;

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

Please don't do this. Instead, move the insertion helper function to the graph,
similar to HGraphBuilder::AddSimulate or
HOptimizedGraphBuilder::AddSoftDeoptimize(). (If we need the helper at all --
see my other comments.)

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

Done.

+
+
 class HBoundsCheck: public HTemplateInstruction<2> {
  public:
-  HBoundsCheck(HValue* index, HValue* length,
-               BoundsCheckKeyMode key_mode = DONT_ALLOW_SMI_KEY,
-               Representation r = Representation::None())
-      : key_mode_(key_mode) {
-    SetOperandAt(0, index);
-    SetOperandAt(1, length);
-    if (r.IsNone()) {
-      // In the normal compilation pipeline the representation is flexible
-      // (see comment to RequiredInputRepresentation).
-      SetFlag(kFlexibleRepresentation);
-    } else {
-      // When compiling stubs we want to set the representation explicitly
-      // so the compilation pipeline can skip the HInferRepresentation phase.
-      set_representation(r);
-    }
-    SetFlag(kUseGVN);
-  }
+  static HBoundsCheck* AddAfter(
+      HInstruction* insertion_point, HValue* index, HValue* length,
+      BoundsCheckKeyMode key_mode = DONT_ALLOW_SMI_KEY,
+      Representation r = Representation::None());
+  static HBoundsCheck* AddToGraph(
+      HGraphBuilder* graph_builder, HValue* index, HValue* length,
+      BoundsCheckKeyMode key_mode = DONT_ALLOW_SMI_KEY,
+      Representation r = Representation::None());
+  bool skip_check() { return skip_check_; }
+  void set_skip_check(bool skip_check) { skip_check_ = skip_check; }
 
   virtual Representation RequiredInputRepresentation(int arg_index) {
     return representation();
   }
   virtual Representation observed_input_representation(int index) {
     return Representation::Integer32();
   }
 
   virtual void PrintDataTo(StringStream* stream);
   virtual void InferRepresentation(HInferRepresentation* h_infer);
 
   HValue* index() { return OperandAt(0); }
   HValue* length() { return OperandAt(1); }
 
   virtual int RedefinedOperandIndex() { return 0; }
 
   DECLARE_CONCRETE_INSTRUCTION(BoundsCheck)
 
  protected:
+  HBoundsCheck(HValue* index, HValue* length,
+               BoundsCheckKeyMode key_mode = DONT_ALLOW_SMI_KEY,
+               Representation r = Representation::None())
+    : key_mode_(key_mode), skip_check_(false) {

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

nit: more indentation

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

Done.

+    SetOperandAt(0, index);
+    SetOperandAt(1, length);
+    if (r.IsNone()) {
+      // In the normal compilation pipeline the representation is flexible
+      // (see InferRepresentation).
+      SetFlag(kFlexibleRepresentation);
+    } else {
+      // When compiling stubs we want to set the representation explicitly
+      // so the compilation pipeline can skip the HInferRepresentation phase.
+      set_representation(r);
+    }
+    SetFlag(kUseGVN);
+  }
+
   virtual bool DataEquals(HValue* other) { return true; }
   BoundsCheckKeyMode key_mode_;
+  bool skip_check_;
 };
 
 
 class HBitwiseBinaryOperation: public HBinaryOperation {
  public:
   HBitwiseBinaryOperation(HValue* context, HValue* left, HValue* right)
       : HBinaryOperation(context, left, right) {
     SetFlag(kFlexibleRepresentation);
     SetFlag(kTruncatingToInt32);
     SetAllSideEffects();
@@ skipping 2604 matching lines @@
   virtual bool IsDeletable() const { return true; }
 };
 
 
 #undef DECLARE_INSTRUCTION
 #undef DECLARE_CONCRETE_INSTRUCTION
 
 } }  // namespace v8::internal
 
 #endif  // V8_HYDROGEN_INSTRUCTIONS_H_