[vm/compiler] Remove speculative int32 range checks

UnboxedInt32 representation is no longer speculative, so
all speculative int32 range checks are removed from Unbox and
IntConverter instructions. Unbox and IntConverter instructions
are now always truncating. IntConverter cannot deoptimize.

TEST=ci

Change-Id: Ie9a83e055e8b8372859b0721ac6fbcf73da39ab8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/392740
Reviewed-by: Slava Egorov <[email protected]>
Commit-Queue: Alexander Markov <[email protected]>

Author: alexmarkov

runtime/vm/compiler/aot/aot_call_specializer.cc

@@ -406,7 +406,7 @@ Definition* AotCallSpecializer::TryOptimizeDivisionOperation(
         Smi::ZoneHandle(Z, Smi::New(value)), kUnboxedInt32);
     InsertBefore(instr, const_def, /*env=*/nullptr, FlowGraph::kValue);
     return new (Z) IntConverterInstr(kUnboxedInt32, kUnboxedInt64,
-                                     new (Z) Value(const_def), DeoptId::kNone);
+                                     new (Z) Value(const_def));
 #else
     return new (Z) UnboxedConstantInstr(Smi::ZoneHandle(Z, Smi::New(value)),
                                         kUnboxedInt64);

runtime/vm/compiler/backend/block_builder.h

