Adding Smi support to Add, Sub, Mul, and Bitwise

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 277 matching lines @@
   }
 
   void Intersect(Range* other);
   void Union(Range* other);
   void CombinedMax(Range* other);
   void CombinedMin(Range* other);
 
   void AddConstant(int32_t value);
   void Sar(int32_t value);
   void Shl(int32_t value);
-  bool AddAndCheckOverflow(Range* other);
-  bool SubAndCheckOverflow(Range* other);
-  bool MulAndCheckOverflow(Range* other);
+  bool AddAndCheckOverflow(const Representation& r, Range* other);
+  bool SubAndCheckOverflow(const Representation& r, Range* other);
+  bool MulAndCheckOverflow(const Representation& r, Range* other);
 
  private:
   int32_t lower_;
   int32_t upper_;
   Range* next_;
   bool can_be_minus_zero_;
 };
 
 
 class UniqueValueId {
@@ skipping 488 matching lines @@
     // sets this flag, it must implement HandleSideEffectDominator() and should
     // indicate which side effects to track by setting GVN flags.
     kTrackSideEffectDominators,
     kCanOverflow,
     kBailoutOnMinusZero,
     kCanBeDivByZero,
     kAllowUndefinedAsNaN,
     kIsArguments,
     kTruncatingToInt32,
     kAllUsesTruncatingToInt32,
+    kTruncatingToSmi,
+    kAllUsesTruncatingToSmi,
     // Set after an instruction is killed.
     kIsDead,
     // Instructions that are allowed to produce full range unsigned integer
     // values are marked with kUint32 flag. If arithmetic shift or a load from
     // EXTERNAL_UNSIGNED_INT_ELEMENTS array is not marked with this flag
     // it will deoptimize if result does not fit into signed integer range.
     // HGraph::ComputeSafeUint32Operations is responsible for setting this
     // flag.
     kUint32,
     // If a phi is involved in the evaluation of a numeric constraint the
@@ skipping 66 matching lines @@
 
   // Note: Never call this method for an unlinked value.
   Isolate* isolate() const;
 
   int id() const { return id_; }
   void set_id(int id) { id_ = id; }
 
   HUseIterator uses() const { return HUseIterator(use_list_); }
 
   virtual bool EmitAtUses() { return false; }
+
   Representation representation() const { return representation_; }
   void ChangeRepresentation(Representation r) {
     ASSERT(CheckFlag(kFlexibleRepresentation));
     ASSERT(!CheckFlag(kCannotBeTagged) || !r.IsTagged());
     RepresentationChanged(r);
     representation_ = r;
     if (r.IsTagged()) {
       // Tagged is the bottom of the lattice, don't go any further.
       ClearFlag(kFlexibleRepresentation);
     }
@@ skipping 255 matching lines @@
   virtual bool DataEquals(HValue* other) {
     UNREACHABLE();
     return false;
   }
 
   virtual Representation RepresentationFromInputs() {
     return representation();
   }
   Representation RepresentationFromUses();
   Representation RepresentationFromUseRequirements();
+  bool HasNonSmiUse();
   virtual void UpdateRepresentation(Representation new_rep,
                                     HInferRepresentationPhase* h_infer,
                                     const char* reason);
   void AddDependantsToWorklist(HInferRepresentationPhase* h_infer);
 
   virtual void RepresentationChanged(Representation to) { }
 
   virtual Range* InferRange(Zone* zone);
   virtual void DeleteFromGraph() = 0;
   virtual void InternalSetOperandAt(int index, HValue* value) = 0;
@@ skipping 528 matching lines @@
   virtual void PrintDataTo(StringStream* stream);
 
   DECLARE_CONCRETE_INSTRUCTION(ForceRepresentation)
 };
 
 
 class HChange: public HUnaryOperation {
  public:
   HChange(HValue* value,
           Representation to,
-          bool is_truncating,
+          bool is_truncating_to_smi,
+          bool is_truncating_to_int32,
           bool allow_undefined_as_nan)
       : HUnaryOperation(value) {
     ASSERT(!value->representation().IsNone());
     ASSERT(!to.IsNone());
     ASSERT(!value->representation().Equals(to));
     set_representation(to);
     SetFlag(kUseGVN);
     if (allow_undefined_as_nan) SetFlag(kAllowUndefinedAsNaN);
-    if (is_truncating) SetFlag(kTruncatingToInt32);
+    if (is_truncating_to_smi) SetFlag(kTruncatingToSmi);
+    if (is_truncating_to_int32) SetFlag(kTruncatingToInt32);
     if (value->representation().IsSmi() || value->type().IsSmi()) {
       set_type(HType::Smi());
     } else {
       set_type(HType::TaggedNumber());
       if (to.IsTagged()) SetGVNFlag(kChangesNewSpacePromotion);
     }
   }
 
   virtual HValue* EnsureAndPropagateNotMinusZero(BitVector* visited);
   virtual HType CalculateInferredType();
@@ skipping 880 matching lines @@
   }
 
  private:
   HUnaryMathOperation(HValue* context, HValue* value, BuiltinFunctionId op)
       : op_(op) {
     SetOperandAt(0, context);
     SetOperandAt(1, value);
     switch (op) {
       case kMathFloor:
       case kMathRound:
+        // TODO(verwaest): Set representation to flexible int starting as smi.
         set_representation(Representation::Integer32());
         break;
       case kMathAbs:
         // Not setting representation here: it is None intentionally.
         SetFlag(kFlexibleRepresentation);
         // TODO(svenpanne) This flag is actually only needed if representation()
         // is tagged, and not when it is an unboxed double or unboxed integer.
         SetGVNFlag(kChangesNewSpacePromotion);
         break;
       case kMathLog:
@@ skipping 799 matching lines @@
   HValue* context() { return OperandAt(0); }
   HValue* left() { return OperandAt(1); }
   HValue* right() { return OperandAt(2); }
 
   // True if switching left and right operands likely generates better code.
   bool AreOperandsBetterSwitched() {
     if (!IsCommutative()) return false;
 
     // Constant operands are better off on the right, they can be inlined in
     // many situations on most platforms.
+    if (right()->IsConstant()) return false;

[Sven Panne, 2013/07/24 11:38:36]

Let's hope that this change doesn't have subtle performance implications...

[Toon Verwaest, 2013/07/24 13:24:01]

Restored to its original form.

On 2013/07/24 11:38:36, Sven Panne wrote:

     if (left()->IsConstant()) return true;
-    if (right()->IsConstant()) return false;
 
     // Otherwise, if there is only one use of the right operand, it would be
     // better off on the left for platforms that only have 2-arg arithmetic
     // ops (e.g ia32, x64) that clobber the left operand.
-    return (right()->UseCount() == 1);
+    return right()->UseCount() == 1;
   }
 
   HValue* BetterLeftOperand() {
     return AreOperandsBetterSwitched() ? right() : left();
   }
 
   HValue* BetterRightOperand() {
     return AreOperandsBetterSwitched() ? left() : right();
   }
 
   void set_observed_input_representation(int index, Representation rep) {
     ASSERT(index >= 1 && index <= 2);
     observed_input_representation_[index - 1] = rep;
   }
 
   virtual void initialize_output_representation(Representation observed) {
     observed_output_representation_ = observed;
   }
 
   virtual Representation observed_input_representation(int index) {
     if (index == 0) return Representation::Tagged();
     return observed_input_representation_[index - 1];
   }
 
+  virtual void UpdateRepresentation(Representation new_rep,
+                                    HInferRepresentationPhase* h_infer,
+                                    const char* reason) {
+    if (!FLAG_smi_binop) {

[Sven Panne, 2013/07/24 11:38:36]

Nit: I think that

   if (new_rep.IsSmi() && !FLAG_smi_binop) ...

is more readable.

Using a ternary instead of an assignment to a parameter would be nice, too, but
this is a personal preference.

[Toon Verwaest, 2013/07/24 13:24:01]

Done.

+      if (new_rep.IsSmi()) new_rep = Representation::Integer32();
+    }
+    HValue::UpdateRepresentation(new_rep, h_infer, reason);
+  }
+
   virtual void InferRepresentation(HInferRepresentationPhase* h_infer);
   virtual Representation RepresentationFromInputs();
+  Representation RepresentationFromOutput();
   virtual void AssumeRepresentation(Representation r);
 
-  virtual void UpdateRepresentation(Representation new_rep,
-                                    HInferRepresentationPhase* h_infer,
-                                    const char* reason) {
-    // By default, binary operations don't handle Smis.
-    if (new_rep.IsSmi()) {
-      new_rep = Representation::Integer32();
-    }
-    HValue::UpdateRepresentation(new_rep, h_infer, reason);
-  }
-
   virtual bool IsCommutative() const { return false; }
 
   virtual void PrintDataTo(StringStream* stream);
 
+  virtual Representation RequiredInputRepresentation(int index) {
+    if (index == 0) return Representation::Tagged();
+    return representation();
+  }
+
   DECLARE_ABSTRACT_INSTRUCTION(BinaryOperation)
 
  private:
   bool IgnoreObservedOutputRepresentation(Representation current_rep);
 
   Representation observed_input_representation_[2];
   Representation observed_output_representation_;
 };
 
 
@@ skipping 258 matching lines @@
 class HBitwiseBinaryOperation: public HBinaryOperation {
  public:
   HBitwiseBinaryOperation(HValue* context, HValue* left, HValue* right)
       : HBinaryOperation(context, left, right) {
     SetFlag(kFlexibleRepresentation);
     SetFlag(kTruncatingToInt32);
     SetFlag(kAllowUndefinedAsNaN);
     SetAllSideEffects();
   }
 
-  virtual Representation RequiredInputRepresentation(int index) {
-    return index == 0
-        ? Representation::Tagged()
-        : representation();
-  }
-
   virtual void RepresentationChanged(Representation to) {
     if (!to.IsTagged()) {
-      ASSERT(to.IsInteger32());
+      ASSERT(to.IsSmiOrInteger32());
       ClearAllSideEffects();
       SetFlag(kUseGVN);
     } else {
       SetAllSideEffects();
       ClearFlag(kUseGVN);
     }
   }
 
   virtual void UpdateRepresentation(Representation new_rep,
                                     HInferRepresentationPhase* h_infer,
                                     const char* reason) {
     // We only generate either int32 or generic tagged bitwise operations.
-    if (new_rep.IsSmi() || new_rep.IsDouble()) {
-      new_rep = Representation::Integer32();
-    }
-    HValue::UpdateRepresentation(new_rep, h_infer, reason);
+    if (new_rep.IsDouble()) new_rep = Representation::Integer32();
+    HBinaryOperation::UpdateRepresentation(new_rep, h_infer, reason);
+  }
+
+  virtual Representation observed_input_representation(int index) {
+    Representation r = HBinaryOperation::observed_input_representation(index);
+    if (r.IsDouble()) return Representation::Integer32();
+    return r;
   }
 
   virtual void initialize_output_representation(Representation observed) {
     if (observed.IsDouble()) observed = Representation::Integer32();
     HBinaryOperation::initialize_output_representation(observed);
   }
 
   virtual HType CalculateInferredType();
 
   DECLARE_ABSTRACT_INSTRUCTION(BitwiseBinaryOperation)
@@ skipping 45 matching lines @@
     if (to.IsTagged()) {
       SetAllSideEffects();
       ClearFlag(kUseGVN);
     } else {
       ClearAllSideEffects();
       SetFlag(kUseGVN);
     }
   }
 
   virtual HType CalculateInferredType();
-  virtual Representation RequiredInputRepresentation(int index) {
-    return index == 0
-        ? Representation::Tagged()
-        : representation();
-  }
 
   DECLARE_ABSTRACT_INSTRUCTION(ArithmeticBinaryOperation)
 
  private:
   virtual bool IsDeletable() const { return true; }
 };
 
 
 class HCompareGeneric: public HBinaryOperation {
  public:
@@ skipping 539 matching lines @@
 
   virtual HValue* EnsureAndPropagateNotMinusZero(BitVector* visited);
 
   virtual HValue* Canonicalize();
 
   // Only commutative if it is certain that not two objects are multiplicated.
   virtual bool IsCommutative() const {
     return !representation().IsTagged();
   }
 
+  virtual void UpdateRepresentation(Representation new_rep,
+                                    HInferRepresentationPhase* h_infer,
+                                    const char* reason) {
+    if (new_rep.IsSmi()) new_rep = Representation::Integer32();
+    HArithmeticBinaryOperation::UpdateRepresentation(new_rep, h_infer, reason);
+  }
+
   DECLARE_CONCRETE_INSTRUCTION(Mul)
 
  protected:
   virtual bool DataEquals(HValue* other) { return true; }
 
   virtual Range* InferRange(Zone* zone);
 
  private:
   HMul(HValue* context, HValue* left, HValue* right)
       : HArithmeticBinaryOperation(context, left, right) {
@@ skipping 19 matching lines @@
       return value != 0 && (IsPowerOf2(value) || IsPowerOf2(-value));
     }
 
     return false;
   }
 
   virtual HValue* EnsureAndPropagateNotMinusZero(BitVector* visited);
 
   virtual HValue* Canonicalize();
 
+  virtual void UpdateRepresentation(Representation new_rep,
+                                    HInferRepresentationPhase* h_infer,
+                                    const char* reason) {
+    if (new_rep.IsSmi()) new_rep = Representation::Integer32();
+    HArithmeticBinaryOperation::UpdateRepresentation(new_rep, h_infer, reason);
+  }
+
   DECLARE_CONCRETE_INSTRUCTION(Mod)
 
  protected:
   virtual bool DataEquals(HValue* other) { return true; }
 
   virtual Range* InferRange(Zone* zone);
 
  private:
   HMod(HValue* context,
        HValue* left,
@@ skipping 22 matching lines @@
       return value != 0 && (IsPowerOf2(value) || IsPowerOf2(-value));
     }
 
     return false;
   }
 
   virtual HValue* EnsureAndPropagateNotMinusZero(BitVector* visited);
 
   virtual HValue* Canonicalize();
 
