 Chromium Code Reviews
 Chromium Code Reviews Issue 20070005:
  Adding Smi support to Add, Sub, Mul, and Bitwise  (Closed) 
  Base URL: https://v8.googlecode.com/svn/branches/bleeding_edge
    
  
    Issue 20070005:
  Adding Smi support to Add, Sub, Mul, and Bitwise  (Closed) 
  Base URL: https://v8.googlecode.com/svn/branches/bleeding_edge| Index: src/hydrogen-instructions.h | 
| diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h | 
| index ad274657687f14f1def2209166fcbd317fc3294f..5637ceec42fd91c81679a941be402e166a36836c 100644 | 
| --- a/src/hydrogen-instructions.h | 
| +++ b/src/hydrogen-instructions.h | 
| @@ -295,9 +295,9 @@ class Range: public ZoneObject { | 
| 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_; | 
| @@ -806,6 +806,8 @@ class HValue: public ZoneObject { | 
| kIsArguments, | 
| kTruncatingToInt32, | 
| kAllUsesTruncatingToInt32, | 
| + kTruncatingToSmi, | 
| + kAllUsesTruncatingToSmi, | 
| // Set after an instruction is killed. | 
| kIsDead, | 
| // Instructions that are allowed to produce full range unsigned integer | 
| @@ -892,6 +894,7 @@ class HValue: public ZoneObject { | 
| HUseIterator uses() const { return HUseIterator(use_list_); } | 
| virtual bool EmitAtUses() { return false; } | 
| + | 
| Representation representation() const { return representation_; } | 
| void ChangeRepresentation(Representation r) { | 
| ASSERT(CheckFlag(kFlexibleRepresentation)); | 
| @@ -1167,6 +1170,7 @@ class HValue: public ZoneObject { | 
| } | 
| Representation RepresentationFromUses(); | 
| Representation RepresentationFromUseRequirements(); | 
| + bool HasNonSmiUse(); | 
| virtual void UpdateRepresentation(Representation new_rep, | 
| HInferRepresentationPhase* h_infer, | 
| const char* reason); | 
| @@ -1715,7 +1719,8 @@ 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()); | 
| @@ -1724,7 +1729,8 @@ class HChange: public HUnaryOperation { | 
| 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 { | 
| @@ -2625,6 +2631,7 @@ class HUnaryMathOperation: public HTemplateInstruction<2> { | 
| switch (op) { | 
| case kMathFloor: | 
| case kMathRound: | 
| + // TODO(verwaest): Set representation to flexible int starting as smi. | 
| set_representation(Representation::Integer32()); | 
| break; | 
| case kMathAbs: | 
| @@ -3444,13 +3451,13 @@ class HBinaryOperation: public HTemplateInstruction<3> { | 
| // Constant operands are better off on the right, they can be inlined in | 
| // many situations on most platforms. | 
| - if (left()->IsConstant()) return true; | 
| if (right()->IsConstant()) return false; | 
| 
Sven Panne
2013/07/24 11:38:36
Let's hope that this change doesn't have subtle pe
 
Toon Verwaest
2013/07/24 13:24:01
Restored to its original form.
On 2013/07/24 11:3
 | 
| + if (left()->IsConstant()) return true; | 
| // 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() { | 
| @@ -3475,24 +3482,29 @@ class HBinaryOperation: public HTemplateInstruction<3> { | 
| return observed_input_representation_[index - 1]; | 
| } | 
| - virtual void InferRepresentation(HInferRepresentationPhase* h_infer); | 
| - virtual Representation RepresentationFromInputs(); | 
| - 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(); | 
| + if (!FLAG_smi_binop) { | 
| 
Sven Panne
2013/07/24 11:38:36
Nit: I think that
   if (new_rep.IsSmi() && !FLAG
 
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 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: | 
| @@ -3771,15 +3783,9 @@ class HBitwiseBinaryOperation: public HBinaryOperation { | 
| 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 { | 
| @@ -3792,10 +3798,14 @@ class HBitwiseBinaryOperation: public HBinaryOperation { | 
| 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) { | 
| @@ -3861,11 +3871,6 @@ class HArithmeticBinaryOperation: public HBinaryOperation { | 
| } | 
| virtual HType CalculateInferredType(); | 
| - virtual Representation RequiredInputRepresentation(int index) { | 
| - return index == 0 | 
| - ? Representation::Tagged() | 
| - : representation(); | 
| - } | 
| DECLARE_ABSTRACT_INSTRUCTION(ArithmeticBinaryOperation) | 
| @@ -4425,6 +4430,13 @@ class HMul: public HArithmeticBinaryOperation { | 
| 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: | 
| @@ -4464,6 +4476,13 @@ class HMod: public HArithmeticBinaryOperation { | 
| 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: | 
| @@ -4506,6 +4525,13 @@ class HDiv: public HArithmeticBinaryOperation { | 
| 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: | 
| @@ -4546,11 +4572,11 @@ class HMathMinMax: public HArithmeticBinaryOperation { | 
| 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; } | 
| @@ -4605,6 +4631,11 @@ class HBitwise: public HBitwiseBinaryOperation { | 
| 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?
 
Toon Verwaest
2013/07/24 13:24:01
Adjusted to >= 0 for bitand, and added a case for
 | 
| + } | 
| } | 
| Token::Value op_; | 
| @@ -4620,6 +4651,13 @@ class HShl: public HBitwiseBinaryOperation { | 
| 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: | 
| @@ -4652,6 +4690,13 @@ class HShr: public HBitwiseBinaryOperation { | 
| 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: | 
| @@ -4684,6 +4729,13 @@ class HSar: public HBitwiseBinaryOperation { | 
| 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: | 
| @@ -4702,6 +4754,13 @@ class HRor: public HBitwiseBinaryOperation { | 
| 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: | 
| @@ -5922,6 +5981,9 @@ class HStoreKeyed | 
| // 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 u
 
Toon Verwaest
2013/07/24 13:24:01
Removed.
On 2013/07/24 11:38:36, Sven Panne wrote
 | 
| + SetFlag(kTruncatingToSmi); | 
| + } | 
| SetFlag(kTruncatingToInt32); | 
| } | 
| } |