Reland "[vm/compiler] Unify methods for UnboxIntegerOpInstr subclasses."

This is a reland of commit baa18e6d29

Fixes IsRepresentable to take a int64_t, not a uint64_t.

Original change's description:
> [vm/compiler] Unify methods for UnboxIntegerOpInstr subclasses.
>
> Unify the Canonicalize() methods on subclasses of UnboxIntegerOp
> so that there aren't subtle differences between canonicalizing
> UnboxInt32 and UnboxUint32 instructions. Also unify the
> ComputeCanDeoptimize() methods for similar reasons.
>
> If canonicalizing a truncating unbox of a constant, then create an
> unboxed constant of the truncated value instead of an unboxed
> constant of the original untruncated value.
>
> Previously, a subclass of Definition that didn't override InferRange
> got a default range solely based on its type. Now, if the representation
> of the definition is an unboxed integer, the default range is the
> intersection of the range inferred from its type with the range
> inferred from its representation.
>
> TEST=ci
>
> Change-Id: Ib022c366904ee6f8a81995bd4c16b87bd876176d
> Cq-Include-Trybots: luci.dart.try:vm-aot-linux-debug-simarm_x64-try,vm-aot-linux-release-x64-try,vm-aot-linux-debug-x64-try,vm-aot-linux-debug-x64c-try,vm-win-release-x64-try,vm-ffi-qemu-linux-release-arm-try,vm-ffi-android-debug-arm-try
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/332400
> Reviewed-by: Slava Egorov <vegorov@google.com>

TEST=ci (including simarm, not just simarm_x64)

Change-Id: I6f3d3976634da9725d1a81faa62ab9b718d6663e
Cq-Include-Trybots: luci.dart.try:vm-aot-linux-debug-simarm_x64-try,vm-aot-linux-release-x64-try,vm-aot-linux-debug-x64-try,vm-aot-linux-debug-x64c-try,vm-win-release-x64-try,vm-ffi-qemu-linux-release-arm-try,vm-ffi-android-debug-arm-try,vm-linux-release-simarm-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/333001
Reviewed-by: Slava Egorov <vegorov@google.com>
This commit is contained in:
Tess Strickland 2023-11-01 12:47:59 +00:00 committed by Commit Queue
parent 8b01baab71
commit 3a6daede9a
6 changed files with 128 additions and 145 deletions

View file

