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/ git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@10091 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java index 8097e60..95c4081 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java
@@ -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; } }
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 57c9069..0195ae8 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
@@ -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; }
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 0ffdcf4..0b67077 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 @@ 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. @@ -485,31 +488,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()); }
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 e064689..408afaf 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 @@ 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; }
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 a477402..016e0bf 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 @@ 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; }