Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(25)

Unified Diff: pkg/compiler/lib/src/cps_ir/shrinking_reductions.dart

Issue 869223003: Revert "Add a shrinking reduction for dead continuation parameters." (Closed) Base URL: https://dart.googlecode.com/svn/branches/bleeding_edge/dart
Patch Set: Created 5 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: pkg/compiler/lib/src/cps_ir/shrinking_reductions.dart
diff --git a/pkg/compiler/lib/src/cps_ir/shrinking_reductions.dart b/pkg/compiler/lib/src/cps_ir/shrinking_reductions.dart
index 756494ca0654ea141b726e20801d208542668239..ef514d4ed7ea5661158b96f5066b651b6c7c29b0 100644
--- a/pkg/compiler/lib/src/cps_ir/shrinking_reductions.dart
+++ b/pkg/compiler/lib/src/cps_ir/shrinking_reductions.dart
@@ -83,9 +83,6 @@ class ShrinkingReducer extends PassMixin {
case _ReductionKind.ETA_CONT:
_reduceEtaCont(task);
break;
- case _ReductionKind.DEAD_PARAMETER:
- _reduceDeadParameter(task);
- break;
default:
assert(false);
}
@@ -179,51 +176,6 @@ class ShrinkingReducer extends PassMixin {
// Perform bookkeeping on removed body and scan for new redexes.
new _RemovalVisitor(_worklist).visit(cont);
}
-
- void _reduceDeadParameter(_ReductionTask task) {
- assert(_isDeadParameter(task.node));
-
- Parameter parameter = task.node;
- Continuation continuation = parameter.parent;
- int index = parameter.parent_index;
-
- // Remove the index'th argument from each invocation.
- Reference<Continuation> current = continuation.firstRef;
- while (current != null) {
- InvokeContinuation invoke = current.parent;
- Reference<Primitive> argument = invoke.arguments[index];
- argument.unlink();
- // Removing an argument can create a dead parameter or dead value redex.
- if (argument.definition is Parameter) {
- if (_isDeadParameter(argument.definition)) {
- _worklist.add(new _ReductionTask(_ReductionKind.DEAD_PARAMETER,
- argument.definition));
- }
- } else {
- Node parent = argument.definition.parent;
- if (parent is LetPrim) {
- if (_isDeadVal(parent)) {
- _worklist.add(new _ReductionTask(_ReductionKind.DEAD_VAL, parent));
- }
- }
- }
- invoke.arguments.removeAt(index);
- current = current.next;
- }
- // Copy the parameters above index down.
- List<Parameter> parameters = continuation.parameters;
- for (int i = index; i < parameters.length - 1; ++i) {
- Parameter p = parameters[i + 1];
- parameters[i] = p;
- p.parent_index = i;
- }
- parameters.removeLast();
-
- // Removing an unused parameter can create an eta-redex.
- if (_isEtaCont(continuation)) {
- _worklist.add(new _ReductionTask(_ReductionKind.ETA_CONT, continuation));
- }
- }
}
/// Returns true iff the bound primitive is unused.
@@ -231,20 +183,14 @@ bool _isDeadVal(LetPrim node) => !node.primitive.hasAtLeastOneUse;
/// Returns true iff the continuation is unused.
bool _isDeadCont(Continuation cont) {
- return !cont.isReturnContinuation && !cont.hasAtLeastOneUse;
+ assert(!cont.isReturnContinuation);
+ return !cont.hasAtLeastOneUse;
}
-/// Returns true iff the continuation has a body (i.e., it is not the return
-/// continuation), it is used exactly once, and that use is as the continuation
-/// of a continuation invocation.
+/// Returns true iff the continuation is used exactly once, and that
+/// use is as the continuation of a continuation invocation.
bool _isBetaContLin(Continuation cont) {
- // There is a restriction on continuation eta-redexes that the body is not an
- // invocation of the return continuation, because that leads to worse code
- // when translating back to direct style (it duplicates returns). There is no
- // such restriction here because continuation beta-reduction is only performed
- // for singly referenced continuations. Thus, there is no possibility of code
- // duplication.
- if (cont.isReturnContinuation || !cont.hasExactlyOneUse) {
+ if (!cont.hasExactlyOneUse) {
return false;
}
@@ -259,49 +205,27 @@ bool _isBetaContLin(Continuation cont) {
/// Returns true iff the continuation consists of a continuation
/// invocation, passing on all parameters. Special cases exist (see below).
bool _isEtaCont(Continuation cont) {
- if (cont.isReturnContinuation || cont.body is! InvokeContinuation) {
+ if (cont.body is! InvokeContinuation) {
return false;
}
InvokeContinuation invoke = cont.body;
Continuation invokedCont = invoke.continuation.definition;
- // Do not eta-reduce return join-points since the direct-style code is worse
+ // Do not eta-reduce return join-points since the resulting code is worse
// in the common case (i.e. returns are moved inside `if` branches).
if (invokedCont.isReturnContinuation) {
return false;
}
// Translation to direct style generates different statements for recursive
- // and non-recursive invokes. It should still be possible to apply eta-cont if
- // this is not a self-invocation.
- //
- // TODO(kmillikin): Remove this restriction if it makes sense to do so.
+ // and non-recursive invokes. It should be possible to apply eta-cont, but
+ // higher order continuations require escape analysis, left as a possibility
+ // for future improvements.
if (invoke.isRecursive) {
return false;
}
- // If cont has more parameters than the invocation has arguments, the extra
- // parameters will be dead and dead-parameter will eventually create the
- // eta-redex if possible.
- //
- // If the invocation's arguments are simply a permutation of cont's
- // parameters, then there is likewise a possible reduction that involves
- // rewriting the invocations of cont. We are missing that reduction here.
- //
- // If cont has fewer parameters than the invocation has arguments then a
- // reduction would still possible, since the extra invocation arguments must
- // be in scope at all the invocations of cont. For example:
- //
- // let cont k1(x1) = k0(x0, x1) in E -eta-> E'
- // where E' has k0(x0, v) substituted for each k1(v).
- //
- // HOWEVER, adding continuation parameters is unlikely to be an optimization
- // since it duplicates assignments used in direct-style to implement parameter
- // passing.
- //
- // TODO(kmillikin): find real occurrences of these patterns, and see if they
- // can be optimized.
if (cont.parameters.length != invoke.arguments.length) {
return false;
}
@@ -317,28 +241,6 @@ bool _isEtaCont(Continuation cont) {
return true;
}
-bool _isDeadParameter(Parameter parameter) {
- // We cannot remove function parameters as an intraprocedural optimization.
- if (parameter.parent is! Continuation || parameter.hasAtLeastOneUse) {
- return false;
- }
-
- // We cannot remove the parameter to a call continuation, because the
- // resulting expression will not be well-formed (call continuations have
- // exactly one argument). The return continuation is a call continuation, so
- // we cannot remove its dummy parameter.
- Continuation continuation = parameter.parent;
- if (continuation.isReturnContinuation) return false;
- Reference<Continuation> current = continuation.firstRef;
- while (current != null) {
- if (current.parent is! InvokeContinuation) return false;
- InvokeContinuation invoke = current.parent;
- if (invoke.continuation.definition != continuation) return false;
- current = current.next;
- }
- return true;
-}
-
/// Traverses a term and adds any found redexes to the worklist.
class _RedexVisitor extends RecursiveVisitor {
final Set<_ReductionTask> worklist;
@@ -352,25 +254,12 @@ class _RedexVisitor extends RecursiveVisitor {
}
void processContinuation(Continuation node) {
- // Continuation beta- and eta-redexes can overlap, namely when an eta-redex
- // is invoked exactly once. We prioritize continuation beta-redexes over
- // eta-redexes because some reductions (e.g., dead parameter elimination)
- // can destroy a continuation eta-redex. If we prioritized eta- over
- // beta-redexes, this would implicitly "create" the corresponding beta-redex
- // (in the sense that it would still apply) and the algorithm would not
- // detect it.
if (_isDeadCont(node)) {
worklist.add(new _ReductionTask(_ReductionKind.DEAD_CONT, node));
- } else if (_isBetaContLin(node)){
- worklist.add(new _ReductionTask(_ReductionKind.BETA_CONT_LIN, node));
} else if (_isEtaCont(node)) {
worklist.add(new _ReductionTask(_ReductionKind.ETA_CONT, node));
- }
- }
-
- void processParameter(Parameter node) {
- if (_isDeadParameter(node)) {
- worklist.add(new _ReductionTask(_ReductionKind.DEAD_PARAMETER, node));
+ } else if (_isBetaContLin(node)){
+ worklist.add(new _ReductionTask(_ReductionKind.BETA_CONT_LIN, node));
}
}
}
@@ -409,7 +298,7 @@ class _RemovalVisitor extends RecursiveVisitor {
Continuation cont = reference.definition;
Node parent = cont.parent;
// The parent might be the deleted sentinel, or it might be a
- // RunnableBody if the continuation is the return continuation.
+ // FunctionDefinition if the continuation is the return continuation.
if (parent is LetCont) {
if (cont.isRecursive && cont.hasAtMostOneUse) {
// Convert recursive to nonrecursive continuations. If the
@@ -419,8 +308,6 @@ class _RemovalVisitor extends RecursiveVisitor {
}
if (_isDeadCont(cont)) {
worklist.add(new _ReductionTask(_ReductionKind.DEAD_CONT, cont));
- } else if (_isBetaContLin(cont)) {
- worklist.add(new _ReductionTask(_ReductionKind.BETA_CONT_LIN, cont));
}
}
}
@@ -431,25 +318,16 @@ class _RemovalVisitor extends RecursiveVisitor {
class ParentVisitor extends RecursiveVisitor {
processFunctionDefinition(FunctionDefinition node) {
node.body.parent = node;
- int index = 0;
- node.parameters.forEach((Parameter parameter) {
- parameter.parent = node;
- parameter.parent_index = index++;
- });
+ node.parameters.forEach((Definition p) => p.parent = node);
}
processRunnableBody(RunnableBody node) {
- node.returnContinuation.parent = node;
node.body.parent = node;
}
processConstructorDefinition(ConstructorDefinition node) {
node.body.parent = node;
- int index = 0;
- node.parameters.forEach((Parameter parameter) {
- parameter.parent = node;
- parameter.parent_index = index++;
- });
+ node.parameters.forEach((Definition p) => p.parent = node);
node.initializers.forEach((Initializer i) => i.parent = node);
}
@@ -470,11 +348,11 @@ class ParentVisitor extends RecursiveVisitor {
}
processLetCont(LetCont node) {
- int index = 0;
- node.continuations.forEach((Continuation continuation) {
- continuation.parent = node;
- continuation.parent_index = index++;
- });
+ for (int i = 0; i < node.continuations.length; ++i) {
+ Continuation cont = node.continuations[i];
+ cont.parent = node;
+ cont.parent_index = i;
+ }
node.body.parent = node;
}
@@ -549,12 +427,8 @@ class ParentVisitor extends RecursiveVisitor {
}
processContinuation(Continuation node) {
- if (node.body != null) node.body.parent = node;
- int index = 0;
- node.parameters.forEach((Parameter parameter) {
- parameter.parent = node;
- parameter.parent_index = index++;
- });
+ node.body.parent = node;
+ node.parameters.forEach((Parameter param) => param.parent = node);
}
// Conditions.
@@ -603,8 +477,6 @@ class _ReductionKind {
static const _ReductionKind BETA_CONT_LIN =
const _ReductionKind('beta-cont-lin', 2);
static const _ReductionKind ETA_CONT = const _ReductionKind('eta-cont', 3);
- static const _ReductionKind DEAD_PARAMETER =
- const _ReductionKind('dead-parameter', 4);
String toString() => name;
}
@@ -616,12 +488,12 @@ class _ReductionTask {
final Node node;
int get hashCode {
- assert(kind.hashCode < (1 << 3));
- return (node.hashCode << 3) | kind.hashCode;
+ assert(kind.hashCode < (1 << 2));
+ return (node.hashCode << 2) | kind.hashCode;
}
_ReductionTask(this.kind, this.node) {
- assert(node is Continuation || node is LetPrim || node is Parameter);
+ assert(node is Continuation || node is LetPrim);
}
bool operator==(_ReductionTask that) {
« no previous file with comments | « pkg/compiler/lib/src/cps_ir/cps_ir_nodes_sexpr.dart ('k') | tests/compiler/dart2js/backend_dart/opt_constprop_test.dart » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698