[compiler] Fix ArgumentsLength confusing semantics in EscapeAnalysisReducer

Change-Id: I41be2c5b0867739dbbe3667144bf6b479c609e53
Bug: chromium:1107221
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2322628
Commit-Queue: Victor Gomes <victorgomes@chromium.org>
Auto-Submit: Victor Gomes <victorgomes@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69122}
This commit is contained in:
Victor Gomes 2020-07-29 12:27:51 +02:00 committed by Commit Bot
parent e5281ef477
commit 73dc8faed2
18 changed files with 131 additions and 152 deletions

View File

@ -1040,7 +1040,7 @@ void CodeGenerator::TranslateStateValueDescriptor(
}
} else if (desc->IsArgumentsLength()) {
if (translation != nullptr) {
translation->ArgumentsLength(desc->arguments_type());
translation->ArgumentsLength();
}
} else if (desc->IsDuplicate()) {
if (translation != nullptr) {

View File

@ -588,7 +588,7 @@ size_t InstructionSelector::AddOperandToStateValueDescriptor(
return 0;
}
case IrOpcode::kArgumentsLengthState: {
values->PushArgumentsLength(ArgumentsStateTypeOf(input->op()));
values->PushArgumentsLength();
return 0;
}
case IrOpcode::kObjectState: {

View File

@ -1135,11 +1135,9 @@ class StateValueDescriptor {
descr.args_type_ = type;
return descr;
}
static StateValueDescriptor ArgumentsLength(ArgumentsStateType type) {
StateValueDescriptor descr(StateValueKind::kArgumentsLength,
MachineType::AnyTagged());
descr.args_type_ = type;
return descr;
static StateValueDescriptor ArgumentsLength() {
return StateValueDescriptor(StateValueKind::kArgumentsLength,
MachineType::AnyTagged());
}
static StateValueDescriptor Plain(MachineType type) {
return StateValueDescriptor(StateValueKind::kPlain, type);
@ -1178,8 +1176,7 @@ class StateValueDescriptor {
return id_;
}
ArgumentsStateType arguments_type() const {
DCHECK(kind_ == StateValueKind::kArgumentsElements ||
kind_ == StateValueKind::kArgumentsLength);
DCHECK(kind_ == StateValueKind::kArgumentsElements);
return args_type_;
}
@ -1253,8 +1250,8 @@ class StateValueList {
void PushArgumentsElements(ArgumentsStateType type) {
fields_.push_back(StateValueDescriptor::ArgumentsElements(type));
}
void PushArgumentsLength(ArgumentsStateType type) {
fields_.push_back(StateValueDescriptor::ArgumentsLength(type));
void PushArgumentsLength() {
fields_.push_back(StateValueDescriptor::ArgumentsLength());
}
void PushDuplicate(size_t id) {
fields_.push_back(StateValueDescriptor::Duplicate(id));

View File

@ -1424,18 +1424,15 @@ const Operator* CommonOperatorBuilder::ArgumentsElementsState(
type); // parameter
}
const Operator* CommonOperatorBuilder::ArgumentsLengthState(
ArgumentsStateType type) {
return zone()->New<Operator1<ArgumentsStateType>>( // --
const Operator* CommonOperatorBuilder::ArgumentsLengthState() {
return zone()->New<Operator>( // --
IrOpcode::kArgumentsLengthState, Operator::kPure, // opcode
"ArgumentsLengthState", // name
0, 0, 0, 1, 0, 0, // counts
type); // parameter
0, 0, 0, 1, 0, 0); // counts
}
ArgumentsStateType ArgumentsStateTypeOf(Operator const* op) {
DCHECK(op->opcode() == IrOpcode::kArgumentsElementsState ||
op->opcode() == IrOpcode::kArgumentsLengthState);
DCHECK(op->opcode() == IrOpcode::kArgumentsElementsState);
return OpParameter<ArgumentsStateType>(op);
}

View File

@ -528,7 +528,7 @@ class V8_EXPORT_PRIVATE CommonOperatorBuilder final
const Operator* TypedStateValues(const ZoneVector<MachineType>* types,
SparseInputMask bitmask);
const Operator* ArgumentsElementsState(ArgumentsStateType type);
const Operator* ArgumentsLengthState(ArgumentsStateType type);
const Operator* ArgumentsLengthState();
const Operator* ObjectState(uint32_t object_id, int pointer_slots);
const Operator* TypedObjectState(uint32_t object_id,
const ZoneVector<MachineType>* types);

View File

@ -146,6 +146,7 @@ class EffectControlLinearizer {
Node* LowerObjectIsSafeInteger(Node* node);
Node* LowerArgumentsFrame(Node* node);
Node* LowerArgumentsLength(Node* node);
Node* LowerRestLength(Node* node);
Node* LowerNewDoubleElements(Node* node);
Node* LowerNewSmiOrObjectElements(Node* node);
Node* LowerNewArgumentsElements(Node* node);
@ -1107,6 +1108,9 @@ bool EffectControlLinearizer::TryWireInStateEffect(Node* node,
case IrOpcode::kArgumentsLength:
result = LowerArgumentsLength(node);
break;
case IrOpcode::kRestLength:
result = LowerRestLength(node);
break;
case IrOpcode::kToBoolean:
result = LowerToBoolean(node);
break;
@ -3592,55 +3596,58 @@ Node* EffectControlLinearizer::LowerToBoolean(Node* node) {
Node* EffectControlLinearizer::LowerArgumentsLength(Node* node) {
Node* arguments_frame = NodeProperties::GetValueInput(node, 0);
int formal_parameter_count = FormalParameterCountOf(node->op());
bool is_rest_length = IsRestLengthOf(node->op());
DCHECK_LE(0, formal_parameter_count);
if (is_rest_length) {
// The ArgumentsLength node is computing the number of rest parameters,
// which is max(0, actual_parameter_count - formal_parameter_count).
// We have to distinguish the case, when there is an arguments adaptor frame
// (i.e., arguments_frame != LoadFramePointer()).
auto if_adaptor_frame = __ MakeLabel();
auto done = __ MakeLabel(MachineRepresentation::kTaggedSigned);
// The ArgumentsLength node is computing the actual number of arguments.
// We have to distinguish the case when there is an arguments adaptor frame
// (i.e., arguments_frame != LoadFramePointer()).
auto if_adaptor_frame = __ MakeLabel();
auto done = __ MakeLabel(MachineRepresentation::kTaggedSigned);
Node* frame = __ LoadFramePointer();
__ GotoIf(__ TaggedEqual(arguments_frame, frame), &done, __ SmiConstant(0));
__ Goto(&if_adaptor_frame);
Node* frame = __ LoadFramePointer();
__ GotoIf(__ TaggedEqual(arguments_frame, frame), &done,
__ SmiConstant(formal_parameter_count));
__ Goto(&if_adaptor_frame);
__ Bind(&if_adaptor_frame);
Node* arguments_length = __ BitcastWordToTaggedSigned(__ Load(
MachineType::Pointer(), arguments_frame,
__ IntPtrConstant(ArgumentsAdaptorFrameConstants::kLengthOffset)));
__ Bind(&if_adaptor_frame);
Node* arguments_length = __ BitcastWordToTaggedSigned(__ Load(
MachineType::Pointer(), arguments_frame,
__ IntPtrConstant(ArgumentsAdaptorFrameConstants::kLengthOffset)));
__ Goto(&done, arguments_length);
Node* rest_length =
__ SmiSub(arguments_length, __ SmiConstant(formal_parameter_count));
__ GotoIf(__ SmiLessThan(rest_length, __ SmiConstant(0)), &done,
__ SmiConstant(0));
__ Goto(&done, rest_length);
__ Bind(&done);
return done.PhiAt(0);
}
__ Bind(&done);
return done.PhiAt(0);
} else {
// The ArgumentsLength node is computing the actual number of arguments.
// We have to distinguish the case when there is an arguments adaptor frame
// (i.e., arguments_frame != LoadFramePointer()).
auto if_adaptor_frame = __ MakeLabel();
auto done = __ MakeLabel(MachineRepresentation::kTaggedSigned);
Node* EffectControlLinearizer::LowerRestLength(Node* node) {
Node* arguments_frame = NodeProperties::GetValueInput(node, 0);
int formal_parameter_count = FormalParameterCountOf(node->op());
DCHECK_LE(0, formal_parameter_count);
Node* frame = __ LoadFramePointer();
__ GotoIf(__ TaggedEqual(arguments_frame, frame), &done,
__ SmiConstant(formal_parameter_count));
__ Goto(&if_adaptor_frame);
// The RestLength node is computing the number of rest parameters,
// which is max(0, actual_parameter_count - formal_parameter_count).
// We have to distinguish the case, when there is an arguments adaptor frame
// (i.e., arguments_frame != LoadFramePointer()).
auto if_adaptor_frame = __ MakeLabel();
auto done = __ MakeLabel(MachineRepresentation::kTaggedSigned);
__ Bind(&if_adaptor_frame);
Node* arguments_length = __ BitcastWordToTaggedSigned(__ Load(
MachineType::Pointer(), arguments_frame,
__ IntPtrConstant(ArgumentsAdaptorFrameConstants::kLengthOffset)));
__ Goto(&done, arguments_length);
Node* frame = __ LoadFramePointer();
__ GotoIf(__ TaggedEqual(arguments_frame, frame), &done, __ SmiConstant(0));
__ Goto(&if_adaptor_frame);
__ Bind(&done);
return done.PhiAt(0);
}
__ Bind(&if_adaptor_frame);
Node* arguments_length = __ BitcastWordToTaggedSigned(__ Load(
MachineType::Pointer(), arguments_frame,
__ IntPtrConstant(ArgumentsAdaptorFrameConstants::kLengthOffset)));
Node* rest_length =
__ SmiSub(arguments_length, __ SmiConstant(formal_parameter_count));
__ GotoIf(__ SmiLessThan(rest_length, __ SmiConstant(0)), &done,
__ SmiConstant(0));
__ Goto(&done, rest_length);
__ Bind(&done);
return done.PhiAt(0);
}
Node* EffectControlLinearizer::LowerArgumentsFrame(Node* node) {

View File

@ -241,14 +241,6 @@ void EscapeAnalysisReducer::Finalize() {
Node* arguments_length = NodeProperties::GetValueInput(node, 1);
if (arguments_length->opcode() != IrOpcode::kArgumentsLength) continue;
// If mapped arguments are specified, then their number is always equal to
// the number of formal parameters. This allows to use just the three-value
// {ArgumentsStateType} enum because the deoptimizer can reconstruct the
// value of {mapped_count} from the number of formal parameters.
DCHECK_IMPLIES(
mapped_count != 0,
mapped_count == FormalParameterCountOf(arguments_length->op()));
Node* arguments_length_state = nullptr;
for (Edge edge : arguments_length->use_edges()) {
Node* use = edge.from();
@ -259,7 +251,7 @@ void EscapeAnalysisReducer::Finalize() {
case IrOpcode::kTypedStateValues:
if (!arguments_length_state) {
arguments_length_state = jsgraph()->graph()->NewNode(
jsgraph()->common()->ArgumentsLengthState(type));
jsgraph()->common()->ArgumentsLengthState());
NodeProperties::SetType(arguments_length_state,
Type::OtherInternal());
}

View File

@ -169,10 +169,10 @@ Reduction JSCreateLowering::ReduceJSCreateArguments(Node* node) {
Node* effect = NodeProperties::GetEffectInput(node);
Node* const arguments_frame =
graph()->NewNode(simplified()->ArgumentsFrame());
Node* const arguments_length = graph()->NewNode(
simplified()->ArgumentsLength(
shared.internal_formal_parameter_count(), false),
arguments_frame);
Node* const arguments_length =
graph()->NewNode(simplified()->ArgumentsLength(
shared.internal_formal_parameter_count()),
arguments_frame);
// Allocate the elements backing store.
bool has_aliased_arguments = false;
Node* const elements = effect = AllocateAliasedArguments(
@ -201,10 +201,10 @@ Reduction JSCreateLowering::ReduceJSCreateArguments(Node* node) {
Node* effect = NodeProperties::GetEffectInput(node);
Node* const arguments_frame =
graph()->NewNode(simplified()->ArgumentsFrame());
Node* const arguments_length = graph()->NewNode(
simplified()->ArgumentsLength(
shared.internal_formal_parameter_count(), false),
arguments_frame);
Node* const arguments_length =
graph()->NewNode(simplified()->ArgumentsLength(
shared.internal_formal_parameter_count()),
arguments_frame);
// Allocate the elements backing store.
Node* const elements = effect =
graph()->NewNode(simplified()->NewArgumentsElements(
@ -231,20 +231,19 @@ Reduction JSCreateLowering::ReduceJSCreateArguments(Node* node) {
Node* effect = NodeProperties::GetEffectInput(node);
Node* const arguments_frame =
graph()->NewNode(simplified()->ArgumentsFrame());
Node* const length = graph()->NewNode(
simplified()->ArgumentsLength(
shared.internal_formal_parameter_count(), false),
arguments_frame);
Node* const arguments_length =
graph()->NewNode(simplified()->ArgumentsLength(
shared.internal_formal_parameter_count()),
arguments_frame);
Node* const rest_length = graph()->NewNode(
simplified()->ArgumentsLength(
shared.internal_formal_parameter_count(), true),
simplified()->RestLength(shared.internal_formal_parameter_count()),
arguments_frame);
// Allocate the elements backing store.
Node* const elements = effect =
graph()->NewNode(simplified()->NewArgumentsElements(
CreateArgumentsType::kRestParameter,
shared.internal_formal_parameter_count()),
arguments_frame, length, effect);
arguments_frame, arguments_length, effect);
// Load the JSArray object map.
Node* const jsarray_map = jsgraph()->Constant(
native_context().js_array_packed_elements_map());

View File

@ -472,6 +472,7 @@
V(ObjectIsUndetectable) \
V(ArgumentsFrame) \
V(ArgumentsLength) \
V(RestLength) \
V(NewDoubleElements) \
V(NewSmiOrObjectElements) \
V(NewArgumentsElements) \

View File

@ -3511,7 +3511,8 @@ class RepresentationSelector {
SetOutput<T>(node, MachineType::PointerRepresentation());
return;
}
case IrOpcode::kArgumentsLength: {
case IrOpcode::kArgumentsLength:
case IrOpcode::kRestLength: {
VisitUnop<T>(node, UseInfo::Word(),
MachineRepresentation::kTaggedSigned);
return;

View File

@ -1625,49 +1625,30 @@ const Operator* SimplifiedOperatorBuilder::TransitionElementsKind(
transition); // parameter
}
namespace {
struct ArgumentsLengthParameters {
int formal_parameter_count;
bool is_rest_length;
};
bool operator==(ArgumentsLengthParameters first,
ArgumentsLengthParameters second) {
return first.formal_parameter_count == second.formal_parameter_count &&
first.is_rest_length == second.is_rest_length;
}
size_t hash_value(ArgumentsLengthParameters param) {
return base::hash_combine(param.formal_parameter_count, param.is_rest_length);
}
std::ostream& operator<<(std::ostream& os, ArgumentsLengthParameters param) {
return os << param.formal_parameter_count << ", "
<< (param.is_rest_length ? "rest length" : "not rest length");
}
} // namespace
const Operator* SimplifiedOperatorBuilder::ArgumentsLength(
int formal_parameter_count, bool is_rest_length) {
return zone()->New<Operator1<ArgumentsLengthParameters>>( // --
IrOpcode::kArgumentsLength, // opcode
Operator::kPure, // flags
"ArgumentsLength", // name
1, 0, 0, 1, 0, 0, // counts
ArgumentsLengthParameters{formal_parameter_count,
is_rest_length}); // parameter
int formal_parameter_count) {
return zone()->New<Operator1<int>>( // --
IrOpcode::kArgumentsLength, // opcode
Operator::kPure, // flags
"ArgumentsLength", // name
1, 0, 0, 1, 0, 0, // counts
formal_parameter_count); // parameter
}
const Operator* SimplifiedOperatorBuilder::RestLength(
int formal_parameter_count) {
return zone()->New<Operator1<int>>( // --
IrOpcode::kRestLength, // opcode
Operator::kPure, // flags
"RestLength", // name
1, 0, 0, 1, 0, 0, // counts
formal_parameter_count); // parameter
}
int FormalParameterCountOf(const Operator* op) {
DCHECK_EQ(IrOpcode::kArgumentsLength, op->opcode());
return OpParameter<ArgumentsLengthParameters>(op).formal_parameter_count;
}
bool IsRestLengthOf(const Operator* op) {
DCHECK_EQ(IrOpcode::kArgumentsLength, op->opcode());
return OpParameter<ArgumentsLengthParameters>(op).is_rest_length;
DCHECK(op->opcode() == IrOpcode::kArgumentsLength ||
op->opcode() == IrOpcode::kRestLength);
return OpParameter<int>(op);
}
bool operator==(CheckParameters const& lhs, CheckParameters const& rhs) {

View File

@ -582,7 +582,6 @@ const NumberOperationParameters& NumberOperationParametersOf(const Operator* op)
V8_WARN_UNUSED_RESULT;
int FormalParameterCountOf(const Operator* op) V8_WARN_UNUSED_RESULT;
bool IsRestLengthOf(const Operator* op) V8_WARN_UNUSED_RESULT;
class AllocateParameters {
public:
@ -939,8 +938,8 @@ class V8_EXPORT_PRIVATE SimplifiedOperatorBuilder final
const Operator* ObjectIsInteger();
const Operator* ArgumentsFrame();
const Operator* ArgumentsLength(int formal_parameter_count,
bool is_rest_length);
const Operator* ArgumentsLength(int formal_parameter_count);
const Operator* RestLength(int formal_parameter_count);
const Operator* NewDoubleElements(AllocationType);
const Operator* NewSmiOrObjectElements(AllocationType);

View File

@ -178,6 +178,11 @@ class V8_EXPORT_PRIVATE TypeCache final {
// fixed array in spread/apply calls.
Type const kArgumentsLengthType = CreateRange(0.0, FixedArray::kMaxLength);
// The valid number of arguments for rest parameters. We can never
// materialize more than the max size of a fixed array, because we require a
// fixed array in spread/apply calls.
Type const kRestLengthType = CreateRange(0.0, FixedArray::kMaxLength);
// The JSArrayIterator::kind property always contains an integer in the
// range [0, 2], representing the possible IterationKinds.
Type const kJSArrayIteratorKindType = CreateRange(0.0, 2.0);

View File

@ -2307,6 +2307,10 @@ Type Typer::Visitor::TypeArgumentsLength(Node* node) {
return TypeCache::Get()->kArgumentsLengthType;
}
Type Typer::Visitor::TypeRestLength(Node* node) {
return TypeCache::Get()->kArgumentsLengthType;
}
Type Typer::Visitor::TypeArgumentsFrame(Node* node) {
return Type::ExternalPointer();
}

View File

@ -1222,6 +1222,7 @@ void Verifier::Visitor::Check(Node* node, const AllNodes& all) {
CheckTypeIs(node, Type::SignedSmall());
break;
case IrOpcode::kArgumentsLength:
case IrOpcode::kRestLength:
CheckValueInputIs(node, 0, Type::ExternalPointer());
CheckTypeIs(node, TypeCache::Get()->kArgumentsLengthType);
break;

View File

@ -2056,10 +2056,7 @@ void Translation::ArgumentsElements(CreateArgumentsType type) {
buffer_->Add(static_cast<uint8_t>(type));
}
void Translation::ArgumentsLength(CreateArgumentsType type) {
buffer_->Add(ARGUMENTS_LENGTH);
buffer_->Add(static_cast<uint8_t>(type));
}
void Translation::ArgumentsLength() { buffer_->Add(ARGUMENTS_LENGTH); }
void Translation::BeginCapturedObject(int length) {
buffer_->Add(CAPTURED_OBJECT);
@ -2160,9 +2157,10 @@ void Translation::StoreJSFrameFunction() {
int Translation::NumberOfOperandsFor(Opcode opcode) {
switch (opcode) {
case ARGUMENTS_LENGTH:
return 0;
case DUPLICATED_OBJECT:
case ARGUMENTS_ELEMENTS:
case ARGUMENTS_LENGTH:
case CAPTURED_OBJECT:
case REGISTER:
case INT32_REGISTER:
@ -2939,7 +2937,6 @@ void TranslatedFrame::AdvanceIterator(
}
Address TranslatedState::ComputeArgumentsPosition(Address input_frame_pointer,
CreateArgumentsType type,
int* length) {
Address parent_frame_pointer = *reinterpret_cast<Address*>(
input_frame_pointer + StandardFrameConstants::kCallerFPOffset);
@ -2960,12 +2957,6 @@ Address TranslatedState::ComputeArgumentsPosition(Address input_frame_pointer,
arguments_frame = input_frame_pointer;
}
if (type == CreateArgumentsType::kRestParameter) {
// If the actual number of arguments is less than the number of formal
// parameters, we have zero rest parameters.
if (length) *length = std::max(0, *length - formal_parameter_count_);
}
return arguments_frame;
}
@ -2978,9 +2969,13 @@ void TranslatedState::CreateArgumentsElementsTranslatedValues(
FILE* trace_file) {
TranslatedFrame& frame = frames_[frame_index];
int length;
int arguments_length;
Address arguments_frame =
ComputeArgumentsPosition(input_frame_pointer, type, &length);
ComputeArgumentsPosition(input_frame_pointer, &arguments_length);
int length = type == CreateArgumentsType::kRestParameter
? std::max(0, arguments_length - formal_parameter_count_)
: arguments_length;
int object_index = static_cast<int>(object_positions_.size());
int value_index = static_cast<int>(frame.values_.size());
@ -3079,15 +3074,13 @@ int TranslatedState::CreateNextTranslatedValue(
}
case Translation::ARGUMENTS_LENGTH: {
CreateArgumentsType arguments_type =
static_cast<CreateArgumentsType>(iterator->Next());
int length;
ComputeArgumentsPosition(fp, arguments_type, &length);
int arguments_length;
ComputeArgumentsPosition(fp, &arguments_length);
if (trace_file != nullptr) {
PrintF(trace_file, "arguments length field (type = %d, length = %d)",
static_cast<uint8_t>(arguments_type), length);
PrintF(trace_file, "arguments length field (length = %d)",
arguments_length);
}
frame.Add(TranslatedValue::NewInt32(this, length));
frame.Add(TranslatedValue::NewInt32(this, arguments_length));
return 0;
}

View File

@ -355,8 +355,7 @@ class TranslatedState {
FixedArray literal_array, Address fp,
RegisterValues* registers, FILE* trace_file);
Address DecompressIfNeeded(intptr_t value);
Address ComputeArgumentsPosition(Address input_frame_pointer,
CreateArgumentsType type, int* length);
Address ComputeArgumentsPosition(Address input_frame_pointer, int* length);
void CreateArgumentsElementsTranslatedValues(int frame_index,
Address input_frame_pointer,
CreateArgumentsType type,
@ -914,7 +913,7 @@ class Translation {
int literal_id,
unsigned height);
void ArgumentsElements(CreateArgumentsType type);
void ArgumentsLength(CreateArgumentsType type);
void ArgumentsLength();
void BeginCapturedObject(int length);
void AddUpdateFeedback(int vector_literal, int slot);
void DuplicateObject(int object_index);

View File

@ -642,13 +642,16 @@ void DeoptimizationData::DeoptimizationDataPrint(std::ostream& os) { // NOLINT
break;
}
case Translation::ARGUMENTS_ELEMENTS:
case Translation::ARGUMENTS_LENGTH: {
case Translation::ARGUMENTS_ELEMENTS: {
CreateArgumentsType arguments_type =
static_cast<CreateArgumentsType>(iterator.Next());
os << "{arguments_type=" << arguments_type << "}";
break;
}
case Translation::ARGUMENTS_LENGTH: {
os << "{arguments_length}";
break;
}
case Translation::CAPTURED_OBJECT: {
int args_length = iterator.Next();