Commit ebe6d7a9 authored by Michael Stanton's avatar Michael Stanton Committed by Commit Bot

Revert "[TurboFan] Diagnostic code to track down bug in representation selection"

This reverts commit f010b28f.

Reason for revert: Introduces a clusterfuzz issue and CAnary crash

Original change's description:
> [TurboFan] Diagnostic code to track down bug in representation selection
> 
> We need to characterize the types of dead (IrOpcode::kDead) nodes
> introduced in compilation phases prior to representation selection.
> Normally, a dead node isn't expected at the start of this phase. The
> question is, which phase introduced the dead node and failed to
> deal with it properly?
> 
> Bug: chromium:780658
> Change-Id: Ief5b45480bb7d704a2d09dafd60b5d389e0fd42e
> Reviewed-on: https://chromium-review.googlesource.com/765968
> Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
> Commit-Queue: Michael Stanton <mvstanton@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#49328}

TBR=mvstanton@chromium.org,mstarzinger@chromium.org

Change-Id: I5d628eb1de630ce4a353b6ef0f80fd74ad740f17
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:780658
Reviewed-on: https://chromium-review.googlesource.com/768747Reviewed-by: 's avatarMichael Stanton <mvstanton@chromium.org>
Commit-Queue: Michael Stanton <mvstanton@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49347}
parent 81931e72
......@@ -3191,7 +3191,7 @@ Node* BytecodeGraphBuilder::MakeNode(const Operator* op, int value_input_count,
// The frame state will be inserted later. Here we misuse the {Dead} node
// as a sentinel to be later overwritten with the real frame state by the
// calls to {PrepareFrameState} within individual visitor methods.
*current_input++ = jsgraph()->Dead(JSGraph::DeadCustomer::GraphBuilding);
*current_input++ = jsgraph()->Dead();
}
if (has_effect) {
*current_input++ = environment()->GetEffectDependency();
......
......@@ -390,9 +390,7 @@ void EffectControlLinearizer::Run() {
// The input blocks do not have the same effect. We have
// to create an effect phi node.
inputs_buffer.clear();
inputs_buffer.resize(
block->PredecessorCount(),
jsgraph()->Dead(JSGraph::DeadCustomer::Unspecified));
inputs_buffer.resize(block->PredecessorCount(), jsgraph()->Dead());
inputs_buffer.push_back(control);
effect = graph()->NewNode(
common()->EffectPhi(static_cast<int>(block->PredecessorCount())),
......
......@@ -74,7 +74,7 @@ Reduction EscapeAnalysisReducer::Reduce(Node* node) {
DCHECK(node->opcode() != IrOpcode::kAllocate &&
node->opcode() != IrOpcode::kFinishRegion);
DCHECK_NE(replacement, node);
if (replacement != jsgraph()->Dead(JSGraph::DeadCustomer::EscapeAnalysis)) {
if (replacement != jsgraph()->Dead()) {
replacement = MaybeGuard(node, replacement);
}
RelaxEffectsAndControls(node);
......@@ -178,7 +178,7 @@ Node* EscapeAnalysisReducer::ReduceDeoptState(Node* node, Node* effect,
Node* field =
analysis_result().GetVirtualObjectField(vobject, offset, effect);
CHECK_NOT_NULL(field);
if (field != jsgraph()->Dead(JSGraph::DeadCustomer::EscapeAnalysis)) {
if (field != jsgraph()->Dead()) {
inputs.push_back(ReduceDeoptState(field, effect, deduplicator));
}
}
......
......@@ -216,10 +216,7 @@ class EscapeAnalysisTracker : public ZoneObject {
replacement->id());
}
void MarkForDeletion() {
SetReplacement(
tracker_->jsgraph_->Dead(JSGraph::DeadCustomer::EscapeAnalysis));
}
void MarkForDeletion() { SetReplacement(tracker_->jsgraph_->Dead()); }
~Scope() {
if (replacement_ != tracker_->replacements_[current_node()] ||
......@@ -433,10 +430,7 @@ VariableTracker::State VariableTracker::MergeInputs(Node* effect_phi) {
// must have been created by a previous reduction of this [effect_phi].
for (int i = 0; i < arity; ++i) {
NodeProperties::ReplaceValueInput(
old_value,
buffer_[i] ? buffer_[i]
: graph_->Dead(JSGraph::DeadCustomer::EscapeAnalysis),
i);
old_value, buffer_[i] ? buffer_[i] : graph_->Dead(), i);
// This change cannot affect the rest of the reducer, so there is no
// need to trigger additional revisitations.
}
......@@ -546,8 +540,7 @@ void ReduceNode(const Operator* op, EscapeAnalysisTracker::Scope* current,
if (const VirtualObject* vobject = current->InitVirtualObject(size_int)) {
// Initialize with dead nodes as a sentinel for uninitialized memory.
for (Variable field : *vobject) {
current->Set(field,
jsgraph->Dead(JSGraph::DeadCustomer::EscapeAnalysis));
current->Set(field, jsgraph->Dead());
}
}
break;
......
......@@ -301,30 +301,10 @@ Node* JSGraph::SingleDeadTypedStateValues() {
SparseInputMask(SparseInputMask::kEndMarker << 1))));
}
Node* JSGraph::Dead(DeadCustomer which) {
switch (which) {
#define RETURN_CUSTOMER_NODE(customer) \
case customer: \
return CACHED(kDead##customer, graph()->NewNode(common()->Dead()));
CUSTOMER_LIST(RETURN_CUSTOMER_NODE)
#undef RETURN_CUSTOMER_NODE
default:
V8_Fatal(__FILE__, __LINE__, "Unexpected Which dead node detected");
}
return nullptr;
Node* JSGraph::Dead() {
return CACHED(kDead, graph()->NewNode(common()->Dead()));
}
const char* JSGraph::WhichDeadNode(Node* node) {
#define SEARCH_FOR_DEAD_CUSTOMER(name) \
if (node == cached_nodes_[kDead##name]) { \
return #name; \
}
CUSTOMER_LIST(SEARCH_FOR_DEAD_CUSTOMER)
#undef SEARCH_FOR_DEAD_CUSTOMER
return "Uncached dead node";
}
void JSGraph::GetCachedNodes(NodeVector* nodes) {
cache_.GetCachedNodes(nodes);
......@@ -335,8 +315,6 @@ void JSGraph::GetCachedNodes(NodeVector* nodes) {
}
}
#undef CACHED
} // namespace compiler
} // namespace internal
} // namespace v8
......@@ -152,26 +152,8 @@ class V8_EXPORT_PRIVATE JSGraph : public NON_EXPORTED_BASE(ZoneObject) {
// dead accumulator.
Node* SingleDeadTypedStateValues();
// Create a control node that serves as dependency for dead nodes.
// TODO(mvstanton): We distinguish between different types of dead nodes
// to track down bug chromium:780658. After fixing this bug, the
// enum can be removed.
#define CUSTOMER_LIST(V) \
V(GraphBuilding) \
V(TypeHint) \
V(Inlining) \
V(ContextSpecialization) \
V(EscapeAnalysis) \
V(GraphReducer) \
V(Unspecified)
enum DeadCustomer {
#define DEFINE_DEAD_CUSTOMER_ENUM(name) name,
CUSTOMER_LIST(DEFINE_DEAD_CUSTOMER_ENUM)
#undef DEFINE_DEAD_CUSTOMER_ENUM
};
Node* Dead(DeadCustomer which);
const char* WhichDeadNode(Node* node);
// Create a control node that serves as dependency for dead nodes.
Node* Dead();
CommonOperatorBuilder* common() const { return common_; }
JSOperatorBuilder* javascript() const { return javascript_; }
......@@ -213,10 +195,8 @@ class V8_EXPORT_PRIVATE JSGraph : public NON_EXPORTED_BASE(ZoneObject) {
kNaNConstant,
kEmptyStateValues,
kSingleDeadTypedStateValues,
#define DEFINE_CACHED_DEAD_CUSTOMER_ENUM(name) kDead##name,
CUSTOMER_LIST(DEFINE_CACHED_DEAD_CUSTOMER_ENUM)
#undef DEFINE_CACHED_DEAD_CUSTOMER_ENUM
kNumCachedNodes // Must remain last.
kDead,
kNumCachedNodes // Must remain last.
};
Isolate* isolate_;
......
......@@ -540,15 +540,11 @@ bool JSInliningHeuristic::TryReuseDispatch(Node* node, Node* callee,
}
// Mark the control inputs dead, so that we can kill the merge.
node->ReplaceInput(input_count - 1,
jsgraph()->Dead(JSGraph::DeadCustomer::Inlining));
callee->ReplaceInput(num_calls,
jsgraph()->Dead(JSGraph::DeadCustomer::Inlining));
effect_phi->ReplaceInput(num_calls,
jsgraph()->Dead(JSGraph::DeadCustomer::Inlining));
node->ReplaceInput(input_count - 1, jsgraph()->Dead());
callee->ReplaceInput(num_calls, jsgraph()->Dead());
effect_phi->ReplaceInput(num_calls, jsgraph()->Dead());
if (checkpoint) {
checkpoint->ReplaceInput(2,
jsgraph()->Dead(JSGraph::DeadCustomer::Inlining));
checkpoint->ReplaceInput(2, jsgraph()->Dead());
}
merge->Kill();
......
......@@ -179,7 +179,7 @@ Reduction JSInliner::InlineCall(Node* call, Node* new_target, Node* context,
control_output);
} else {
ReplaceWithValue(exception_target, exception_target, exception_target,
jsgraph()->Dead(JSGraph::DeadCustomer::Inlining));
jsgraph()->Dead());
}
}
......@@ -224,9 +224,8 @@ Reduction JSInliner::InlineCall(Node* call, Node* new_target, Node* context,
ReplaceWithValue(call, value_output, effect_output, control_output);
return Changed(value_output);
} else {
ReplaceWithValue(call, jsgraph()->Dead(JSGraph::DeadCustomer::Inlining),
jsgraph()->Dead(JSGraph::DeadCustomer::Inlining),
jsgraph()->Dead(JSGraph::DeadCustomer::Inlining));
ReplaceWithValue(call, jsgraph()->Dead(), jsgraph()->Dead(),
jsgraph()->Dead());
return Changed(call);
}
}
......
......@@ -890,8 +890,7 @@ Reduction JSNativeContextSpecialization::ReduceNamedAccess(
// Generate the final merge point for all (polymorphic) branches.
int const control_count = static_cast<int>(controls.size());
if (control_count == 0) {
value = effect = control =
jsgraph()->Dead(JSGraph::DeadCustomer::ContextSpecialization);
value = effect = control = jsgraph()->Dead();
} else if (control_count == 1) {
value = values.front();
effect = effects.front();
......@@ -977,8 +976,7 @@ Reduction JSNativeContextSpecialization::ReduceJSLoadNamed(Node* node) {
DCHECK_EQ(IrOpcode::kJSLoadNamed, node->opcode());
NamedAccess const& p = NamedAccessOf(node->op());
Node* const receiver = NodeProperties::GetValueInput(node, 0);
Node* const value =
jsgraph()->Dead(JSGraph::DeadCustomer::ContextSpecialization);
Node* const value = jsgraph()->Dead();
// Check if we have a constant receiver.
HeapObjectMatcher m(receiver);
......@@ -1245,8 +1243,7 @@ Reduction JSNativeContextSpecialization::ReduceElementAccess(
// Generate the final merge point for all (polymorphic) branches.
int const control_count = static_cast<int>(controls.size());
if (control_count == 0) {
value = effect = control =
jsgraph()->Dead(JSGraph::DeadCustomer::ContextSpecialization);
value = effect = control = jsgraph()->Dead();
} else if (control_count == 1) {
value = values.front();
effect = effects.front();
......@@ -1445,7 +1442,7 @@ Reduction JSNativeContextSpecialization::ReduceJSLoadProperty(Node* node) {
PropertyAccess const& p = PropertyAccessOf(node->op());
Node* receiver = NodeProperties::GetValueInput(node, 0);
Node* name = NodeProperties::GetValueInput(node, 1);
Node* value = jsgraph()->Dead(JSGraph::DeadCustomer::ContextSpecialization);
Node* value = jsgraph()->Dead();
Node* effect = NodeProperties::GetEffectInput(node);
Node* control = NodeProperties::GetControlInput(node);
......
......@@ -428,7 +428,7 @@ Node* JSTypeHintLowering::TryBuildSoftDeopt(FeedbackNexus& nexus, Node* effect,
if ((flags() & kBailoutOnUninitialized) && nexus.IsUninitialized()) {
Node* deoptimize = jsgraph()->graph()->NewNode(
jsgraph()->common()->Deoptimize(DeoptimizeKind::kSoft, reason),
jsgraph()->Dead(JSGraph::DeadCustomer::TypeHint), effect, control);
jsgraph()->Dead(), effect, control);
Node* frame_state = NodeProperties::FindFrameStateBefore(deoptimize);
deoptimize->ReplaceInput(0, frame_state);
return deoptimize;
......
......@@ -664,8 +664,7 @@ class SourcePositionWrapper final : public Reducer {
class JSGraphReducer final : public GraphReducer {
public:
JSGraphReducer(JSGraph* jsgraph, Zone* zone)
: GraphReducer(zone, jsgraph->graph(),
jsgraph->Dead(JSGraph::DeadCustomer::GraphReducer)) {}
: GraphReducer(zone, jsgraph->graph(), jsgraph->Dead()) {}
~JSGraphReducer() final {}
};
......
......@@ -206,16 +206,6 @@ class InputUseInfos {
#endif // DEBUG
// TODO(mvstanton): Remove after fixing chromium:780658.
static void UnexpectedDeadOpcode(const char* file, int line,
const char* deadCode, Node* node) {
// Record the deadcode in the stack to ease minidump debugging.
const char* format_string = "Unexpected dead opcode %s, node #%i\n.";
char buffer[256];
snprintf(buffer, sizeof(buffer), format_string, deadCode, node->id());
V8_Fatal(file, line, format_string, deadCode, node->id());
}
bool CanOverflowSigned32(const Operator* op, Type* left, Type* right,
Zone* type_zone) {
// We assume the inputs are checked Signed32 (or known statically
......@@ -3057,11 +3047,6 @@ class RepresentationSelector {
// Assume the output is tagged.
return SetOutput(node, MachineRepresentation::kTagged);
case IrOpcode::kDead: {
const char* deadVersion = jsgraph_->WhichDeadNode(node);
UnexpectedDeadOpcode(__FILE__, __LINE__, deadVersion, node);
break;
}
default:
V8_Fatal(
__FILE__, __LINE__,
......@@ -3107,7 +3092,7 @@ class RepresentationSelector {
DCHECK_EQ(0, node->op()->EffectOutputCount());
}
node->ReplaceUses(jsgraph_->Dead(JSGraph::DeadCustomer::Unspecified));
node->ReplaceUses(jsgraph_->Dead());
node->NullAllInputs(); // The {node} is now dead.
}
......
......@@ -94,9 +94,7 @@ WasmGraphBuilder::WasmGraphBuilder(
DCHECK_NOT_NULL(jsgraph_);
}
Node* WasmGraphBuilder::Error() {
return jsgraph()->Dead(JSGraph::DeadCustomer::Unspecified);
}
Node* WasmGraphBuilder::Error() { return jsgraph()->Dead(); }
Node* WasmGraphBuilder::Start(unsigned params) {
Node* start = graph()->NewNode(jsgraph()->common()->Start(params));
......
......@@ -27,8 +27,7 @@ class BranchEliminationTest : public GraphTest {
JSOperatorBuilder javascript(zone());
JSGraph jsgraph(isolate(), graph(), common(), &javascript, nullptr,
machine());
GraphReducer graph_reducer(
zone(), graph(), jsgraph.Dead(JSGraph::DeadCustomer::Unspecified));
GraphReducer graph_reducer(zone(), graph(), jsgraph.Dead());
BranchElimination branch_condition_elimination(&graph_reducer, &jsgraph,
zone());
graph_reducer.AddReducer(&branch_condition_elimination);
......
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