+  virtual void UpdateRepresentation(Representation new_rep,
+                                    HInferRepresentationPhase* h_infer,
+                                    const char* reason) {
+    if (new_rep.IsSmi()) new_rep = Representation::Integer32();
+    HArithmeticBinaryOperation::UpdateRepresentation(new_rep, h_infer, reason);
+  }
+
   DECLARE_CONCRETE_INSTRUCTION(Div)
 
  protected:
   virtual bool DataEquals(HValue* other) { return true; }
 
   virtual Range* InferRange(Zone* zone);
 
  private:
   HDiv(HValue* context, HValue* left, HValue* right)
       : HArithmeticBinaryOperation(context, left, right) {
@@ skipping 20 matching lines @@
 
   virtual Representation observed_input_representation(int index) {
     return RequiredInputRepresentation(index);
   }
 
   virtual void InferRepresentation(HInferRepresentationPhase* h_infer);
 
   virtual Representation RepresentationFromInputs() {
     Representation left_rep = left()->representation();
     Representation right_rep = right()->representation();
-    if ((left_rep.IsNone() || left_rep.IsInteger32()) &&
-        (right_rep.IsNone() || right_rep.IsInteger32())) {
-      return Representation::Integer32();
-    }
-    return Representation::Double();
+    Representation result = Representation::Smi();
+    result = result.generalize(left_rep);
+    result = result.generalize(right_rep);
+    if (result.IsTagged()) return Representation::Double();
+    return result;
   }
 
   virtual bool IsCommutative() const { return true; }
 
   Operation operation() { return operation_; }
 
   DECLARE_CONCRETE_INSTRUCTION(MathMinMax)
 
  protected:
   virtual bool DataEquals(HValue* other) {
@@ skipping 34 matching lines @@
   virtual bool DataEquals(HValue* other) {
     return op() == HBitwise::cast(other)->op();
   }
 
   virtual Range* InferRange(Zone* zone);
 
  private:
   HBitwise(Token::Value op, HValue* context, HValue* left, HValue* right)
       : HBitwiseBinaryOperation(context, left, right), op_(op) {
     ASSERT(op == Token::BIT_AND || op == Token::BIT_OR || op == Token::BIT_XOR);
+    if (op == Token::BIT_AND &&
+        ((left->IsConstant() && left->representation().IsSmi()) ||
+         (right->IsConstant() && right->representation().IsSmi()))) {
+      SetFlag(kTruncatingToSmi);

[Sven Panne, 2013/07/24 11:38:36]

I don't understand this part. Why is this correct? At least a comment is needed.

[Toon Verwaest, 2013/07/24 13:24:01]

Adjusted to >= 0 for bitand, and added a case for <0 for bitor; and added
comment.

On 2013/07/24 11:38:36, Sven Panne wrote:

+    }
   }
 
   Token::Value op_;
 };
 
 
 class HShl: public HBitwiseBinaryOperation {
  public:
   static HInstruction* New(Zone* zone,
                            HValue* context,
                            HValue* left,
                            HValue* right);
 
   virtual Range* InferRange(Zone* zone);
 
+  virtual void UpdateRepresentation(Representation new_rep,
+                                    HInferRepresentationPhase* h_infer,
+                                    const char* reason) {
+    if (new_rep.IsSmi()) new_rep = Representation::Integer32();
+    HBitwiseBinaryOperation::UpdateRepresentation(new_rep, h_infer, reason);
+  }
+
   DECLARE_CONCRETE_INSTRUCTION(Shl)
 
  protected:
   virtual bool DataEquals(HValue* other) { return true; }
 
  private:
   HShl(HValue* context, HValue* left, HValue* right)
       : HBitwiseBinaryOperation(context, left, right) { }
 };
 
@@ skipping 12 matching lines @@
         // like ((base + offset) >> scale) with one single decomposition.
         left()->TryDecompose(decomposition);
         return true;
       }
     }
     return false;
   }
 
   virtual Range* InferRange(Zone* zone);
 