@ -1980,41 +1980,25 @@ bool IntConverterInstr::ComputeCanDeoptimize() const {
RangeBoundary::kRangeBoundaryInt32);
}
bool UnboxInt32Instr::ComputeCanDeoptimize() const {
bool UnboxIntegerInstr::ComputeCanDeoptimize() const {
if (SpeculativeModeOfInputs() == kNotSpeculative) {
return false;
}
const intptr_t value_cid = value()->Type()->ToCid();
if (value_cid == kSmiCid) {
return (compiler::target::kSmiBits > 32) && !is_truncating() &&
!RangeUtils::Fits(value()->definition()->range(),
RangeBoundary::kRangeBoundaryInt32);
} else if (value_cid == kMintCid) {
return !is_truncating() &&
!RangeUtils::Fits(value()->definition()->range(),
RangeBoundary::kRangeBoundaryInt32);
} else if (is_truncating() && value()->definition()->IsBoxInteger()) {
return false;
} else if ((compiler::target::kSmiBits < 32) && value()->Type()->IsInt()) {
return !RangeUtils::Fits(value()->definition()->range(),
RangeBoundary::kRangeBoundaryInt32);
} else {
if (!value()->Type()->IsInt()) {
return true;
}
}
bool UnboxUint32Instr::ComputeCanDeoptimize() const {
ASSERT(is_truncating());
if (SpeculativeModeOfInputs() == kNotSpeculative) {
if (representation() == kUnboxedInt64 || is_truncating()) {
return false;
}
if ((value()->Type()->ToCid() == kSmiCid) ||
(value()->Type()->ToCid() == kMintCid)) {
const intptr_t rep_bitsize =
RepresentationUtils::ValueSize(representation()) * kBitsPerByte;
if (value()->Type()->ToCid() == kSmiCid &&
compiler::target::kSmiBits <= rep_bitsize) {
return false;
}
// Check input value's range.
Range* value_range = value()->definition()->range();
return !RangeUtils::Fits(value_range, RangeBoundary::kRangeBoundaryInt64);
return !RangeUtils::IsWithin(value()->definition()->range(),
RepresentationUtils::MinValue(representation()),
RepresentationUtils::MaxValue(representation()));
}
bool BinaryInt32OpInstr::ComputeCanDeoptimize() const {
@ -3306,40 +3290,24 @@ Definition* UnboxIntegerInstr::Canonicalize(FlowGraph* flow_graph) {
set_speculative_mode(kNotSpeculative);
}
return this;
}
Definition* UnboxInt32Instr::Canonicalize(FlowGraph* flow_graph) {
Definition* replacement = UnboxIntegerInstr::Canonicalize(flow_graph);
if (replacement != this) {
return replacement;
}
ConstantInstr* c = value()->definition()->AsConstant();
if ((c != nullptr) && c->value().IsInteger()) {
if (!is_truncating()) {
// Check that constant fits into 32-bit integer.
const int64_t value = Integer::Cast(c->value()).AsInt64Value();
if (!Utils::IsInt(32, value)) {
return this;
if (value()->BindsToConstant()) {
const auto& obj = value()->BoundConstant();
if (obj.IsInteger()) {
if (representation() == kUnboxedInt64) {
return flow_graph->GetConstant(obj, representation());
}
const int64_t intval = Integer::Cast(obj).AsInt64Value();
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());
}
}
return flow_graph->GetConstant(c->value(), kUnboxedInt32);
}
return this;
}
Definition* UnboxInt64Instr::Canonicalize(FlowGraph* flow_graph) {
Definition* replacement = UnboxIntegerInstr::Canonicalize(flow_graph);
if (replacement != this) {
return replacement;
}
ConstantInstr* c = value()->definition()->AsConstant();
if (c != nullptr && c->value().IsInteger()) {
return flow_graph->GetConstant(c->value(), kUnboxedInt64);
}
return this;

View file

@ -8511,6 +8511,8 @@ class UnboxIntegerInstr : public UnboxInstr {
void mark_truncating() { is_truncating_ = true; }
virtual bool ComputeCanDeoptimize() const;
virtual CompileType ComputeType() const;
virtual bool AttributesEqual(const Instruction& other) const {
@ -8521,6 +8523,8 @@ class UnboxIntegerInstr : public UnboxInstr {
virtual Definition* Canonicalize(FlowGraph* flow_graph);
virtual void InferRange(RangeAnalysis* analysis, Range* range);
DECLARE_ABSTRACT_INSTRUCTION(UnboxInteger)
PRINT_OPERANDS_TO_SUPPORT
@ -8570,10 +8574,6 @@ class UnboxUint32Instr : public UnboxInteger32Instr {
ASSERT(is_truncating());
}
virtual bool ComputeCanDeoptimize() const;
virtual void InferRange(RangeAnalysis* analysis, Range* range);
DECLARE_INSTRUCTION_NO_BACKEND(UnboxUint32)
DECLARE_EMPTY_SERIALIZATION(UnboxUint32Instr, UnboxInteger32Instr)
@ -8594,12 +8594,6 @@ class UnboxInt32Instr : public UnboxInteger32Instr {
deopt_id,
speculative_mode) {}
virtual bool ComputeCanDeoptimize() const;
virtual void InferRange(RangeAnalysis* analysis, Range* range);
virtual Definition* Canonicalize(FlowGraph* flow_graph);
DECLARE_INSTRUCTION_NO_BACKEND(UnboxInt32)
DECLARE_EMPTY_SERIALIZATION(UnboxInt32Instr, UnboxInteger32Instr)
@ -8619,18 +8613,6 @@ class UnboxInt64Instr : public UnboxIntegerInstr {
deopt_id,
speculative_mode) {}
virtual void InferRange(RangeAnalysis* analysis, Range* range);
virtual Definition* Canonicalize(FlowGraph* flow_graph);
virtual bool ComputeCanDeoptimize() const {
if (SpeculativeModeOfInputs() == kNotSpeculative) {
return false;
}
return !value()->Type()->IsInt();
}
DECLARE_INSTRUCTION_NO_BACKEND(UnboxInt64)
DECLARE_EMPTY_SERIALIZATION(UnboxInt64Instr, UnboxIntegerInstr)

View file

@ -121,6 +121,12 @@ int64_t RepresentationUtils::MaxValue(Representation rep) {
}
#undef REP_MAX_VALUE_CLAUSE
bool RepresentationUtils::IsRepresentable(Representation rep, int64_t value) {
const intptr_t bit_size = ValueSize(rep) * kBitsPerByte;
return IsUnsigned(rep) ? Utils::IsUint(bit_size, value)
: Utils::IsInt(bit_size, value);
}
const char* Location::RepresentationToCString(Representation repr) {
switch (repr) {
#define REPR_CASE(Name, __, ___) \

View file

@ -97,12 +97,16 @@ struct RepresentationUtils : AllStatic {
static compiler::OperandSize OperandSize(Representation rep);
// The minimum integral value that can be represented.
// Assumes that [rep] is a unboxed integer.
// Assumes that [rep] is an unboxed integer.
static int64_t MinValue(Representation rep);
// The maximum integral value that can be represented.
// Assumes that [rep] is a unboxed integer.
// Assumes that [rep] is an unboxed integer.
static int64_t MaxValue(Representation rep);
// Whether the given value is representable in the given representation.
// Assumes that [rep] is an unboxed integer.
static bool IsRepresentable(Representation rep, int64_t value);
};
// The representation for word-sized unboxed fields.

View file

@ -24,6 +24,35 @@ DECLARE_FLAG(bool, trace_constant_propagation);
// Quick access to the locally defined zone() method.
#define Z (zone())
#if defined(DEBUG)
static void CheckRangeForRepresentation(const Assert& assert,
const Instruction* instr,
const Range* range,
Representation rep) {
const Range other = Range::Full(rep);
if (!RangeUtils::IsWithin(range, &other)) {
assert.Fail(
"During range analysis for:\n %s\n"
"expected range containing only %s-representable values, but got %s",
instr->ToCString(), RepresentationToCString(rep),
Range::ToCString(range));
}
}
#define ASSERT_VALID_RANGE_FOR_REPRESENTATION(instr, range, representation) \
do { \
CheckRangeForRepresentation(dart::Assert(__FILE__, __LINE__), instr, \
range, representation); \
} while (false)
#else
#define ASSERT_VALID_RANGE_FOR_REPRESENTATION(instr, range, representation) \
do { \
USE(instr); \
USE(range); \
USE(representation); \
} while (false)
#endif
void RangeAnalysis::Analyze() {
CollectValues();
InsertConstraints();
@ -2121,6 +2150,19 @@ bool Range::IsWithin(int64_t min_int, int64_t max_int) const {
return OnlyGreaterThanOrEqualTo(min_int) && OnlyLessThanOrEqualTo(max_int);
}
bool Range::IsWithin(const Range* other) const {
auto const lower_bound = other->min().LowerBound();
auto const upper_bound = other->max().UpperBound();
if (lower_bound.IsNegativeInfinity()) {
if (upper_bound.IsPositiveInfinity()) return true;
return OnlyLessThanOrEqualTo(other->max().ConstantValue());
} else if (upper_bound.IsPositiveInfinity()) {
return OnlyGreaterThanOrEqualTo(other->min().ConstantValue());
} else {
return IsWithin(other->min().ConstantValue(), other->max().ConstantValue());
}
}
bool Range::Overlaps(int64_t min_int, int64_t max_int) const {
RangeBoundary lower = min().LowerBound();
RangeBoundary upper = max().UpperBound();
@ -2601,6 +2643,12 @@ void Definition::InferRange(RangeAnalysis* analysis, Range* range) {
// Only Smi and Mint supported.
FATAL("Unsupported type in: %s", ToCString());
}
// If the representation also gives us range information, then refine
// the range from the type by using the intersection of the two.
if (RepresentationUtils::IsUnboxedInteger(representation())) {
*range = Range::Full(representation()).Intersect(range);
}
}
static bool DependsOnSymbol(const RangeBoundary& a, Definition* symbol) {
@ -3079,60 +3127,35 @@ void ShiftIntegerOpInstr::InferRange(RangeAnalysis* analysis, Range* range) {
void BoxIntegerInstr::InferRange(RangeAnalysis* analysis, Range* range) {
const Range* value_range = value()->definition()->range();
if (!Range::IsUnknown(value_range)) {
if (Range::IsUnknown(value_range)) {
*range = Range::Full(from_representation());
} else {
ASSERT_VALID_RANGE_FOR_REPRESENTATION(value()->definition(), value_range,
from_representation());
*range = *value_range;
}
}
void UnboxInt32Instr::InferRange(RangeAnalysis* analysis, Range* range) {
if (value()->Type()->ToCid() == kSmiCid) {
const Range* value_range = analysis->GetSmiRange(value());
if (!Range::IsUnknown(value_range)) {
*range = *value_range;
}
} else if (RangeAnalysis::IsIntegerDefinition(value()->definition())) {
const Range* value_range = analysis->GetIntRange(value());
if (!Range::IsUnknown(value_range)) {
*range = *value_range;
}
} else if (value()->Type()->ToCid() == kSmiCid) {
*range = Range::Full(RangeBoundary::kRangeBoundarySmi);
} else {
*range = Range::Full(RangeBoundary::kRangeBoundaryInt32);
}
}
void UnboxIntegerInstr::InferRange(RangeAnalysis* analysis, Range* range) {
auto* const value_range = value()->Type()->ToCid() == kSmiCid
? analysis->GetSmiRange(value())
: value()->definition()->range();
const Range to_range = Range::Full(representation());
void UnboxUint32Instr::InferRange(RangeAnalysis* analysis, Range* range) {
const Range* value_range = nullptr;
if (value()->Type()->ToCid() == kSmiCid) {
value_range = analysis->GetSmiRange(value());
} else if (RangeAnalysis::IsIntegerDefinition(value()->definition())) {
value_range = analysis->GetIntRange(value());
} else {
*range = Range(RangeBoundary::FromConstant(0),
RangeBoundary::FromConstant(kMaxUint32));
return;
}
if (!Range::IsUnknown(value_range)) {
if (value_range->IsPositive()) {
*range = *value_range;
} else {
*range = Range(RangeBoundary::FromConstant(0),
RangeBoundary::FromConstant(kMaxUint32));
}
}
}
void UnboxInt64Instr::InferRange(RangeAnalysis* analysis, Range* range) {
const Range* value_range = value()->definition()->range();
if (value_range != nullptr) {
if (Range::IsUnknown(value_range)) {
*range = to_range;
} else if (value_range->IsWithin(&to_range)) {
*range = *value_range;
} else if (!value()->definition()->IsInt64Definition() &&
(value()->definition()->Type()->ToCid() != kSmiCid)) {
*range = Range::Full(RangeBoundary::kRangeBoundaryInt64);
} else if (is_truncating()) {
// If truncating, then in most cases any non-representable values means
// no assumption can be made about the truncated value.
*range = to_range;
} else {
// When not truncating, then unboxing deoptimizes if the value is outside
// the range representation.
*range = value_range->Intersect(&to_range);
}
ASSERT_VALID_RANGE_FOR_REPRESENTATION(this, range, representation());
}
void IntConverterInstr::InferRange(RangeAnalysis* analysis, Range* range) {
@ -3140,13 +3163,14 @@ void IntConverterInstr::InferRange(RangeAnalysis* analysis, Range* range) {
ASSERT(RepresentationUtils::IsUnboxedInteger(to()));
ASSERT(from() == kUntagged || RepresentationUtils::IsUnboxedInteger(from()));
auto* const value_range = value()->definition()->range();
const Range* const value_range = value()->definition()->range();
const Range to_range = Range::Full(to());
if (from() == kUntagged) {
ASSERT(value_range == nullptr); // Not an integer-valued definition.
*range = Range::Full(to());
*range = to_range;
} else if (Range::IsUnknown(value_range)) {
*range = Range::Full(to());
*range = to_range;
} else if (RepresentationUtils::ValueSize(to()) >
RepresentationUtils::ValueSize(from()) &&
(!RepresentationUtils::IsUnsigned(to()) ||
@ -3161,29 +3185,21 @@ void IntConverterInstr::InferRange(RangeAnalysis* analysis, Range* range) {
// are the same size) or a larger value is being truncated. That means
// we need to determine whether or not the value range lies within the
// range of numbers that have the same representation (modulo truncation).
const int64_t min_overlap =
Utils::Maximum(RepresentationUtils::MinValue(from()),
RepresentationUtils::MinValue(to()));
const int64_t max_overlap =
Utils::Minimum(RepresentationUtils::MaxValue(from()),
RepresentationUtils::MaxValue(to()));
if (value_range->IsWithin(min_overlap, max_overlap)) {
const Range common_range = Range::Full(from()).Intersect(&to_range);
if (value_range->IsWithin(&common_range)) {
*range = *value_range;
} else {
// In most cases, if there are non-representable values, then no
// assumptions can be made about the converted value.
*range = Range::Full(to());
*range = to_range;
}
} else {
// The conversion deoptimizes if the value is outside the range represented
// by to(), so we can just take the intersection.
const auto& to_range = Range::Full(to());
*range = value_range->Intersect(&to_range);
}
ASSERT(RangeUtils::IsWithin(range, RepresentationUtils::MinValue(to()),
RepresentationUtils::MaxValue(to())));
ASSERT_VALID_RANGE_FOR_REPRESENTATION(this, range, to());
}
void AssertAssignableInstr::InferRange(RangeAnalysis* analysis, Range* range) {

View file

@ -428,6 +428,9 @@ class Range : public ZoneAllocated {
// Inclusive.
bool IsWithin(int64_t min_int, int64_t max_int) const;
// Inclusive.
bool IsWithin(const Range* other) const;
// Inclusive.
bool Overlaps(int64_t min_int, int64_t max_int) const;
@ -560,10 +563,14 @@ class RangeUtils : public AllStatic {
return !Range::IsUnknown(range) && range->Fits(size);
}
static bool IsWithin(Range* range, int64_t min, int64_t max) {
static bool IsWithin(const Range* range, int64_t min, int64_t max) {
return !Range::IsUnknown(range) && range->IsWithin(min, max);
}
static bool IsWithin(const Range* range, const Range* other) {
return !Range::IsUnknown(range) && range->IsWithin(other);
}
static bool IsPositive(Range* range) {
return !Range::IsUnknown(range) && range->IsPositive();
}