Skip to content

V
V8
Commit 6ddd8314 authored by neis's avatar neis Committed by Commit bot
Browse files
Options

[compiler] Deal with some old TODOs in the typer. 

This is mostly about DCHECKs. Enabling some requires a few
changes to tests that were not careful about types.

BUG=

Review-Url: https://codereview.chromium.org/2033703002
Cr-Commit-Position: refs/heads/master@{#36734}
parent 9dcb67dd
Show whitespace changes
Inline Side-by-side
Showing with 33 additions and 103 deletions
src/compiler/typer.cc
View file @ 6ddd8314
......@@ -43,8 +43,8 @@ Typer::Typer(Isolate* isolate, Graph* graph, Flags flags,
Type* infinity = Type::Constant(factory->infinity_value(), zone);
Type* minus_infinity = Type::Constant(factory->minus_infinity_value(), zone);
// TODO(neis): Unfortunately, the infinities created in other places might
// be different ones (eg the result of NewNumber in TypeNumberConstant).
// Unfortunately, the infinities created in other places might be different
// ones (eg the result of NewNumber in TypeNumberConstant).
Type* truncating_to_zero =
Type::Union(Type::Union(infinity, minus_infinity, zone),
Type::MinusZeroOrNaN(), zone);
......@@ -534,7 +534,6 @@ Type* Typer::Visitor::NumberTrunc(Type* type, Typer* t) {
}
Type* Typer::Visitor::NumberToInt32(Type* type, Typer* t) {
// TODO(neis): DCHECK(type->Is(Type::Number()));
if (type->Is(Type::Signed32())) return type;
if (type->Is(t->cache_.kZeroish)) return t->cache_.kSingletonZero;
if (type->Is(t->signed32ish_)) {
......@@ -547,7 +546,6 @@ Type* Typer::Visitor::NumberToInt32(Type* type, Typer* t) {
Type* Typer::Visitor::NumberToUint32(Type* type, Typer* t) {
// TODO(neis): DCHECK(type->Is(Type::Number()));
if (type->Is(Type::Unsigned32())) return type;
if (type->Is(t->cache_.kZeroish)) return t->cache_.kSingletonZero;
if (type->Is(t->unsigned32ish_)) {
......@@ -766,7 +764,6 @@ Type* Typer::Visitor::JSEqualTyper(Type* lhs, Type* rhs, Typer* t) {
if (lhs->IsConstant() && rhs->Is(lhs)) {
// Types are equal and are inhabited only by a single semantic value,
// which is not nan due to the earlier check.
// TODO(neis): Extend this to Range(x,x), MinusZero, ...?
return t->singleton_true_;
}
return Type::Boolean();
......@@ -910,7 +907,6 @@ Type* Typer::Visitor::JSBitwiseOrTyper(Type* lhs, Type* rhs, Typer* t) {
max = std::min(max, -1.0);
}
return Type::Range(min, max, t->zone());
// TODO(neis): Be precise for singleton inputs, here and elsewhere.
}
......@@ -1085,7 +1081,6 @@ Type* Typer::Visitor::JSAddTyper(Type* lhs, Type* rhs, Typer* t) {
if (lhs->IsRange() && rhs->IsRange()) {
return JSAddRanger(lhs->AsRange(), rhs->AsRange(), t);
}
// TODO(neis): Deal with numeric bitsets here and elsewhere.
return Type::Number();
}
......@@ -1169,7 +1164,6 @@ Type* Typer::Visitor::JSDivideTyper(Type* lhs, Type* rhs, Typer* t) {
rhs = ToNumber(rhs, t);
if (lhs->Is(Type::NaN()) || rhs->Is(Type::NaN())) return Type::NaN();
// Division is tricky, so all we do is try ruling out nan.
// TODO(neis): try ruling out -0 as well?
bool maybe_nan =
lhs->Maybe(Type::NaN()) || rhs->Maybe(t->cache_.kZeroish) ||
((lhs->Min() == -V8_INFINITY || lhs->Max() == +V8_INFINITY) &&
......@@ -1856,39 +1850,43 @@ Type* ChangeRepresentation(Type* type, Type* rep, Zone* zone) {
Type* Typer::Visitor::TypeChangeTaggedSignedToInt32(Node* node) {
Type* arg = Operand(node, 0);
// TODO(neis): DCHECK(arg->Is(Type::Signed32()));
// TODO(jarin): DCHECK(arg->Is(Type::Signed32()));
// Many tests fail this check.
return ChangeRepresentation(arg, Type::UntaggedIntegral32(), zone());
}
Type* Typer::Visitor::TypeChangeTaggedToInt32(Node* node) {
Type* arg = Operand(node, 0);
// TODO(neis): DCHECK(arg->Is(Type::Signed32()));
DCHECK(arg->Is(Type::Signed32()));
return ChangeRepresentation(arg, Type::UntaggedIntegral32(), zone());
}
Type* Typer::Visitor::TypeChangeTaggedToUint32(Node* node) {
Type* arg = Operand(node, 0);
// TODO(neis): DCHECK(arg->Is(Type::Unsigned32()));
DCHECK(arg->Is(Type::Unsigned32()));
return ChangeRepresentation(arg, Type::UntaggedIntegral32(), zone());
}
Type* Typer::Visitor::TypeChangeTaggedToFloat64(Node* node) {
Type* arg = Operand(node, 0);
// TODO(neis): DCHECK(arg->Is(Type::Number()));
DCHECK(arg->Is(Type::Number()));
return ChangeRepresentation(arg, Type::UntaggedFloat64(), zone());
}
Type* Typer::Visitor::TypeTruncateTaggedToFloat64(Node* node) {
Type* arg = Operand(node, 0);
// TODO(neis): DCHECK(arg->Is(Type::NumberOrUndefined()));
DCHECK(arg->Is(Type::NumberOrUndefined()));
return ChangeRepresentation(arg, Type::UntaggedFloat64(), zone());
}
Type* Typer::Visitor::TypeChangeInt31ToTaggedSigned(Node* node) {
Type* arg = Operand(node, 0);
// TODO(neis): DCHECK(arg->Is(Type::Signed31()));
// TODO(jarin): DCHECK(arg->Is(Type::Signed31()));
// Some mjsunit/asm and mjsunit/wasm tests fail this check.
// For instance, asm/int32-umod fails with Signed32/UntaggedIntegral32 in
// simplified-lowering (after propagation).
Type* rep =
arg->Is(Type::SignedSmall()) ? Type::TaggedSigned() : Type::Tagged();
return ChangeRepresentation(arg, rep, zone());
......@@ -1896,35 +1894,38 @@ Type* Typer::Visitor::TypeChangeInt31ToTaggedSigned(Node* node) {
Type* Typer::Visitor::TypeChangeInt32ToTagged(Node* node) {
Type* arg = Operand(node, 0);
// TODO(neis): DCHECK(arg->Is(Type::Signed32()));
// TODO(jarin): DCHECK(arg->Is(Type::Signed32()));
// Two tests fail this check: mjsunit/asm/sqlite3/sqlite-safe-heap and
// mjsunit/wasm/embenchen/lua_binarytrees. The first one fails with Any/Any in
// simplified-lowering (after propagation).
Type* rep =
arg->Is(Type::SignedSmall()) ? Type::TaggedSigned() : Type::Tagged();
return ChangeRepresentation(arg, rep, zone());
}
Type* Typer::Visitor::TypeChangeUint32ToTagged(Node* node) {
Type* arg = Operand(node, 0);
// TODO(neis): DCHECK(arg->Is(Type::Unsigned32()));
// TODO(jarin): DCHECK(arg->Is(Type::Unsigned32()));
// This fails in benchmarks/octane/mandreel (--turbo).
return ChangeRepresentation(arg, Type::Tagged(), zone());
}
Type* Typer::Visitor::TypeChangeFloat64ToTagged(Node* node) {
Type* arg = Operand(node, 0);
// TODO(neis): CHECK(arg.upper->Is(Type::Number()));
// TODO(jarin): DCHECK(arg->Is(Type::Number()));
// Some (or all) mjsunit/wasm/embenchen/ tests fail this check when run with
// --turbo and --always-opt.
return ChangeRepresentation(arg, Type::Tagged(), zone());
}
Type* Typer::Visitor::TypeChangeTaggedToBit(Node* node) {
Type* arg = Operand(node, 0);
// TODO(neis): DCHECK(arg.upper->Is(Type::Boolean()));
DCHECK(arg->Is(Type::Boolean()));
return ChangeRepresentation(arg, Type::UntaggedBit(), zone());
}
Type* Typer::Visitor::TypeChangeBitToTagged(Node* node) {
Type* arg = Operand(node, 0);
// TODO(neis): DCHECK(arg.upper->Is(Type::Boolean()));
return ChangeRepresentation(arg, Type::TaggedPointer(), zone());
}
......@@ -1946,7 +1947,10 @@ Type* Typer::Visitor::TypeCheckedTaggedToFloat64(Node* node) {
Type* Typer::Visitor::TypeTruncateTaggedToWord32(Node* node) {
Type* arg = Operand(node, 0);
// TODO(neis): DCHECK(arg->Is(Type::Number()));
// TODO(jarin): DCHECK(arg->Is(Type::NumberOrUndefined()));
// Several mjsunit and cctest tests fail this check. For instance,
// mjsunit/compiler/regress-607493 fails with Any/Any in simplified-lowering
// (after propagation).
return ChangeRepresentation(arg, Type::UntaggedIntegral32(), zone());
}
......
test/cctest/compiler/test-simplified-lowering.cc
View file @ 6ddd8314
......@@ -1167,7 +1167,7 @@ TEST(InsertBasicChanges) {
Type::Unsigned32());
CheckChangeInsertion(IrOpcode::kChangeFloat64ToTagged, MachineType::Float64(),
MachineType::AnyTagged());
MachineType::AnyTagged(), Type::Number());
CheckChangeInsertion(IrOpcode::kChangeTaggedToFloat64,
MachineType::AnyTagged(), MachineType::Float64(),
Type::Number());
......@@ -1258,7 +1258,7 @@ TEST(InsertChangesAroundFloat64Binops) {
for (size_t i = 0; i < arraysize(ops); i++) {
CheckChangesAroundBinop(&t, ops[i], IrOpcode::kChangeTaggedToFloat64,
IrOpcode::kChangeFloat64ToTagged);
IrOpcode::kChangeFloat64ToTagged, Type::Number());
}
}
......@@ -1488,8 +1488,9 @@ TEST(InsertChangeForStoreElementIndex) {
TEST(InsertChangeForLoadElement) {
// TODO(titzer): test all load/store representation change insertions.
TestingGraph t(Type::Any(), Type::Signed32(), Type::Any());
ElementAccess access = {kTaggedBase, FixedArrayBase::kHeaderSize, Type::Any(),
MachineType::Float64(), kNoWriteBarrier};
ElementAccess access = {kTaggedBase, FixedArrayBase::kHeaderSize,
Type::Number(), MachineType::Float64(),
kNoWriteBarrier};
Node* load = t.graph()->NewNode(t.simplified()->LoadElement(access), t.p0,
t.p1, t.start, t.start);
......@@ -1506,7 +1507,7 @@ TEST(InsertChangeForLoadField) {
TestingGraph t(Type::Any(), Type::Signed32());
FieldAccess access = {
kTaggedBase, FixedArrayBase::kHeaderSize, Handle<Name>::null(),
Type::Any(), MachineType::Float64(), kNoWriteBarrier};
Type::Number(), MachineType::Float64(), kNoWriteBarrier};
Node* load = t.graph()->NewNode(t.simplified()->LoadField(access), t.p0,
t.start, t.start);
......
test/unittests/compiler/simplified-operator-reducer-unittest.cc
View file @ 6ddd8314
......@@ -91,26 +91,6 @@ const int32_t kInt32Values[] = {
1866841746, 2032089723, 2147483647};
const uint32_t kUint32Values[] = {
0x0, 0x5, 0x8, 0xc, 0xd, 0x26,
0x28, 0x29, 0x30, 0x34, 0x3e, 0x42,
0x50, 0x5b, 0x63, 0x71, 0x77, 0x7c,
0x83, 0x88, 0x96, 0x9c, 0xa3, 0xfa,
0x7a7, 0x165d, 0x234d, 0x3acb, 0x43a5, 0x4573,
0x5b4f, 0x5f14, 0x6996, 0x6c6e, 0x7289, 0x7b9a,
0x7bc9, 0x86bb, 0xa839, 0xaa41, 0xb03b, 0xc942,
0xce68, 0xcf4c, 0xd3ad, 0xdea3, 0xe90c, 0xed86,
0xfba5, 0x172dcc6, 0x114d8fc1, 0x182d6c9d, 0x1b1e3fad, 0x1db033bf,
0x1e1de755, 0x1f625c80, 0x28f6cf00, 0x2acb6a94, 0x2c20240e, 0x2f0fe54e,
0x31863a7c, 0x33325474, 0x3532fae3, 0x3bab82ea, 0x4c4b83a2, 0x4cd93d1e,
0x4f7331d4, 0x5491b09b, 0x57cc6ff9, 0x60d3b4dc, 0x653f5904, 0x690ae256,
0x69fe3276, 0x6bebf0ba, 0x6e2c69a3, 0x73b84ff7, 0x7b3a1924, 0x7ed032d9,
0x84dd734b, 0x8552ea53, 0x8680754f, 0x8e9660eb, 0x94fe2b9c, 0x972d30cf,
0x9b98c482, 0xb158667e, 0xb432932c, 0xb5b70989, 0xb669971a, 0xb7c359d1,
0xbeb15c0d, 0xc171c53d, 0xc743dd38, 0xc8e2af50, 0xc98e2df0, 0xd9d1cdf9,
0xdcc91049, 0xe46f396d, 0xee991950, 0xef64e521, 0xf7aeefc9, 0xffffffff};
const double kNaNs[] = {-std::numeric_limits<double>::quiet_NaN(),
std::numeric_limits<double>::quiet_NaN(),
bit_cast<double>(V8_UINT64_C(0x7FFFFFFFFFFFFFFF)),
......@@ -314,26 +294,6 @@ TEST_F(SimplifiedOperatorReducerTest,
}
TEST_F(SimplifiedOperatorReducerTest, ChangeTaggedToInt32WithConstant) {
TRACED_FOREACH(double, n, kFloat64Values) {
Reduction reduction = Reduce(graph()->NewNode(
simplified()->ChangeTaggedToInt32(), NumberConstant(n)));
ASSERT_TRUE(reduction.Changed());
EXPECT_THAT(reduction.replacement(), IsInt32Constant(DoubleToInt32(n)));
}
}
TEST_F(SimplifiedOperatorReducerTest, ChangeTaggedToInt32WithNaNConstant) {
TRACED_FOREACH(double, nan, kNaNs) {
Reduction reduction = Reduce(graph()->NewNode(
simplified()->ChangeTaggedToInt32(), NumberConstant(nan)));
ASSERT_TRUE(reduction.Changed());
EXPECT_THAT(reduction.replacement(), IsInt32Constant(0));
}
}
// -----------------------------------------------------------------------------
// ChangeTaggedToUint32
......@@ -360,41 +320,6 @@ TEST_F(SimplifiedOperatorReducerTest,
}
TEST_F(SimplifiedOperatorReducerTest, ChangeTaggedToUint32WithConstant) {
TRACED_FOREACH(double, n, kFloat64Values) {
Reduction reduction = Reduce(graph()->NewNode(
simplified()->ChangeTaggedToUint32(), NumberConstant(n)));
ASSERT_TRUE(reduction.Changed());
EXPECT_THAT(reduction.replacement(),
IsInt32Constant(bit_cast<int32_t>(DoubleToUint32(n))));
}
}
TEST_F(SimplifiedOperatorReducerTest, ChangeTaggedToUint32WithNaNConstant) {
TRACED_FOREACH(double, nan, kNaNs) {
Reduction reduction = Reduce(graph()->NewNode(
simplified()->ChangeTaggedToUint32(), NumberConstant(nan)));
ASSERT_TRUE(reduction.Changed());
EXPECT_THAT(reduction.replacement(), IsInt32Constant(0));
}
}
// -----------------------------------------------------------------------------
// ChangeUint32ToTagged
TEST_F(SimplifiedOperatorReducerTest, ChangeUint32ToTagged) {
TRACED_FOREACH(uint32_t, n, kUint32Values) {
Reduction reduction =
Reduce(graph()->NewNode(simplified()->ChangeUint32ToTagged(),
Int32Constant(bit_cast<int32_t>(n))));
ASSERT_TRUE(reduction.Changed());
EXPECT_THAT(reduction.replacement(), IsNumberConstant(BitEq(FastUI2D(n))));
}
}
// -----------------------------------------------------------------------------
// TruncateTaggedToWord32
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment