Skip to content
Permalink
Browse files

Revert "[TurboFan] Diagnostic code to track down bug in representatio…

…n selection"

This reverts commit f010b28.

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/768747
Reviewed-by: Michael Stanton <mvstanton@chromium.org>
Commit-Queue: Michael Stanton <mvstanton@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49347}
  • Loading branch information...
ripsawridge authored and Commit Bot committed Nov 14, 2017
1 parent 81931e7 commit ebe6d7a97f962c18595a2a1efaa93fa1c5ede604
@@ -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);

0 comments on commit ebe6d7a

Please sign in to comment.
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.