Revision: 10091
Author: sco...@google.com
Date: Wed Apr 27 13:35:10 2011
Log: Fix a class of compiler bugs related to staticImpl.
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.
http://gwt-code-reviews.appspot.com/1428804/
http://code.google.com/p/google-web-toolkit/source/detail?r=10091
Modified:
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/MethodInliner.java
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/SameParameterValueOptimizer.java
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/TypeTightener.java
=======================================
---
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java
Tue Apr 19 10:10:18 2011
+++
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java
Wed Apr 27 13:35:10 2011
@@ -541,19 +541,6 @@
maybeRescueJavaScriptObjectPassingIntoJava(method.getType());
}
rescueOverridingMethods(method);
-
- /*
- * Special case: also rescue an associated staticImpl. 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, reaching code
that was
- * pruned.
- */
- JMethod staticImpl = program.getStaticImpl(method);
- if (staticImpl != null) {
- rescue(staticImpl);
- }
return true;
}
}
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/MethodInliner.java Tue
Apr 19 10:10:18 2011
+++ /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/MethodInliner.java Wed
Apr 27 13:35:10 2011
@@ -144,6 +144,20 @@
@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;
}
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java Tue Apr 19
10:10:18 2011
+++ /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java Wed Apr 27
13:35:10 2011
@@ -360,7 +360,8 @@
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 @@
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 @@
* 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.
@@ -484,31 +487,6 @@
}
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());
=======================================
---
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/SameParameterValueOptimizer.java
Tue Apr 19 10:10:18 2011
+++
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/SameParameterValueOptimizer.java
Wed Apr 27 13:35:10 2011
@@ -132,12 +132,6 @@
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;
}
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/TypeTightener.java Tue
Apr 19 10:10:18 2011
+++ /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/TypeTightener.java Wed
Apr 27 13:35:10 2011
@@ -300,43 +300,7 @@
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
