OSDN Git Service

Fix and improve shift and rotate operations.
authorRoland Levillain <rpl@google.com>
Tue, 22 Mar 2016 14:57:31 +0000 (14:57 +0000)
committerRoland Levillain <rpl@google.com>
Tue, 22 Mar 2016 14:57:31 +0000 (14:57 +0000)
- Define maximum int and long shift & rotate distances as
  int32_t constants, as shift & rotate distances are 32-bit
  integer values.
- Consider the (long, long) inputs case as invalid for
  static evaluation of shift & rotate rotations.
- Add more checks in shift & rotate operations constructors
  as well as in art::GraphChecker.

Change-Id: I754b326c3a341c9cc567d1720b327dad6fcbf9d6

compiler/optimizing/code_generator_arm.cc
compiler/optimizing/code_generator_arm64.cc
compiler/optimizing/code_generator_mips.cc
compiler/optimizing/code_generator_mips64.cc
compiler/optimizing/code_generator_x86.cc
compiler/optimizing/code_generator_x86_64.cc
compiler/optimizing/graph_checker.cc
compiler/optimizing/instruction_simplifier.cc
compiler/optimizing/nodes.h

index 4c9c25f..54793fd 100644 (file)
@@ -3221,7 +3221,7 @@ void InstructionCodeGeneratorARM::HandleLongRotate(LocationSummary* locations) {
   if (rhs.IsConstant()) {
     uint64_t rot = CodeGenerator::GetInt64ValueOf(rhs.GetConstant());
     // Map all rotations to +ve. equivalents on the interval [0,63].
-    rot &= kMaxLongShiftValue;
+    rot &= kMaxLongShiftDistance;
     // For rotates over a word in size, 'pre-rotate' by 32-bits to keep rotate
     // logic below to a simple pair of binary orr.
     // (e.g. 34 bits == in_reg swap + 2 bits right.)
@@ -3374,7 +3374,7 @@ void InstructionCodeGeneratorARM::HandleShift(HBinaryOperation* op) {
       if (second.IsRegister()) {
         Register second_reg = second.AsRegister<Register>();
         // ARM doesn't mask the shift count so we need to do it ourselves.
-        __ and_(out_reg, second_reg, ShifterOperand(kMaxIntShiftValue));
+        __ and_(out_reg, second_reg, ShifterOperand(kMaxIntShiftDistance));
         if (op->IsShl()) {
           __ Lsl(out_reg, first_reg, out_reg);
         } else if (op->IsShr()) {
@@ -3384,7 +3384,7 @@ void InstructionCodeGeneratorARM::HandleShift(HBinaryOperation* op) {
         }
       } else {
         int32_t cst = second.GetConstant()->AsIntConstant()->GetValue();
-        uint32_t shift_value = static_cast<uint32_t>(cst & kMaxIntShiftValue);
+        uint32_t shift_value = cst & kMaxIntShiftDistance;
         if (shift_value == 0) {  // ARM does not support shifting with 0 immediate.
           __ Mov(out_reg, first_reg);
         } else if (op->IsShl()) {
@@ -3410,7 +3410,7 @@ void InstructionCodeGeneratorARM::HandleShift(HBinaryOperation* op) {
         Register second_reg = second.AsRegister<Register>();
 
         if (op->IsShl()) {
-          __ and_(o_l, second_reg, ShifterOperand(kMaxLongShiftValue));
+          __ and_(o_l, second_reg, ShifterOperand(kMaxLongShiftDistance));
           // Shift the high part
           __ Lsl(o_h, high, o_l);
           // Shift the low part and `or` what overflew on the high part
@@ -3424,7 +3424,7 @@ void InstructionCodeGeneratorARM::HandleShift(HBinaryOperation* op) {
           // Shift the low part
           __ Lsl(o_l, low, o_l);
         } else if (op->IsShr()) {
-          __ and_(o_h, second_reg, ShifterOperand(kMaxLongShiftValue));
+          __ and_(o_h, second_reg, ShifterOperand(kMaxLongShiftDistance));
           // Shift the low part
           __ Lsr(o_l, low, o_h);
           // Shift the high part and `or` what underflew on the low part
@@ -3438,7 +3438,7 @@ void InstructionCodeGeneratorARM::HandleShift(HBinaryOperation* op) {
           // Shift the high part
           __ Asr(o_h, high, o_h);
         } else {
-          __ and_(o_h, second_reg, ShifterOperand(kMaxLongShiftValue));
+          __ and_(o_h, second_reg, ShifterOperand(kMaxLongShiftDistance));
           // same as Shr except we use `Lsr`s and not `Asr`s
           __ Lsr(o_l, low, o_h);
           __ rsb(temp, o_h, ShifterOperand(kArmBitsPerWord));
@@ -3454,7 +3454,7 @@ void InstructionCodeGeneratorARM::HandleShift(HBinaryOperation* op) {
         DCHECK_NE(o_l, high);
         DCHECK_NE(o_h, low);
         int32_t cst = second.GetConstant()->AsIntConstant()->GetValue();
-        uint32_t shift_value = static_cast<uint32_t>(cst & kMaxLongShiftValue);
+        uint32_t shift_value = cst & kMaxLongShiftDistance;
         if (shift_value > 32) {
           if (op->IsShl()) {
             __ Lsl(o_h, low, shift_value - 32);
index 088dbb3..46fd852 100644 (file)
@@ -1818,9 +1818,8 @@ void InstructionCodeGeneratorARM64::HandleShift(HBinaryOperation* instr) {
       Register lhs = InputRegisterAt(instr, 0);
       Operand rhs = InputOperandAt(instr, 1);
       if (rhs.IsImmediate()) {
-        uint32_t shift_value = (type == Primitive::kPrimInt)
-          ? static_cast<uint32_t>(rhs.immediate() & kMaxIntShiftValue)
-          : static_cast<uint32_t>(rhs.immediate() & kMaxLongShiftValue);
+        uint32_t shift_value = rhs.immediate() &
+            (type == Primitive::kPrimInt ? kMaxIntShiftDistance : kMaxLongShiftDistance);
         if (instr->IsShl()) {
           __ Lsl(dst, lhs, shift_value);
         } else if (instr->IsShr()) {
@@ -1921,9 +1920,8 @@ void InstructionCodeGeneratorARM64::VisitArm64DataProcWithShifterOp(
   // conversion) can have a different type from the current instruction's type,
   // so we manually indicate the type.
   Register right_reg = RegisterFrom(instruction->GetLocations()->InAt(1), type);
-  int64_t shift_amount = (type == Primitive::kPrimInt)
-    ? static_cast<uint32_t>(instruction->GetShiftAmount() & kMaxIntShiftValue)
-    : static_cast<uint32_t>(instruction->GetShiftAmount() & kMaxLongShiftValue);
+  int64_t shift_amount = instruction->GetShiftAmount() &
+      (type == Primitive::kPrimInt ? kMaxIntShiftDistance : kMaxLongShiftDistance);
 
   Operand right_operand(0);
 
index 4c92063..684d89f 100644 (file)
@@ -1455,9 +1455,8 @@ void InstructionCodeGeneratorMIPS::HandleShift(HBinaryOperation* instr) {
   bool use_imm = rhs_location.IsConstant();
   Register rhs_reg = use_imm ? ZERO : rhs_location.AsRegister<Register>();
   int64_t rhs_imm = use_imm ? CodeGenerator::GetInt64ValueOf(rhs_location.GetConstant()) : 0;
-  const uint32_t shift_mask = (type == Primitive::kPrimInt)
-      ? kMaxIntShiftValue
-      : kMaxLongShiftValue;
+  const uint32_t shift_mask =
+      (type == Primitive::kPrimInt) ? kMaxIntShiftDistance : kMaxLongShiftDistance;
   const uint32_t shift_value = rhs_imm & shift_mask;
   // Are the INS (Insert Bit Field) and ROTR instructions supported?
   bool has_ins_rotr = codegen_->GetInstructionSetFeatures().IsMipsIsaRevGreaterThanEqual2();
index 955c9a4..0276a07 100644 (file)
@@ -1208,9 +1208,8 @@ void InstructionCodeGeneratorMIPS64::HandleShift(HBinaryOperation* instr) {
       }
 
       if (use_imm) {
-        uint32_t shift_value = (type == Primitive::kPrimInt)
-          ? static_cast<uint32_t>(rhs_imm & kMaxIntShiftValue)
-          : static_cast<uint32_t>(rhs_imm & kMaxLongShiftValue);
+        uint32_t shift_value = rhs_imm &
+            (type == Primitive::kPrimInt ? kMaxIntShiftDistance : kMaxLongShiftDistance);
 
         if (shift_value == 0) {
           if (dst != lhs) {
index fb3216c..3123c8b 100644 (file)
@@ -3775,7 +3775,7 @@ void InstructionCodeGeneratorX86::HandleShift(HBinaryOperation* op) {
           __ shrl(first_reg, second_reg);
         }
       } else {
-        int32_t shift = second.GetConstant()->AsIntConstant()->GetValue() & kMaxIntShiftValue;
+        int32_t shift = second.GetConstant()->AsIntConstant()->GetValue() & kMaxIntShiftDistance;
         if (shift == 0) {
           return;
         }
@@ -3803,7 +3803,7 @@ void InstructionCodeGeneratorX86::HandleShift(HBinaryOperation* op) {
         }
       } else {
         // Shift by a constant.
-        int shift = second.GetConstant()->AsIntConstant()->GetValue() & kMaxLongShiftValue;
+        int32_t shift = second.GetConstant()->AsIntConstant()->GetValue() & kMaxLongShiftDistance;
         // Nothing to do if the shift is 0, as the input is already the output.
         if (shift != 0) {
           if (op->IsShl()) {
@@ -3960,7 +3960,7 @@ void InstructionCodeGeneratorX86::VisitRor(HRor* ror) {
       Register second_reg = second.AsRegister<Register>();
       __ rorl(first_reg, second_reg);
     } else {
-      Immediate imm(second.GetConstant()->AsIntConstant()->GetValue() & kMaxIntShiftValue);
+      Immediate imm(second.GetConstant()->AsIntConstant()->GetValue() & kMaxIntShiftDistance);
       __ rorl(first_reg, imm);
     }
     return;
@@ -3981,7 +3981,7 @@ void InstructionCodeGeneratorX86::VisitRor(HRor* ror) {
     __ cmovl(kNotEqual, first_reg_hi, first_reg_lo);
     __ cmovl(kNotEqual, first_reg_lo, temp_reg);
   } else {
-    int32_t shift_amt = CodeGenerator::GetInt64ValueOf(second.GetConstant()) & kMaxLongShiftValue;
+    int32_t shift_amt = second.GetConstant()->AsIntConstant()->GetValue() & kMaxLongShiftDistance;
     if (shift_amt == 0) {
       // Already fine.
       return;
index 7648f61..b3b98be 100644 (file)
@@ -3799,7 +3799,7 @@ void InstructionCodeGeneratorX86_64::HandleShift(HBinaryOperation* op) {
           __ shrl(first_reg, second_reg);
         }
       } else {
-        Immediate imm(second.GetConstant()->AsIntConstant()->GetValue() & kMaxIntShiftValue);
+        Immediate imm(second.GetConstant()->AsIntConstant()->GetValue() & kMaxIntShiftDistance);
         if (op->IsShl()) {
           __ shll(first_reg, imm);
         } else if (op->IsShr()) {
@@ -3821,7 +3821,7 @@ void InstructionCodeGeneratorX86_64::HandleShift(HBinaryOperation* op) {
           __ shrq(first_reg, second_reg);
         }
       } else {
-        Immediate imm(second.GetConstant()->AsIntConstant()->GetValue() & kMaxLongShiftValue);
+        Immediate imm(second.GetConstant()->AsIntConstant()->GetValue() & kMaxLongShiftDistance);
         if (op->IsShl()) {
           __ shlq(first_reg, imm);
         } else if (op->IsShr()) {
@@ -3868,7 +3868,7 @@ void InstructionCodeGeneratorX86_64::VisitRor(HRor* ror) {
         CpuRegister second_reg = second.AsRegister<CpuRegister>();
         __ rorl(first_reg, second_reg);
       } else {
-        Immediate imm(second.GetConstant()->AsIntConstant()->GetValue() & kMaxIntShiftValue);
+        Immediate imm(second.GetConstant()->AsIntConstant()->GetValue() & kMaxIntShiftDistance);
         __ rorl(first_reg, imm);
       }
       break;
@@ -3877,7 +3877,7 @@ void InstructionCodeGeneratorX86_64::VisitRor(HRor* ror) {
         CpuRegister second_reg = second.AsRegister<CpuRegister>();
         __ rorq(first_reg, second_reg);
       } else {
-        Immediate imm(second.GetConstant()->AsIntConstant()->GetValue() & kMaxLongShiftValue);
+        Immediate imm(second.GetConstant()->AsIntConstant()->GetValue() & kMaxLongShiftDistance);
         __ rorq(first_reg, imm);
       }
       break;
index 9491ef6..055061d 100644 (file)
@@ -925,9 +925,12 @@ void GraphChecker::VisitBinaryOperation(HBinaryOperation* op) {
   Primitive::Type lhs_type = op->InputAt(0)->GetType();
   Primitive::Type rhs_type = op->InputAt(1)->GetType();
   Primitive::Type result_type = op->GetType();
+
+  // Type consistency between inputs.
   if (op->IsUShr() || op->IsShr() || op->IsShl() || op->IsRor()) {
     if (Primitive::PrimitiveKind(rhs_type) != Primitive::kPrimInt) {
-      AddError(StringPrintf("Shift operation %s %d has a non-int kind second input: %s of type %s.",
+      AddError(StringPrintf("Shift/rotate operation %s %d has a non-int kind second input: "
+                            "%s of type %s.",
                             op->DebugName(), op->GetId(),
                             op->InputAt(1)->DebugName(),
                             Primitive::PrettyDescriptor(rhs_type)));
@@ -941,21 +944,38 @@ void GraphChecker::VisitBinaryOperation(HBinaryOperation* op) {
     }
   }
 
+  // Type consistency between result and input(s).
   if (op->IsCompare()) {
     if (result_type != Primitive::kPrimInt) {
       AddError(StringPrintf("Compare operation %d has a non-int result type: %s.",
                             op->GetId(),
                             Primitive::PrettyDescriptor(result_type)));
     }
+  } else if (op->IsUShr() || op->IsShr() || op->IsShl() || op->IsRor()) {
+    // Only check the first input (value), as the second one (distance)
+    // must invariably be of kind `int`.
+    if (result_type != Primitive::PrimitiveKind(lhs_type)) {
+      AddError(StringPrintf("Shift/rotate operation %s %d has a result type different "
+                            "from its left-hand side (value) input kind: %s vs %s.",
+                            op->DebugName(), op->GetId(),
+                            Primitive::PrettyDescriptor(result_type),
+                            Primitive::PrettyDescriptor(lhs_type)));
+    }
   } else {
-    // Use the first input, so that we can also make this check for shift and rotate operations.
     if (Primitive::PrimitiveKind(result_type) != Primitive::PrimitiveKind(lhs_type)) {
       AddError(StringPrintf("Binary operation %s %d has a result kind different "
-                            "from its input kind: %s vs %s.",
+                            "from its left-hand side input kind: %s vs %s.",
                             op->DebugName(), op->GetId(),
                             Primitive::PrettyDescriptor(result_type),
                             Primitive::PrettyDescriptor(lhs_type)));
     }
+    if (Primitive::PrimitiveKind(result_type) != Primitive::PrimitiveKind(rhs_type)) {
+      AddError(StringPrintf("Binary operation %s %d has a result kind different "
+                            "from its right-hand side input kind: %s vs %s.",
+                            op->DebugName(), op->GetId(),
+                            Primitive::PrettyDescriptor(result_type),
+                            Primitive::PrettyDescriptor(rhs_type)));
+    }
   }
 }
 
index 4a8186a..72ea4b6 100644 (file)
@@ -240,8 +240,9 @@ void InstructionSimplifierVisitor::VisitShift(HBinaryOperation* instruction) {
 
   if (input_cst != nullptr) {
     int64_t cst = Int64FromConstant(input_cst);
-    int64_t mask =
-        (input_other->GetType() == Primitive::kPrimLong) ? kMaxLongShiftValue : kMaxIntShiftValue;
+    int64_t mask = (input_other->GetType() == Primitive::kPrimLong)
+        ? kMaxLongShiftDistance
+        : kMaxIntShiftDistance;
     if ((cst & mask) == 0) {
       // Replace code looking like
       //    SHL dst, src, 0
index 6736319..53da069 100644 (file)
@@ -72,8 +72,10 @@ static const int kDefaultNumberOfExceptionalPredecessors = 0;
 static const int kDefaultNumberOfDominatedBlocks = 1;
 static const int kDefaultNumberOfBackEdges = 1;
 
-static constexpr uint32_t kMaxIntShiftValue = 0x1f;
-static constexpr uint64_t kMaxLongShiftValue = 0x3f;
+// The maximum (meaningful) distance (31) that can be used in an integer shift/rotate operation.
+static constexpr int32_t kMaxIntShiftDistance = 0x1f;
+// The maximum (meaningful) distance (63) that can be used in a long shift/rotate operation.
+static constexpr int32_t kMaxLongShiftDistance = 0x3f;
 
 static constexpr uint32_t kUnknownFieldIndex = static_cast<uint32_t>(-1);
 static constexpr uint16_t kUnknownClassDefIndex = static_cast<uint16_t>(-1);
@@ -3442,10 +3444,8 @@ class HCompare : public HBinaryOperation {
                          SideEffectsForArchRuntimeCalls(comparison_type),
                          dex_pc) {
     SetPackedField<ComparisonBiasField>(bias);
-    if (kIsDebugBuild) {
-      DCHECK_EQ(comparison_type, Primitive::PrimitiveKind(first->GetType()));
-      DCHECK_EQ(comparison_type, Primitive::PrimitiveKind(second->GetType()));
-    }
+    DCHECK_EQ(comparison_type, Primitive::PrimitiveKind(first->GetType()));
+    DCHECK_EQ(comparison_type, Primitive::PrimitiveKind(second->GetType()));
   }
 
   template <typename T>
@@ -4445,37 +4445,39 @@ class HDivZeroCheck : public HExpression<1> {
 class HShl : public HBinaryOperation {
  public:
   HShl(Primitive::Type result_type,
-       HInstruction* left,
-       HInstruction* right,
+       HInstruction* value,
+       HInstruction* distance,
        uint32_t dex_pc = kNoDexPc)
-      : HBinaryOperation(result_type, left, right, SideEffects::None(), dex_pc) {}
+      : HBinaryOperation(result_type, value, distance, SideEffects::None(), dex_pc) {
+    DCHECK_EQ(result_type, Primitive::PrimitiveKind(value->GetType()));
+    DCHECK_EQ(Primitive::kPrimInt, Primitive::PrimitiveKind(distance->GetType()));
+  }
 
-  template <typename T, typename U, typename V>
-  T Compute(T x, U y, V max_shift_value) const {
-    static_assert(std::is_same<V, typename std::make_unsigned<T>::type>::value,
-                  "V is not the unsigned integer type corresponding to T");
-    return x << (y & max_shift_value);
+  template <typename T>
+  T Compute(T value, int32_t distance, int32_t max_shift_distance) const {
+    return value << (distance & max_shift_distance);
   }
 
-  HConstant* Evaluate(HIntConstant* x, HIntConstant* y) const OVERRIDE {
+  HConstant* Evaluate(HIntConstant* value, HIntConstant* distance) const OVERRIDE {
     return GetBlock()->GetGraph()->GetIntConstant(
-        Compute(x->GetValue(), y->GetValue(), kMaxIntShiftValue), GetDexPc());
+        Compute(value->GetValue(), distance->GetValue(), kMaxIntShiftDistance), GetDexPc());
   }
-  HConstant* Evaluate(HLongConstant* x, HIntConstant* y) const OVERRIDE {
+  HConstant* Evaluate(HLongConstant* value, HIntConstant* distance) const OVERRIDE {
     return GetBlock()->GetGraph()->GetLongConstant(
-        Compute(x->GetValue(), y->GetValue(), kMaxLongShiftValue), GetDexPc());
+        Compute(value->GetValue(), distance->GetValue(), kMaxLongShiftDistance), GetDexPc());
   }
-  HConstant* Evaluate(HLongConstant* x, HLongConstant* y) const OVERRIDE {
-    return GetBlock()->GetGraph()->GetLongConstant(
-        Compute(x->GetValue(), y->GetValue(), kMaxLongShiftValue), GetDexPc());
+  HConstant* Evaluate(HLongConstant* value ATTRIBUTE_UNUSED,
+                      HLongConstant* distance ATTRIBUTE_UNUSED) const OVERRIDE {
+    LOG(FATAL) << DebugName() << " is not defined for the (long, long) case.";
+    UNREACHABLE();
   }
-  HConstant* Evaluate(HFloatConstant* x ATTRIBUTE_UNUSED,
-                      HFloatConstant* y ATTRIBUTE_UNUSED) const OVERRIDE {
+  HConstant* Evaluate(HFloatConstant* value ATTRIBUTE_UNUSED,
+                      HFloatConstant* distance ATTRIBUTE_UNUSED) const OVERRIDE {
     LOG(FATAL) << DebugName() << " is not defined for float values";
     UNREACHABLE();
   }
-  HConstant* Evaluate(HDoubleConstant* x ATTRIBUTE_UNUSED,
-                      HDoubleConstant* y ATTRIBUTE_UNUSED) const OVERRIDE {
+  HConstant* Evaluate(HDoubleConstant* value ATTRIBUTE_UNUSED,
+                      HDoubleConstant* distance ATTRIBUTE_UNUSED) const OVERRIDE {
     LOG(FATAL) << DebugName() << " is not defined for double values";
     UNREACHABLE();
   }
@@ -4489,37 +4491,39 @@ class HShl : public HBinaryOperation {
 class HShr : public HBinaryOperation {
  public:
   HShr(Primitive::Type result_type,
-       HInstruction* left,
-       HInstruction* right,
+       HInstruction* value,
+       HInstruction* distance,
        uint32_t dex_pc = kNoDexPc)
-      : HBinaryOperation(result_type, left, right, SideEffects::None(), dex_pc) {}
+      : HBinaryOperation(result_type, value, distance, SideEffects::None(), dex_pc) {
+    DCHECK_EQ(result_type, Primitive::PrimitiveKind(value->GetType()));
+    DCHECK_EQ(Primitive::kPrimInt, Primitive::PrimitiveKind(distance->GetType()));
+  }
 
-  template <typename T, typename U, typename V>
-  T Compute(T x, U y, V max_shift_value) const {
-    static_assert(std::is_same<V, typename std::make_unsigned<T>::type>::value,
-                  "V is not the unsigned integer type corresponding to T");
-    return x >> (y & max_shift_value);
+  template <typename T>
+  T Compute(T value, int32_t distance, int32_t max_shift_distance) const {
+    return value >> (distance & max_shift_distance);
   }
 
-  HConstant* Evaluate(HIntConstant* x, HIntConstant* y) const OVERRIDE {
+  HConstant* Evaluate(HIntConstant* value, HIntConstant* distance) const OVERRIDE {
     return GetBlock()->GetGraph()->GetIntConstant(
-        Compute(x->GetValue(), y->GetValue(), kMaxIntShiftValue), GetDexPc());
+        Compute(value->GetValue(), distance->GetValue(), kMaxIntShiftDistance), GetDexPc());
   }
-  HConstant* Evaluate(HLongConstant* x, HIntConstant* y) const OVERRIDE {
+  HConstant* Evaluate(HLongConstant* value, HIntConstant* distance) const OVERRIDE {
     return GetBlock()->GetGraph()->GetLongConstant(
-        Compute(x->GetValue(), y->GetValue(), kMaxLongShiftValue), GetDexPc());
+        Compute(value->GetValue(), distance->GetValue(), kMaxLongShiftDistance), GetDexPc());
   }
-  HConstant* Evaluate(HLongConstant* x, HLongConstant* y) const OVERRIDE {
-    return GetBlock()->GetGraph()->GetLongConstant(
-        Compute(x->GetValue(), y->GetValue(), kMaxLongShiftValue), GetDexPc());
+  HConstant* Evaluate(HLongConstant* value ATTRIBUTE_UNUSED,
+                      HLongConstant* distance ATTRIBUTE_UNUSED) const OVERRIDE {
+    LOG(FATAL) << DebugName() << " is not defined for the (long, long) case.";
+    UNREACHABLE();
   }
-  HConstant* Evaluate(HFloatConstant* x ATTRIBUTE_UNUSED,
-                      HFloatConstant* y ATTRIBUTE_UNUSED) const OVERRIDE {
+  HConstant* Evaluate(HFloatConstant* value ATTRIBUTE_UNUSED,
+                      HFloatConstant* distance ATTRIBUTE_UNUSED) const OVERRIDE {
     LOG(FATAL) << DebugName() << " is not defined for float values";
     UNREACHABLE();
   }
-  HConstant* Evaluate(HDoubleConstant* x ATTRIBUTE_UNUSED,
-                      HDoubleConstant* y ATTRIBUTE_UNUSED) const OVERRIDE {
+  HConstant* Evaluate(HDoubleConstant* value ATTRIBUTE_UNUSED,
+                      HDoubleConstant* distance ATTRIBUTE_UNUSED) const OVERRIDE {
     LOG(FATAL) << DebugName() << " is not defined for double values";
     UNREACHABLE();
   }
@@ -4533,38 +4537,41 @@ class HShr : public HBinaryOperation {
 class HUShr : public HBinaryOperation {
  public:
   HUShr(Primitive::Type result_type,
-        HInstruction* left,
-        HInstruction* right,
+        HInstruction* value,
+        HInstruction* distance,
         uint32_t dex_pc = kNoDexPc)
-      : HBinaryOperation(result_type, left, right, SideEffects::None(), dex_pc) {}
+      : HBinaryOperation(result_type, value, distance, SideEffects::None(), dex_pc) {
+    DCHECK_EQ(result_type, Primitive::PrimitiveKind(value->GetType()));
+    DCHECK_EQ(Primitive::kPrimInt, Primitive::PrimitiveKind(distance->GetType()));
+  }
 
-  template <typename T, typename U, typename V>
-  T Compute(T x, U y, V max_shift_value) const {
-    static_assert(std::is_same<V, typename std::make_unsigned<T>::type>::value,
-                  "V is not the unsigned integer type corresponding to T");
-    V ux = static_cast<V>(x);
-    return static_cast<T>(ux >> (y & max_shift_value));
+  template <typename T>
+  T Compute(T value, int32_t distance, int32_t max_shift_distance) const {
+    typedef typename std::make_unsigned<T>::type V;
+    V ux = static_cast<V>(value);
+    return static_cast<T>(ux >> (distance & max_shift_distance));
   }
 
-  HConstant* Evaluate(HIntConstant* x, HIntConstant* y) const OVERRIDE {
+  HConstant* Evaluate(HIntConstant* value, HIntConstant* distance) const OVERRIDE {
     return GetBlock()->GetGraph()->GetIntConstant(
-        Compute(x->GetValue(), y->GetValue(), kMaxIntShiftValue), GetDexPc());
+        Compute(value->GetValue(), distance->GetValue(), kMaxIntShiftDistance), GetDexPc());
   }
-  HConstant* Evaluate(HLongConstant* x, HIntConstant* y) const OVERRIDE {
+  HConstant* Evaluate(HLongConstant* value, HIntConstant* distance) const OVERRIDE {
     return GetBlock()->GetGraph()->GetLongConstant(
-        Compute(x->GetValue(), y->GetValue(), kMaxLongShiftValue), GetDexPc());
+        Compute(value->GetValue(), distance->GetValue(), kMaxLongShiftDistance), GetDexPc());
   }
-  HConstant* Evaluate(HLongConstant* x, HLongConstant* y) const OVERRIDE {
-    return GetBlock()->GetGraph()->GetLongConstant(
-        Compute(x->GetValue(), y->GetValue(), kMaxLongShiftValue), GetDexPc());
+  HConstant* Evaluate(HLongConstant* value ATTRIBUTE_UNUSED,
+                      HLongConstant* distance ATTRIBUTE_UNUSED) const OVERRIDE {
+    LOG(FATAL) << DebugName() << " is not defined for the (long, long) case.";
+    UNREACHABLE();
   }
-  HConstant* Evaluate(HFloatConstant* x ATTRIBUTE_UNUSED,
-                      HFloatConstant* y ATTRIBUTE_UNUSED) const OVERRIDE {
+  HConstant* Evaluate(HFloatConstant* value ATTRIBUTE_UNUSED,
+                      HFloatConstant* distance ATTRIBUTE_UNUSED) const OVERRIDE {
     LOG(FATAL) << DebugName() << " is not defined for float values";
     UNREACHABLE();
   }
-  HConstant* Evaluate(HDoubleConstant* x ATTRIBUTE_UNUSED,
-                      HDoubleConstant* y ATTRIBUTE_UNUSED) const OVERRIDE {
+  HConstant* Evaluate(HDoubleConstant* value ATTRIBUTE_UNUSED,
+                      HDoubleConstant* distance ATTRIBUTE_UNUSED) const OVERRIDE {
     LOG(FATAL) << DebugName() << " is not defined for double values";
     UNREACHABLE();
   }
@@ -4690,45 +4697,43 @@ class HRor : public HBinaryOperation {
  public:
   HRor(Primitive::Type result_type, HInstruction* value, HInstruction* distance)
     : HBinaryOperation(result_type, value, distance) {
-    if (kIsDebugBuild) {
-      DCHECK_EQ(result_type, Primitive::PrimitiveKind(value->GetType()));
-      DCHECK_EQ(Primitive::kPrimInt, Primitive::PrimitiveKind(distance->GetType()));
-    }
+    DCHECK_EQ(result_type, Primitive::PrimitiveKind(value->GetType()));
+    DCHECK_EQ(Primitive::kPrimInt, Primitive::PrimitiveKind(distance->GetType()));
   }
 
-  template <typename T, typename U, typename V>
-  T Compute(T x, U y, V max_shift_value) const {
-    static_assert(std::is_same<V, typename std::make_unsigned<T>::type>::value,
-                  "V is not the unsigned integer type corresponding to T");
-    V ux = static_cast<V>(x);
-    if ((y & max_shift_value) == 0) {
+  template <typename T>
+  T Compute(T value, int32_t distance, int32_t max_shift_value) const {
+    typedef typename std::make_unsigned<T>::type V;
+    V ux = static_cast<V>(value);
+    if ((distance & max_shift_value) == 0) {
       return static_cast<T>(ux);
     } else {
       const V reg_bits = sizeof(T) * 8;
-      return static_cast<T>(ux >> (y & max_shift_value)) |
-                           (x << (reg_bits - (y & max_shift_value)));
+      return static_cast<T>(ux >> (distance & max_shift_value)) |
+                           (value << (reg_bits - (distance & max_shift_value)));
     }
   }
 
-  HConstant* Evaluate(HIntConstant* x, HIntConstant* y) const OVERRIDE {
+  HConstant* Evaluate(HIntConstant* value, HIntConstant* distance) const OVERRIDE {
     return GetBlock()->GetGraph()->GetIntConstant(
-        Compute(x->GetValue(), y->GetValue(), kMaxIntShiftValue), GetDexPc());
+        Compute(value->GetValue(), distance->GetValue(), kMaxIntShiftDistance), GetDexPc());
   }
-  HConstant* Evaluate(HLongConstant* x, HIntConstant* y) const OVERRIDE {
+  HConstant* Evaluate(HLongConstant* value, HIntConstant* distance) const OVERRIDE {
     return GetBlock()->GetGraph()->GetLongConstant(
-        Compute(x->GetValue(), y->GetValue(), kMaxLongShiftValue), GetDexPc());
+        Compute(value->GetValue(), distance->GetValue(), kMaxLongShiftDistance), GetDexPc());
   }
-  HConstant* Evaluate(HLongConstant* x, HLongConstant* y) const OVERRIDE {
-    return GetBlock()->GetGraph()->GetLongConstant(
-        Compute(x->GetValue(), y->GetValue(), kMaxLongShiftValue), GetDexPc());
+  HConstant* Evaluate(HLongConstant* value ATTRIBUTE_UNUSED,
+                      HLongConstant* distance ATTRIBUTE_UNUSED) const OVERRIDE {
+    LOG(FATAL) << DebugName() << " is not defined for the (long, long) case.";
+    UNREACHABLE();
   }
-  HConstant* Evaluate(HFloatConstant* x ATTRIBUTE_UNUSED,
-                      HFloatConstant* y ATTRIBUTE_UNUSED) const OVERRIDE {
+  HConstant* Evaluate(HFloatConstant* value ATTRIBUTE_UNUSED,
+                      HFloatConstant* distance ATTRIBUTE_UNUSED) const OVERRIDE {
     LOG(FATAL) << DebugName() << " is not defined for float values";
     UNREACHABLE();
   }
-  HConstant* Evaluate(HDoubleConstant* x ATTRIBUTE_UNUSED,
-                      HDoubleConstant* y ATTRIBUTE_UNUSED) const OVERRIDE {
+  HConstant* Evaluate(HDoubleConstant* value ATTRIBUTE_UNUSED,
+                      HDoubleConstant* distance ATTRIBUTE_UNUSED) const OVERRIDE {
     LOG(FATAL) << DebugName() << " is not defined for double values";
     UNREACHABLE();
   }