+  virtual void UpdateRepresentation(Representation new_rep,
+                                    HInferRepresentationPhase* h_infer,
+                                    const char* reason) {
+    if (new_rep.IsSmi()) new_rep = Representation::Integer32();
+    HBitwiseBinaryOperation::UpdateRepresentation(new_rep, h_infer, reason);
+  }
+
   DECLARE_CONCRETE_INSTRUCTION(Shr)
 
  protected:
   virtual bool DataEquals(HValue* other) { return true; }
 
  private:
   HShr(HValue* context, HValue* left, HValue* right)
       : HBitwiseBinaryOperation(context, left, right) { }
 };
 
@@ skipping 12 matching lines @@
         // like ((base + offset) >> scale) with one single decomposition.
         left()->TryDecompose(decomposition);
         return true;
       }
     }
     return false;
   }
 
   virtual Range* InferRange(Zone* zone);
 
+  virtual void UpdateRepresentation(Representation new_rep,
+                                    HInferRepresentationPhase* h_infer,
+                                    const char* reason) {
+    if (new_rep.IsSmi()) new_rep = Representation::Integer32();
+    HBitwiseBinaryOperation::UpdateRepresentation(new_rep, h_infer, reason);
+  }
+
   DECLARE_CONCRETE_INSTRUCTION(Sar)
 
  protected:
   virtual bool DataEquals(HValue* other) { return true; }
 
  private:
   HSar(HValue* context, HValue* left, HValue* right)
       : HBitwiseBinaryOperation(context, left, right) { }
 };
 
 
 class HRor: public HBitwiseBinaryOperation {
  public:
   HRor(HValue* context, HValue* left, HValue* right)
        : HBitwiseBinaryOperation(context, left, right) {
     ChangeRepresentation(Representation::Integer32());
   }
 
+  virtual void UpdateRepresentation(Representation new_rep,
+                                    HInferRepresentationPhase* h_infer,
+                                    const char* reason) {
+    if (new_rep.IsSmi()) new_rep = Representation::Integer32();
+    HBitwiseBinaryOperation::UpdateRepresentation(new_rep, h_infer, reason);
+  }
+
   DECLARE_CONCRETE_INSTRUCTION(Ror)
 
  protected:
   virtual bool DataEquals(HValue* other) { return true; }
 };
 
 
 class HOsrEntry: public HTemplateInstruction<0> {
  public:
   explicit HOsrEntry(BailoutId ast_id) : ast_id_(ast_id) {
@@ skipping 1200 matching lines @@
       SetGVNFlag(kChangesDoubleArrayElements);
     } else if (IsFastSmiElementsKind(elements_kind)) {
       SetGVNFlag(kChangesArrayElements);
     } else {
       SetGVNFlag(kChangesArrayElements);
     }
 
     // EXTERNAL_{UNSIGNED_,}{BYTE,SHORT,INT}_ELEMENTS are truncating.
     if (elements_kind >= EXTERNAL_BYTE_ELEMENTS &&
         elements_kind <= EXTERNAL_UNSIGNED_INT_ELEMENTS) {
+      if (elements_kind <= EXTERNAL_SHORT_ELEMENTS) {

[Sven Panne, 2013/07/24 11:38:36]

Stuff like this has been there before, but it is ugly as hell: This can't be
understood without seeing the ElementsKind definition (and this even lacks a
comment that the order of enums carries some semantics... :-P

[Toon Verwaest, 2013/07/24 13:24:01]

Removed.

On 2013/07/24 11:38:36, Sven Panne wrote:

+        SetFlag(kTruncatingToSmi);
+      }
       SetFlag(kTruncatingToInt32);
     }
   }
 
   virtual bool HasEscapingOperandAt(int index) { return index != 0; }
   virtual Representation RequiredInputRepresentation(int index) {
     // kind_fast:       tagged[int32] = tagged
     // kind_double:     tagged[int32] = double
     // kind_smi   :     tagged[int32] = smi
     // kind_external: external[int32] = (double | int32)
@@ skipping 702 matching lines @@
   virtual bool IsDeletable() const { return true; }
 };
 
 
 #undef DECLARE_INSTRUCTION
 #undef DECLARE_CONCRETE_INSTRUCTION
 
 } }  // namespace v8::internal
 
 #endif  // V8_HYDROGEN_INSTRUCTIONS_H_