Reviewers: cromwellian, jbrosenberg,
Message:
Ran into this general class of issue... originally I set out to add
staticImpl handling logic to a couple more places, such as
ImplicitUpcastAnalyzer. But the more I thought about it, the more it
struck me as a real wart, and one that's not giving us much value.
Perhaps even negative value by causing code bloat. I think it's much
simpler to just never inline staticImpls into the calling virtual
method.
Please review this at http://gwt-code-reviews.appspot.com/1428804/
Affected files:
M dev/core/src/com/google/gwt/dev/jjs/impl/MethodInliner.java
M dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java
M
dev/core/src/com/google/gwt/dev/jjs/impl/SameParameterValueOptimizer.java
M dev/core/src/com/google/gwt/dev/jjs/impl/TypeTightener.java
Index: dev/core/src/com/google/gwt/dev/jjs/impl/MethodInliner.java
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/MethodInliner.java
b/dev/core/src/com/google/gwt/dev/jjs/impl/MethodInliner.java
index
d7f5f3fecfdb50302c7a723d3d9ead02d30d9e50..92eb2bb156eaee1683049a95472d1d7f9ce09c0d
100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/MethodInliner.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/MethodInliner.java
@@ -145,6 +145,20 @@ public class MethodInliner {
@Override
public boolean visit(JMethod x, Context ctx) {
currentMethod = x;
+ if (program.getStaticImpl(x) != null) {
+ /*
+ * Never inline a static impl into the calling instance method. We
used
+ * to allow this, and it required all kinds of special logic in the
+ * optimizers to keep the AST sane. This was because it was
possible to
+ * tighten an instance call to its static impl after the static
impl had
+ * already been inlined, this meant any "flow" type optimizer
would have
+ * to fake artifical flow from the instance method to the static
impl.
+ *
+ * TODO: allow the inlining if we are the last remaining call
site, and
+ * prune the static impl? But it might tend to generate more code.
+ */
+ return false;
+ }
return true;
}
Index: dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java
b/dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java
index
0ffdcf40faefc2fdc8b3c4ecf293c2ac372d5568..0b67077fe1db9ca11caf48e0fd5be80f15815e0e
100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java
@@ -360,7 +360,8 @@ public class Pruner {
for (int i = 0; i < type.getMethods().size(); ++i) {
JMethod method = type.getMethods().get(i);
- if (!methodIsReferenced(method) ||
pruneViaNoninstantiability(isInstantiated, method)) {
+ if (!referencedNonTypes.contains(method)
+ || pruneViaNoninstantiability(isInstantiated, method)) {
// Never prune clinit directly out of the class.
if (i > 0) {
type.removeMethod(i);
@@ -394,7 +395,7 @@ public class Pruner {
for (int i = 1; i < type.getMethods().size(); ++i) {
JMethod method = type.getMethods().get(i);
// all other interface methods are instance and abstract
- if (!isInstantiated || !methodIsReferenced(method)) {
+ if (!isInstantiated || !referencedNonTypes.contains(method)) {
type.removeMethod(i);
madeChanges();
--i;
@@ -426,6 +427,8 @@ public class Pruner {
* devirtualizations. If the instance method has been pruned, then
it's
* okay. Also, it's okay on the final pass since no more
* devirtualizations will occur.
+ *
+ * TODO: prune params; MakeCallsStatic smarter to account for it.
*/
JMethod staticImplFor = program.staticImplFor(x);
// Unless the instance method has already been pruned, of course.
@@ -485,31 +488,6 @@ public class Pruner {
return false;
}
- /**
- * Returns <code>true</code> if a method is referenced.
- */
- private boolean methodIsReferenced(JMethod method) {
- // Is the method directly referenced?
- if (referencedNonTypes.contains(method)) {
- return true;
- }
-
- /*
- * Special case: if method is the static impl for a live instance
method,
- * don't prune it unless this is the final prune.
- *
- * In some cases, the staticImpl can be inlined into the instance
method
- * but still be needed at other call sites.
- */
- JMethod staticImplFor = program.staticImplFor(method);
- if (staticImplFor != null &&
referencedNonTypes.contains(staticImplFor)) {
- if (saveCodeGenTypes) {
- return true;
- }
- }
- return false;
- }
-
private boolean pruneViaNoninstantiability(boolean isInstantiated,
CanBeStatic it) {
return (!isInstantiated && !it.isStatic());
}
Index:
dev/core/src/com/google/gwt/dev/jjs/impl/SameParameterValueOptimizer.java
diff --git
a/dev/core/src/com/google/gwt/dev/jjs/impl/SameParameterValueOptimizer.java
b/dev/core/src/com/google/gwt/dev/jjs/impl/SameParameterValueOptimizer.java
index
e0646893e12de6ba7f5850b017d0ebbb49ef4373..408afafd207d2070269467475e7c59cd305c429f
100644
---
a/dev/core/src/com/google/gwt/dev/jjs/impl/SameParameterValueOptimizer.java
+++
b/dev/core/src/com/google/gwt/dev/jjs/impl/SameParameterValueOptimizer.java
@@ -132,12 +132,6 @@ public class SameParameterValueOptimizer {
rescuedMethods.add(m);
}
rescuedMethods.add(x);
- } else {
- JMethod staticImpl = program.staticImplFor(x);
- if (staticImpl != null &&
staticImpl.getEnclosingType().getMethods().contains(staticImpl)) {
- // instance method is still alive.
- rescuedMethods.add(x);
- }
}
return true;
}
Index: dev/core/src/com/google/gwt/dev/jjs/impl/TypeTightener.java
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/TypeTightener.java
b/dev/core/src/com/google/gwt/dev/jjs/impl/TypeTightener.java
index
a4774029a8656bc219d6cf17d8eaf9c7de6c6dd6..016e0bf89f6994b69188a58face2ee669fb48ba7
100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/TypeTightener.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/TypeTightener.java
@@ -300,43 +300,7 @@ public class TypeTightener {
set.add(baseParam);
}
}
- } else if (program.isStaticImpl(x)) {
- /*
- * Special case: also add upRefs from a staticImpl's params to the
- * params of the instance method it is implementing. Most of the
time,
- * this would happen naturally since the instance method delegates
to
- * the static. However, in cases where the static has been inlined
into
- * the instance method, future optimization could tighten an
instance
- * call into a static call at some other call site, and fail to
inline.
- * If we allowed a staticImpl param to be tighter than its instance
- * param, badness would ensue.
- */
- JMethod staticImplFor = program.staticImplFor(x);
- if (staticImplFor == null
- |
| !staticImplFor.getEnclosingType().getMethods().contains(staticImplFor)) {
- // The instance method has already been pruned.
- return true;
- }
- assert (x.getParams().size() == staticImplFor.getParams().size() +
1);
- for (int j = 0, c = x.getParams().size(); j < c; ++j) {
- JParameter param = x.getParams().get(j);
- Set<JParameter> set = paramUpRefs.get(param);
- if (set == null) {
- set = new HashSet<JParameter>();
- paramUpRefs.put(param, set);
- }
- if (j == 0) {
- // Fake an assignment-to-self to prevent tightening; consider
this
- // an implicit assignment from a this reference of the looser
type.
- assert (param.isThis());
- set.add(param);
- } else {
- JParameter baseParam = staticImplFor.getParams().get(j - 1);
- set.add(baseParam);
- }
- }
}
-
return true;
}
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
