[turbofan] Only use Tagged machine representation for tagged state values.
This avoids using kTaggedSigned and kTaggedPointer because the semantic information of those type could be invalid in unreachable code. For example, SmiCheck(0.1) has representation TaggedSigned, but it is later compiled to DeoptimizeUnless(ObjectIsSmi(0.1)) with the constant 0.1 directly connected to the uses. If the use is state-values, which recorded the TaggedSigned representation of CheckSmi, the code generator will be confused because it will see constant 0.1 that claims to be TaggedSigned value. BUG=chromium:675704 Review-Url: https://codereview.chromium.org/2656243004 Cr-Commit-Position: refs/heads/master@{#42756}
This commit is contained in:
parent
e342ec90f0
commit
6cd2d4ba41
@ -867,16 +867,15 @@ void CodeGenerator::AddTranslationForOperand(Translation* translation,
|
||||
} else if (type == MachineType::Uint8() || type == MachineType::Uint16() ||
|
||||
type == MachineType::Uint32()) {
|
||||
translation->StoreUint32StackSlot(LocationOperand::cast(op)->index());
|
||||
} else if (IsAnyTagged(type.representation())) {
|
||||
translation->StoreStackSlot(LocationOperand::cast(op)->index());
|
||||
} else {
|
||||
CHECK(false);
|
||||
CHECK_EQ(MachineRepresentation::kTagged, type.representation());
|
||||
translation->StoreStackSlot(LocationOperand::cast(op)->index());
|
||||
}
|
||||
} else if (op->IsFPStackSlot()) {
|
||||
if (type.representation() == MachineRepresentation::kFloat64) {
|
||||
translation->StoreDoubleStackSlot(LocationOperand::cast(op)->index());
|
||||
} else {
|
||||
DCHECK_EQ(MachineRepresentation::kFloat32, type.representation());
|
||||
CHECK_EQ(MachineRepresentation::kFloat32, type.representation());
|
||||
translation->StoreFloatStackSlot(LocationOperand::cast(op)->index());
|
||||
}
|
||||
} else if (op->IsRegister()) {
|
||||
@ -889,27 +888,26 @@ void CodeGenerator::AddTranslationForOperand(Translation* translation,
|
||||
} else if (type == MachineType::Uint8() || type == MachineType::Uint16() ||
|
||||
type == MachineType::Uint32()) {
|
||||
translation->StoreUint32Register(converter.ToRegister(op));
|
||||
} else if (IsAnyTagged(type.representation())) {
|
||||
translation->StoreRegister(converter.ToRegister(op));
|
||||
} else {
|
||||
CHECK(false);
|
||||
CHECK_EQ(MachineRepresentation::kTagged, type.representation());
|
||||
translation->StoreRegister(converter.ToRegister(op));
|
||||
}
|
||||
} else if (op->IsFPRegister()) {
|
||||
InstructionOperandConverter converter(this, instr);
|
||||
if (type.representation() == MachineRepresentation::kFloat64) {
|
||||
translation->StoreDoubleRegister(converter.ToDoubleRegister(op));
|
||||
} else {
|
||||
DCHECK_EQ(MachineRepresentation::kFloat32, type.representation());
|
||||
CHECK_EQ(MachineRepresentation::kFloat32, type.representation());
|
||||
translation->StoreFloatRegister(converter.ToFloatRegister(op));
|
||||
}
|
||||
} else if (op->IsImmediate()) {
|
||||
} else {
|
||||
CHECK(op->IsImmediate());
|
||||
InstructionOperandConverter converter(this, instr);
|
||||
Constant constant = converter.ToConstant(op);
|
||||
Handle<Object> constant_object;
|
||||
switch (constant.type()) {
|
||||
case Constant::kInt32:
|
||||
if (type.representation() == MachineRepresentation::kTagged ||
|
||||
type.representation() == MachineRepresentation::kTaggedSigned) {
|
||||
if (type.representation() == MachineRepresentation::kTagged) {
|
||||
// When pointers are 4 bytes, we can use int32 constants to represent
|
||||
// Smis.
|
||||
DCHECK_EQ(4, kPointerSize);
|
||||
@ -947,37 +945,28 @@ void CodeGenerator::AddTranslationForOperand(Translation* translation,
|
||||
// TODO(jarin,bmeurer): We currently pass in raw pointers to the
|
||||
// JSFunction::entry here. We should really consider fixing this.
|
||||
DCHECK(type.representation() == MachineRepresentation::kWord64 ||
|
||||
type.representation() == MachineRepresentation::kTagged ||
|
||||
type.representation() == MachineRepresentation::kTaggedSigned);
|
||||
type.representation() == MachineRepresentation::kTagged);
|
||||
DCHECK_EQ(8, kPointerSize);
|
||||
constant_object =
|
||||
handle(reinterpret_cast<Smi*>(constant.ToInt64()), isolate());
|
||||
DCHECK(constant_object->IsSmi());
|
||||
break;
|
||||
case Constant::kFloat32:
|
||||
if (type.representation() == MachineRepresentation::kTaggedSigned) {
|
||||
DCHECK(IsSmiDouble(constant.ToFloat32()));
|
||||
} else {
|
||||
DCHECK(type.representation() == MachineRepresentation::kFloat32 ||
|
||||
CanBeTaggedPointer(type.representation()));
|
||||
}
|
||||
DCHECK(type.representation() == MachineRepresentation::kFloat32 ||
|
||||
type.representation() == MachineRepresentation::kTagged);
|
||||
constant_object = isolate()->factory()->NewNumber(constant.ToFloat32());
|
||||
break;
|
||||
case Constant::kFloat64:
|
||||
if (type.representation() == MachineRepresentation::kTaggedSigned) {
|
||||
DCHECK(IsSmiDouble(constant.ToFloat64()));
|
||||
} else {
|
||||
DCHECK(type.representation() == MachineRepresentation::kFloat64 ||
|
||||
CanBeTaggedPointer(type.representation()));
|
||||
}
|
||||
DCHECK(type.representation() == MachineRepresentation::kFloat64 ||
|
||||
type.representation() == MachineRepresentation::kTagged);
|
||||
constant_object = isolate()->factory()->NewNumber(constant.ToFloat64());
|
||||
break;
|
||||
case Constant::kHeapObject:
|
||||
DCHECK(CanBeTaggedPointer(type.representation()));
|
||||
DCHECK_EQ(MachineRepresentation::kTagged, type.representation());
|
||||
constant_object = constant.ToHeapObject();
|
||||
break;
|
||||
default:
|
||||
CHECK(false);
|
||||
UNREACHABLE();
|
||||
}
|
||||
if (constant_object.is_identical_to(info()->closure())) {
|
||||
translation->StoreJSFrameFunction();
|
||||
@ -985,8 +974,6 @@ void CodeGenerator::AddTranslationForOperand(Translation* translation,
|
||||
int literal_id = DefineDeoptimizationLiteral(constant_object);
|
||||
translation->StoreLiteral(literal_id);
|
||||
}
|
||||
} else {
|
||||
CHECK(false);
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -980,7 +980,7 @@ class RepresentationSelector {
|
||||
}
|
||||
}
|
||||
|
||||
MachineSemantic DeoptValueSemanticOf(Type* type) {
|
||||
static MachineSemantic DeoptValueSemanticOf(Type* type) {
|
||||
// We only need signedness to do deopt correctly.
|
||||
if (type->Is(Type::Signed32())) {
|
||||
return MachineSemantic::kInt32;
|
||||
@ -991,6 +991,29 @@ class RepresentationSelector {
|
||||
}
|
||||
}
|
||||
|
||||
static MachineType DeoptMachineTypeOf(MachineRepresentation rep, Type* type) {
|
||||
if (!type->IsInhabited()) {
|
||||
return MachineType::None();
|
||||
}
|
||||
// TODO(turbofan): Special treatment for ExternalPointer here,
|
||||
// to avoid incompatible truncations. We really need a story
|
||||
// for the JSFunction::entry field.
|
||||
if (type->Is(Type::ExternalPointer())) {
|
||||
return MachineType::Pointer();
|
||||
}
|
||||
// Do not distinguish between various Tagged variations.
|
||||
if (IsAnyTagged(rep)) {
|
||||
return MachineType::AnyTagged();
|
||||
}
|
||||
MachineType machine_type(rep, DeoptValueSemanticOf(type));
|
||||
DCHECK(machine_type.representation() != MachineRepresentation::kWord32 ||
|
||||
machine_type.semantic() == MachineSemantic::kInt32 ||
|
||||
machine_type.semantic() == MachineSemantic::kUint32);
|
||||
DCHECK(machine_type.representation() != MachineRepresentation::kBit ||
|
||||
type->Is(Type::Boolean()));
|
||||
return machine_type;
|
||||
}
|
||||
|
||||
void VisitStateValues(Node* node) {
|
||||
if (propagate()) {
|
||||
for (int i = 0; i < node->InputCount(); i++) {
|
||||
@ -1003,17 +1026,8 @@ class RepresentationSelector {
|
||||
ZoneVector<MachineType>(node->InputCount(), zone);
|
||||
for (int i = 0; i < node->InputCount(); i++) {
|
||||
Node* input = node->InputAt(i);
|
||||
NodeInfo* input_info = GetInfo(input);
|
||||
Type* input_type = TypeOf(input);
|
||||
MachineRepresentation rep = input_type->IsInhabited()
|
||||
? input_info->representation()
|
||||
: MachineRepresentation::kNone;
|
||||
MachineType machine_type(rep, DeoptValueSemanticOf(input_type));
|
||||
DCHECK(machine_type.representation() !=
|
||||
MachineRepresentation::kWord32 ||
|
||||
machine_type.semantic() == MachineSemantic::kInt32 ||
|
||||
machine_type.semantic() == MachineSemantic::kUint32);
|
||||
(*types)[i] = machine_type;
|
||||
(*types)[i] =
|
||||
DeoptMachineTypeOf(GetInfo(input)->representation(), TypeOf(input));
|
||||
}
|
||||
SparseInputMask mask = SparseInputMaskOf(node->op());
|
||||
NodeProperties::ChangeOp(
|
||||
@ -1047,28 +1061,8 @@ class RepresentationSelector {
|
||||
ZoneVector<MachineType>(node->InputCount(), zone);
|
||||
for (int i = 0; i < node->InputCount(); i++) {
|
||||
Node* input = node->InputAt(i);
|
||||
NodeInfo* input_info = GetInfo(input);
|
||||
Type* input_type = TypeOf(input);
|
||||
// TODO(turbofan): Special treatment for ExternalPointer here,
|
||||
// to avoid incompatible truncations. We really need a story
|
||||
// for the JSFunction::entry field.
|
||||
if (!input_type->IsInhabited()) {
|
||||
(*types)[i] = MachineType::None();
|
||||
} else if (input_type->Is(Type::ExternalPointer())) {
|
||||
(*types)[i] = MachineType::Pointer();
|
||||
} else {
|
||||
MachineRepresentation rep = input_type->IsInhabited()
|
||||
? input_info->representation()
|
||||
: MachineRepresentation::kNone;
|
||||
MachineType machine_type(rep, DeoptValueSemanticOf(input_type));
|
||||
DCHECK(machine_type.representation() !=
|
||||
MachineRepresentation::kWord32 ||
|
||||
machine_type.semantic() == MachineSemantic::kInt32 ||
|
||||
machine_type.semantic() == MachineSemantic::kUint32);
|
||||
DCHECK(machine_type.representation() != MachineRepresentation::kBit ||
|
||||
input_type->Is(Type::Boolean()));
|
||||
(*types)[i] = machine_type;
|
||||
}
|
||||
(*types)[i] =
|
||||
DeoptMachineTypeOf(GetInfo(input)->representation(), TypeOf(input));
|
||||
}
|
||||
NodeProperties::ChangeOp(node,
|
||||
jsgraph_->common()->TypedObjectState(types));
|
||||
|
26
test/mjsunit/compiler/regress-675704.js
Normal file
26
test/mjsunit/compiler/regress-675704.js
Normal file
@ -0,0 +1,26 @@
|
||||
// Copyright 2017 the V8 project authors. All rights reserved.
|
||||
// Use of this source code is governed by a BSD-style license that can be
|
||||
// found in the LICENSE file.
|
||||
|
||||
// Flags: --allow-natives-syntax
|
||||
|
||||
function foo(a) {
|
||||
this.a = a;
|
||||
// Note that any call would do, it doesn't need to be %MaxSmi()
|
||||
this.x = this.a + %MaxSmi();
|
||||
}
|
||||
|
||||
function g(x) {
|
||||
new foo(2);
|
||||
|
||||
if (x) {
|
||||
for (var i = 0.1; i < 1.1; i++) {
|
||||
new foo(i);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
g(false);
|
||||
g(false);
|
||||
%OptimizeFunctionOnNextCall(g);
|
||||
g(true);
|
Loading…
Reference in New Issue
Block a user