@@ -109,12 +109,9 @@ class BlockBuilder : public ValueObject {
                                : Boxing::NativeRepresentation(rep);
     Definition* unboxed_value = AddDefinition(
         UnboxInstr::Create(unbox_rep, value, DeoptId::kNone, value_mode));
-    if (rep != unbox_rep && unboxed_value->IsUnboxInteger()) {
-      ASSERT(RepresentationUtils::ValueSize(rep) <
-             RepresentationUtils::ValueSize(unbox_rep));
-      // Mark unboxing of small unboxed integer representations as truncating.
-      unboxed_value->AsUnboxInteger()->mark_truncating();
-    }
+    ASSERT((rep == unbox_rep) || !unboxed_value->IsUnboxInteger() ||
+           (RepresentationUtils::ValueSize(rep) <
+            RepresentationUtils::ValueSize(unbox_rep)));
     if (value_mode == UnboxInstr::ValueMode::kHasValidType) {
       // The type of |value| has already been checked and it is safe to
       // adjust reaching type. This is done manually because there is no type

runtime/vm/compiler/backend/constant_propagator.cc

@@ -1509,9 +1509,8 @@ void ConstantPropagator::VisitUnbox(UnboxInstr* instr) {
       SetValue(instr, non_constant_);
       return;
     }
-    if (unbox_int->is_truncating() &&
-        ((unbox_int->representation() == kUnboxedInt32) ||
-         (unbox_int->representation() == kUnboxedUint32))) {
+    if ((unbox_int->representation() == kUnboxedInt32) ||
+        (unbox_int->representation() == kUnboxedUint32)) {
       const int64_t result_val = Evaluator::TruncateTo(
           Integer::Cast(value).Value(), unbox_int->representation());
       value = Integer::NewCanonical(result_val);

runtime/vm/compiler/backend/flow_graph.cc

@@ -1974,8 +1974,7 @@ void FlowGraph::InsertConversion(Representation from,
 
   Definition* converted = nullptr;
   if (IsUnboxedInteger(from) && IsUnboxedInteger(to)) {
-    converted = new (Z)
-        IntConverterInstr(from, to, use->CopyWithType(), DeoptId::kNone);
+    converted = new (Z) IntConverterInstr(from, to, use->CopyWithType());
   } else if ((from == kUnboxedInt32) && (to == kUnboxedDouble)) {
     converted = new Int32ToDoubleInstr(use->CopyWithType());
   } else if ((from == kUnboxedInt64) && (to == kUnboxedDouble) &&

runtime/vm/compiler/backend/il.cc

@@ -2027,33 +2027,6 @@ void Instruction::Goto(JoinEntryInstr* entry) {
   LinkTo(new GotoInstr(entry, CompilerState::Current().GetNextDeoptId()));
 }
 
-bool IntConverterInstr::ComputeCanDeoptimize() const {
-  return (to() == kUnboxedInt32) && !is_truncating() &&
-         !RangeUtils::Fits(value()->definition()->range(),
-                           RangeBoundary::kRangeBoundaryInt32);
-}
-
-bool UnboxIntegerInstr::ComputeCanDeoptimize() const {
-  if (value_mode() == ValueMode::kHasValidType) {
-    return false;
-  }
-  if (!value()->Type()->IsInt()) {
-    return true;
-  }
-  if (representation() == kUnboxedInt64 || is_truncating()) {
-    return false;
-  }
-  const intptr_t rep_bitsize =
-      RepresentationUtils::ValueSize(representation()) * kBitsPerByte;
-  if (value()->Type()->ToCid() == kSmiCid &&
-      compiler::target::kSmiBits <= rep_bitsize) {
-    return false;
-  }
-  return !RangeUtils::IsWithin(value()->definition()->range(),
-                               RepresentationUtils::MinValue(representation()),
-                               RepresentationUtils::MaxValue(representation()));
-}
-
 bool BinaryInt32OpInstr::ComputeCanDeoptimize() const {
   switch (op_kind()) {
     case Token::kBIT_AND:
@@ -3369,19 +3342,9 @@ Definition* UnboxIntegerInstr::Canonicalize(FlowGraph* flow_graph) {
       return box_defn->value()->definition();
     } else {
       // Only operate on explicit unboxed operands.
-      IntConverterInstr* converter = new IntConverterInstr(
-          from_representation, representation(),
-          box_defn->value()->CopyWithType(),
-          (representation() == kUnboxedInt32) ? GetDeoptId() : DeoptId::kNone);
-      // TODO(vegorov): marking resulting converter as truncating when
-      // unboxing can't deoptimize is a workaround for the missing
-      // deoptimization environment when we insert converter after
-      // EliminateEnvironments and there is a mismatch between predicates
-      // UnboxIntConverterInstr::CanDeoptimize and UnboxInt32::CanDeoptimize.
-      if ((representation() == kUnboxedInt32) &&
-          (is_truncating() || !CanDeoptimize())) {
-        converter->mark_truncating();
-      }
+      IntConverterInstr* converter =
+          new IntConverterInstr(from_representation, representation(),
+                                box_defn->value()->CopyWithType());
       flow_graph->InsertBefore(this, converter, env(), FlowGraph::kValue);
       return converter;
     }
@@ -3403,13 +3366,11 @@ Definition* UnboxIntegerInstr::Canonicalize(FlowGraph* flow_graph) {
       if (RepresentationUtils::IsRepresentable(representation(), intval)) {
         return flow_graph->GetConstant(obj, representation());
       }
-      if (is_truncating()) {
-        const int64_t result = Evaluator::TruncateTo(intval, representation());
-        return flow_graph->GetConstant(
-            Integer::ZoneHandle(flow_graph->zone(),
-                                Integer::NewCanonical(result)),
-            representation());
-      }
+      const int64_t result = Evaluator::TruncateTo(intval, representation());
+      return flow_graph->GetConstant(
+          Integer::ZoneHandle(flow_graph->zone(),
+                              Integer::NewCanonical(result)),
+          representation());
     }
   }
 
@@ -3426,11 +3387,10 @@ Definition* IntConverterInstr::Canonicalize(FlowGraph* flow_graph) {
       const int64_t value = Integer::Cast(constant->value()).Value();
       const int64_t result =
           Evaluator::TruncateTo(Evaluator::TruncateTo(value, from()), to());
-      if (is_truncating() || (value == result)) {
-        auto& box = Integer::Handle(Integer::New(result, Heap::kOld));
-        box ^= box.Canonicalize(flow_graph->thread());
-        return flow_graph->GetConstant(box, to());
-      }
+      return flow_graph->GetConstant(
+          Integer::ZoneHandle(flow_graph->zone(),
+                              Integer::NewCanonical(result)),
+          to());
     }
   }
 
@@ -3461,13 +3421,9 @@ Definition* IntConverterInstr::Canonicalize(FlowGraph* flow_graph) {
       return this;
     }
 
-    IntConverterInstr* converter = new IntConverterInstr(
-        first_converter->from(), representation(),
-        first_converter->value()->CopyWithType(),
-        (to() == kUnboxedInt32) ? GetDeoptId() : DeoptId::kNone);
-    if ((representation() == kUnboxedInt32) && is_truncating()) {
-      converter->mark_truncating();
-    }
+    IntConverterInstr* converter =
+        new IntConverterInstr(first_converter->from(), representation(),
+                              first_converter->value()->CopyWithType());
     flow_graph->InsertBefore(this, converter, env(), FlowGraph::kValue);
     return converter;
   }
@@ -3479,9 +3435,7 @@ Definition* IntConverterInstr::Canonicalize(FlowGraph* flow_graph) {
     // and code path that unboxes Mint into Int32. We should just schedule
     // these instructions close to each other instead of fusing them.
     Definition* replacement =
-        new UnboxInt32Instr(is_truncating() ? UnboxInt32Instr::kTruncate
-                                            : UnboxInt32Instr::kNoTruncation,
-                            unbox_defn->value()->CopyWithType(), GetDeoptId(),
+        new UnboxInt32Instr(unbox_defn->value()->CopyWithType(), GetDeoptId(),
                             unbox_defn->value_mode());
     flow_graph->InsertBefore(this, replacement, env(), FlowGraph::kValue);
     return replacement;
@@ -4038,12 +3992,7 @@ UnboxInstr* UnboxInstr::Create(Representation to,
                                UnboxInstr::ValueMode value_mode) {
   switch (to) {
     case kUnboxedInt32:
-      // We must truncate if we can't deoptimize.
-      return new UnboxInt32Instr(
-          (value_mode == UnboxInstr::ValueMode::kCheckType)
-              ? UnboxInt32Instr::kNoTruncation
-              : UnboxInt32Instr::kTruncate,
-          value, deopt_id, value_mode);
+      return new UnboxInt32Instr(value, deopt_id, value_mode);
 
     case kUnboxedUint32:
       return new UnboxUint32Instr(value, deopt_id, value_mode);

runtime/vm/compiler/backend/il.h

@@ -8616,24 +8616,10 @@ class UnboxIntegerInstr : public UnboxInstr {
   enum TruncationMode { kTruncate, kNoTruncation };
 
   UnboxIntegerInstr(Representation representation,
-                    TruncationMode truncation_mode,
                     Value* value,
                     intptr_t deopt_id,
                     ValueMode value_mode)
-      : UnboxInstr(representation, value, deopt_id, value_mode),
-        is_truncating_(truncation_mode == kTruncate) {}
-
-  bool is_truncating() const { return is_truncating_; }
-
-  void mark_truncating() { is_truncating_ = true; }
-
-  virtual bool ComputeCanDeoptimize() const;
-
-  virtual bool AttributesEqual(const Instruction& other) const {
-    auto const other_unbox = other.AsUnboxInteger();
-    return UnboxInstr::AttributesEqual(other) &&
-           (other_unbox->is_truncating_ == is_truncating_);
-  }
+      : UnboxInstr(representation, value, deopt_id, value_mode) {}
 
   virtual Definition* Canonicalize(FlowGraph* flow_graph);
 
@@ -8643,12 +8629,7 @@ class UnboxIntegerInstr : public UnboxInstr {
 
   PRINT_OPERANDS_TO_SUPPORT
 
-#define FIELD_LIST(F) F(bool, is_truncating_)
-
-  DECLARE_INSTRUCTION_SERIALIZABLE_FIELDS(UnboxIntegerInstr,
-                                          UnboxInstr,
-                                          FIELD_LIST)
-#undef FIELD_LIST
+  DECLARE_EMPTY_SERIALIZATION(UnboxIntegerInstr, UnboxInstr)
 
  private:
   DISALLOW_COPY_AND_ASSIGN(UnboxIntegerInstr);
@@ -8657,15 +8638,10 @@ class UnboxIntegerInstr : public UnboxInstr {
 class UnboxInteger32Instr : public UnboxIntegerInstr {
  public:
   UnboxInteger32Instr(Representation representation,
-                      TruncationMode truncation_mode,
                       Value* value,
                       intptr_t deopt_id,
                       ValueMode value_mode)
-      : UnboxIntegerInstr(representation,
-                          truncation_mode,
-                          value,
-                          deopt_id,
-                          value_mode) {}
+      : UnboxIntegerInstr(representation, value, deopt_id, value_mode) {}
 
   DECLARE_INSTRUCTION_BACKEND()
 
@@ -8678,13 +8654,7 @@ class UnboxInteger32Instr : public UnboxIntegerInstr {
 class UnboxUint32Instr : public UnboxInteger32Instr {
  public:
   UnboxUint32Instr(Value* value, intptr_t deopt_id, ValueMode value_mode)
-      : UnboxInteger32Instr(kUnboxedUint32,
-                            kTruncate,
-                            value,
-                            deopt_id,
-                            value_mode) {
-    ASSERT(is_truncating());
-  }
+      : UnboxInteger32Instr(kUnboxedUint32, value, deopt_id, value_mode) {}
 
   DECLARE_INSTRUCTION_NO_BACKEND(UnboxUint32)
 
@@ -8696,15 +8666,8 @@ class UnboxUint32Instr : public UnboxInteger32Instr {
 
 class UnboxInt32Instr : public UnboxInteger32Instr {
  public:
-  UnboxInt32Instr(TruncationMode truncation_mode,
-                  Value* value,
-                  intptr_t deopt_id,
-                  ValueMode value_mode)
-      : UnboxInteger32Instr(kUnboxedInt32,
-                            truncation_mode,
-                            value,
-                            deopt_id,
-                            value_mode) {}
+  UnboxInt32Instr(Value* value, intptr_t deopt_id, ValueMode value_mode)
+      : UnboxInteger32Instr(kUnboxedInt32, value, deopt_id, value_mode) {}
 
   DECLARE_INSTRUCTION_NO_BACKEND(UnboxInt32)
 
@@ -8717,11 +8680,7 @@ class UnboxInt32Instr : public UnboxInteger32Instr {
 class UnboxInt64Instr : public UnboxIntegerInstr {
  public:
   UnboxInt64Instr(Value* value, intptr_t deopt_id, ValueMode value_mode)
-      : UnboxIntegerInstr(kUnboxedInt64,
-                          kNoTruncation,
-                          value,
-                          deopt_id,
-                          value_mode) {}
+      : UnboxIntegerInstr(kUnboxedInt64, value, deopt_id, value_mode) {}
 
   DECLARE_INSTRUCTION_NO_BACKEND(UnboxInt64)
 
@@ -10788,14 +10747,10 @@ class CheckConditionInstr : public Instruction {
 
 class IntConverterInstr : public TemplateDefinition<1, NoThrow, Pure> {
  public:
-  IntConverterInstr(Representation from,
-                    Representation to,
-                    Value* value,
-                    intptr_t deopt_id)
-      : TemplateDefinition(deopt_id),
+  IntConverterInstr(Representation from, Representation to, Value* value)
+      : TemplateDefinition(DeoptId::kNone),
         from_representation_(from),
-        to_representation_(to),
-        is_truncating_(to == kUnboxedUint32) {
+        to_representation_(to) {
     ASSERT(from != to);
     // Integer conversion doesn't currently handle non-native representations.
     ASSERT_EQUAL(Boxing::NativeRepresentation(from), from);
@@ -10816,13 +10771,10 @@ class IntConverterInstr : public TemplateDefinition<1, NoThrow, Pure> {
 
   Representation from() const { return from_representation_; }
   Representation to() const { return to_representation_; }
-  bool is_truncating() const { return is_truncating_; }
-
-  void mark_truncating() { is_truncating_ = true; }
 
   Definition* Canonicalize(FlowGraph* flow_graph);
 
-  virtual bool ComputeCanDeoptimize() const;
+  virtual bool ComputeCanDeoptimize() const { return false; }
 
   virtual Representation representation() const { return to(); }
 
@@ -10834,8 +10786,7 @@ class IntConverterInstr : public TemplateDefinition<1, NoThrow, Pure> {
   virtual bool AttributesEqual(const Instruction& other) const {
     ASSERT(other.IsIntConverter());
     auto const converter = other.AsIntConverter();
-    return (converter->from() == from()) && (converter->to() == to()) &&
-           (converter->is_truncating() == is_truncating());
+    return (converter->from() == from()) && (converter->to() == to());
   }
 
   virtual intptr_t DeoptimizationTarget() const { return GetDeoptId(); }
@@ -10850,15 +10801,13 @@ class IntConverterInstr : public TemplateDefinition<1, NoThrow, Pure> {
 
   DECLARE_INSTRUCTION(IntConverter);
 
-  DECLARE_ATTRIBUTES_NAMED(("from", "to", "is_truncating"),
-                           (from(), to(), is_truncating()))
+  DECLARE_ATTRIBUTES_NAMED(("from", "to"), (from(), to()))
 
   PRINT_OPERANDS_TO_SUPPORT
 
 #define FIELD_LIST(F)                                                          \
   F(const Representation, from_representation_)                                \
-  F(const Representation, to_representation_)                                  \
-  F(bool, is_truncating_)
+  F(const Representation, to_representation_)
 
   DECLARE_INSTRUCTION_SERIALIZABLE_FIELDS(IntConverterInstr,
                                           TemplateDefinition,