# HG changeset patch # User Christian Haeubl # Date 1359971604 -3600 # Node ID ed51e7237e94217a459b4b2a7abdebc2ae916222 # Parent aa933ac2a7faa3b22080cf849353344363b7f664 extracted TODOs as issues fixed GRAAL-71 (non-static final fields cannot always be assumed as constant) adjusted inlining policy diff -r aa933ac2a7fa -r ed51e7237e94 graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/inlining/InliningTest.java --- a/graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/inlining/InliningTest.java Mon Feb 04 10:10:05 2013 +0100 +++ b/graal/com.oracle.graal.compiler.test/src/com/oracle/graal/compiler/test/inlining/InliningTest.java Mon Feb 04 10:53:24 2013 +0100 @@ -36,7 +36,6 @@ import com.oracle.graal.phases.*; import com.oracle.graal.phases.common.*; -// TODO (chaeubl): add more test cases @SuppressWarnings("unused") public class InliningTest extends GraalCompilerTest { diff -r aa933ac2a7fa -r ed51e7237e94 graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotResolvedJavaField.java --- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotResolvedJavaField.java Mon Feb 04 10:10:05 2013 +0100 +++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotResolvedJavaField.java Mon Feb 04 10:53:24 2013 +0100 @@ -87,10 +87,16 @@ } return constant; } else { + /* + * for non-static final fields, we must assume that they are only initialized if they + * have a non-default value. + */ assert !Modifier.isStatic(flags); - // TODO (chaeubl) HotSpot does not trust final non-static fields (see ciField.cpp) if (Modifier.isFinal(getModifiers())) { - return readValue(receiver); + Constant value = readValue(receiver); + if (!value.isDefaultForKind()) { + return value; + } } } return null; diff -r aa933ac2a7fa -r ed51e7237e94 graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/InliningPhase.java --- a/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/InliningPhase.java Mon Feb 04 10:10:05 2013 +0100 +++ b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/InliningPhase.java Mon Feb 04 10:53:24 2013 +0100 @@ -107,7 +107,6 @@ } metricInliningPerformed.increment(); } catch (BailoutException bailout) { - // TODO determine if we should really bail out of the whole compilation. throw bailout; } catch (AssertionError e) { throw new GraalInternalError(e).addContext(candidate.toString()); diff -r aa933ac2a7fa -r ed51e7237e94 graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/InliningUtil.java --- a/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/InliningUtil.java Mon Feb 04 10:10:05 2013 +0100 +++ b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/InliningUtil.java Mon Feb 04 10:53:24 2013 +0100 @@ -338,10 +338,7 @@ @Override public void inline(StructuredGraph graph, GraalCodeCacheProvider runtime, InliningCallback callback, Assumptions assumptions) { createGuard(graph, runtime); - - StructuredGraph calleeGraph = getGraph(concrete, callback); - assumptions.recordMethodContents(concrete); - InliningUtil.inline(invoke, calleeGraph, false); + inline(invoke, concrete, callback, assumptions, false); } @Override @@ -822,8 +819,6 @@ return getExactInlineInfo(invoke, optimisticOpts, holder.resolveMethod(targetMethod)); } - // TODO (chaeubl): we could also use the type determined after assumptions for the - // type-checked inlining case as it might have an effect on type filtering if (assumptions.useOptimisticAssumptions()) { ResolvedJavaType uniqueSubtype = holder.findUniqueConcreteSubtype(); if (uniqueSubtype != null) { @@ -834,8 +829,6 @@ if (concrete != null) { return getAssumptionInlineInfo(invoke, optimisticOpts, concrete, new Assumptions.ConcreteMethod(targetMethod, holder, concrete)); } - - // TODO (chaeubl): C1 has one more assumption in the case of interfaces } // type check based inlining @@ -896,14 +889,6 @@ notRecordedTypeProbability * 100); } - // TODO (chaeubl) inlining of multiple methods should work differently - // 1. check which methods can be inlined - // 2. for those methods, use weight and probability to compute which of them should be - // inlined - // 3. do the inlining - // a) all seen methods can be inlined -> do so and guard with deopt - // b) some methods can be inlined -> inline them and fall back to invocation if violated - // determine concrete methods and map type to specific method ArrayList concreteMethods = new ArrayList<>(); int[] typesToConcretes = new int[ptypes.size()]; diff -r aa933ac2a7fa -r ed51e7237e94 graal/com.oracle.graal.phases/src/com/oracle/graal/phases/GraalOptions.java --- a/graal/com.oracle.graal.phases/src/com/oracle/graal/phases/GraalOptions.java Mon Feb 04 10:10:05 2013 +0100 +++ b/graal/com.oracle.graal.phases/src/com/oracle/graal/phases/GraalOptions.java Mon Feb 04 10:53:24 2013 +0100 @@ -51,14 +51,14 @@ public static float BoostInliningForEscapeAnalysis = 2f; public static float RelevanceCapForInlining = 1f; - public static int TrivialBytecodeSize = 10; - public static int NormalBytecodeSize = 100; + public static int TrivialBytecodeSize = 0; // TODO (chaeubl): change that to 10 when it survives bootstrapping + public static int NormalBytecodeSize = 150; public static int MaximumBytecodeSize = 500; public static int TrivialComplexity = 10; - public static int NormalComplexity = 40; + public static int NormalComplexity = 60; public static int MaximumComplexity = 400; public static int TrivialCompiledCodeSize = 150; - public static int NormalCompiledCodeSize = 500; + public static int NormalCompiledCodeSize = 750; public static int MaximumCompiledCodeSize = 4000; public static int SmallCompiledCodeSize = 1000; diff -r aa933ac2a7fa -r ed51e7237e94 graal/com.oracle.graal.phases/src/com/oracle/graal/phases/OptimisticOptimizations.java --- a/graal/com.oracle.graal.phases/src/com/oracle/graal/phases/OptimisticOptimizations.java Mon Feb 04 10:10:05 2013 +0100 +++ b/graal/com.oracle.graal.phases/src/com/oracle/graal/phases/OptimisticOptimizations.java Mon Feb 04 10:53:24 2013 +0100 @@ -52,9 +52,10 @@ if (checkDeoptimizations(method.getProfilingInfo(), deoptReason)) { enabledOpts.add(optimization); } else { - // TODO (chaeubl): change to Debug.log when we are sure that optimistic optimizations - // are not disabled - // unnecessarily + /* + * TODO (chaeubl): see GRAAL-75 (remove when we are sure that optimistic optimizations + * are not disabled unnecessarily + */ TTY.println("WARN: deactivated optimistic optimization %s for %s", optimization.name(), MetaUtil.format("%H.%n(%p)", method)); disabledOptimisticOptsMetric.increment(); }