From bd70a67061608f2e7f7067f3cdfd9384e3e91915 Mon Sep 17 00:00:00 2001 From: phh Date: Tue, 7 Jul 2020 23:26:14 +0100 Subject: [PATCH] 8236867: Enhance Graal interface handling Summary: Includes "[GR-14791] Fix NPE in InliningData.getInlineInfo" (Graal 4c69db20e2b614c29af27ed95d20e3b80bd21848) Reviewed-by: andrew Contributed-by: benty@amazon.com --- .../vm/ci/hotspot/HotSpotConstantPool.java | 33 ++++ .../src/jdk/vm/ci/meta/ConstantPool.java | 10 ++ .../test/SingleImplementorInterfaceTest.java | 154 ++++++++++++++++++ .../core/test/inlining/InliningTest.java | 9 +- .../graalvm/compiler/java/BytecodeParser.java | 23 ++- .../compiler/nodes/CallTargetNode.java | 52 +++++- .../nodes/java/MethodCallTargetNode.java | 47 +++--- .../common/inlining/walker/InliningData.java | 32 ++-- .../classfile/ClassfileConstantPool.java | 40 ++++- .../classfile/ConstantPoolPatch.java | 31 ++++ .../serviceprovider/GraalServices.java | 32 ++++ 11 files changed, 417 insertions(+), 46 deletions(-) create mode 100644 src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.core.test/src/org/graalvm/compiler/core/test/SingleImplementorInterfaceTest.java create mode 100644 src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.replacements/src/org/graalvm/compiler/replacements/classfile/ConstantPoolPatch.java diff --git a/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotConstantPool.java b/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotConstantPool.java index c870138824..b5d6956afb 100644 --- a/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotConstantPool.java +++ b/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotConstantPool.java @@ -613,6 +613,39 @@ public final class HotSpotConstantPool implements ConstantPool, MetaspaceWrapper } } + @Override + public JavaType lookupReferencedType(int cpi, int opcode) { + int index; + switch (opcode) { + case Bytecodes.CHECKCAST: + case Bytecodes.INSTANCEOF: + case Bytecodes.NEW: + case Bytecodes.ANEWARRAY: + case Bytecodes.MULTIANEWARRAY: + case Bytecodes.LDC: + case Bytecodes.LDC_W: + case Bytecodes.LDC2_W: + index = cpi; + break; + case Bytecodes.GETSTATIC: + case Bytecodes.PUTSTATIC: + case Bytecodes.GETFIELD: + case Bytecodes.PUTFIELD: + case Bytecodes.INVOKEVIRTUAL: + case Bytecodes.INVOKESPECIAL: + case Bytecodes.INVOKESTATIC: + case Bytecodes.INVOKEINTERFACE: { + index = rawIndexToConstantPoolCacheIndex(cpi, opcode); + index = getKlassRefIndexAt(index); + break; + } + default: + throw JVMCIError.shouldNotReachHere("Unexpected opcode " + opcode); + } + final Object type = compilerToVM().lookupKlassInPool(this, index); + return getJavaType(type); + } + @Override public JavaField lookupField(int cpi, ResolvedJavaMethod method, int opcode) { final int index = rawIndexToConstantPoolCacheIndex(cpi, opcode); diff --git a/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.meta/src/jdk/vm/ci/meta/ConstantPool.java b/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.meta/src/jdk/vm/ci/meta/ConstantPool.java index 1df1c1faee..5711b4ebd7 100644 --- a/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.meta/src/jdk/vm/ci/meta/ConstantPool.java +++ b/src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.meta/src/jdk/vm/ci/meta/ConstantPool.java @@ -46,6 +46,16 @@ public interface ConstantPool { */ void loadReferencedType(int cpi, int opcode); + /** + * Looks up the type referenced by the constant pool entry at {@code cpi} as referenced by the + * {@code opcode} bytecode instruction. + * + * @param cpi the index of a constant pool entry that references a type + * @param opcode the opcode of the instruction with {@code cpi} as an operand + * @return a reference to the compiler interface type + */ + JavaType lookupReferencedType(int cpi, int opcode); + /** * Looks up a reference to a field. If {@code opcode} is non-negative, then resolution checks * specific to the bytecode it denotes are performed if the field is already resolved. Checks diff --git a/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.core.test/src/org/graalvm/compiler/core/test/SingleImplementorInterfaceTest.java b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.core.test/src/org/graalvm/compiler/core/test/SingleImplementorInterfaceTest.java new file mode 100644 index 0000000000..df9c482661 --- /dev/null +++ b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.core.test/src/org/graalvm/compiler/core/test/SingleImplementorInterfaceTest.java @@ -0,0 +1,154 @@ +/* + * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package org.graalvm.compiler.core.test; + +import jdk.vm.ci.meta.ResolvedJavaType; +import org.graalvm.compiler.nodes.CallTargetNode; +import org.graalvm.compiler.nodes.InvokeNode; +import org.graalvm.compiler.nodes.StructuredGraph; +import org.graalvm.compiler.nodes.java.InstanceOfNode; +import org.graalvm.compiler.phases.OptimisticOptimizations; +import org.graalvm.compiler.phases.common.CanonicalizerPhase; +import org.graalvm.compiler.phases.common.inlining.InliningPhase; +import org.graalvm.compiler.phases.tiers.HighTierContext; +import org.graalvm.compiler.phases.tiers.PhaseContext; +import org.graalvm.compiler.serviceprovider.GraalServices; +import org.junit.Test; + +public class SingleImplementorInterfaceTest extends GraalCompilerTest { + + public interface Interface0 { + void interfaceMethod(); + } + + public interface Interface1 extends Interface0 { + + } + + public interface Interface2 extends Interface1 { + } + + @SuppressWarnings("all") + public static class SingleImplementor1 implements Interface1 { + public void interfaceMethod() { + } + } + + // Requires that the CHA analysis starts from the referenced type. Since {@code + // SingleImplementor1} + // is not a single implementor of {@code Interface2} devirtualization shouldn't happen. + @SuppressWarnings("all") + private static void singleImplementorInterfaceSnippet1(Interface2 i) { + i.interfaceMethod(); + } + + // Devirtualization should happen in this case. + @SuppressWarnings("all") + private static void singleImplementorInterfaceSnippet2(Interface1 i) { + i.interfaceMethod(); + } + + @Test + public void testSingleImplementorInterfaceDevirtualization1() { + ResolvedJavaType singleImplementorType = getMetaAccess().lookupJavaType(SingleImplementor1.class); + ResolvedJavaType expectedReferencedType = getMetaAccess().lookupJavaType(Interface2.class); + singleImplementorType.initialize(); + StructuredGraph graph = parseEager("singleImplementorInterfaceSnippet1", StructuredGraph.AllowAssumptions.YES); + new CanonicalizerPhase().apply(graph, new PhaseContext(getProviders())); + // Devirtualization shouldn't work in this case. The invoke should remain intact. + InvokeNode invoke = graph.getNodes().filter(InvokeNode.class).first(); + assertTrue(invoke != null, "Should have an invoke"); + assertTrue(invoke.callTarget().invokeKind() == CallTargetNode.InvokeKind.Interface, "Should still be an interface call"); + if (GraalServices.hasLookupReferencedType()) { + assertTrue(invoke.callTarget().referencedType() != null, "Invoke should have a reference class set"); + assertTrue(invoke.callTarget().referencedType().equals(expectedReferencedType)); + } + } + + @Test + public void testSingleImplementorInterfaceDevirtualization2() { + ResolvedJavaType singleImplementorType = getMetaAccess().lookupJavaType(SingleImplementor1.class); + singleImplementorType.initialize(); + StructuredGraph graph = parseEager("singleImplementorInterfaceSnippet2", StructuredGraph.AllowAssumptions.YES); + new CanonicalizerPhase().apply(graph, new PhaseContext(getProviders())); + InvokeNode invoke = graph.getNodes().filter(InvokeNode.class).first(); + assertTrue(invoke != null, "Should have an invoke"); + if (GraalServices.hasLookupReferencedType()) { + assertTrue(invoke.callTarget().invokeKind() == CallTargetNode.InvokeKind.Special, "Should be devirtualized"); + InstanceOfNode instanceOfNode = graph.getNodes().filter(InstanceOfNode.class).first(); + assertTrue(instanceOfNode != null, "Missing the subtype check"); + assertTrue(instanceOfNode.getCheckedStamp().type().equals(singleImplementorType), "Checking against a wrong type"); + } else { + assertTrue(invoke.callTarget().invokeKind() == CallTargetNode.InvokeKind.Interface, "Should not be devirtualized"); + } + } + + @Test + public void testSingleImplementorInterfaceInlining1() { + ResolvedJavaType singleImplementorType = getMetaAccess().lookupJavaType(SingleImplementor1.class); + ResolvedJavaType expectedReferencedType = getMetaAccess().lookupJavaType(Interface2.class); + singleImplementorType.initialize(); + StructuredGraph graph = parseEager("singleImplementorInterfaceSnippet1", StructuredGraph.AllowAssumptions.YES); + HighTierContext context = new HighTierContext(getProviders(), getDefaultGraphBuilderSuite(), OptimisticOptimizations.ALL); + new InliningPhase(new CanonicalizerPhase()).apply(graph, context); + // Inlining shouldn't do anything + InvokeNode invoke = graph.getNodes().filter(InvokeNode.class).first(); + assertTrue(invoke != null, "Should have an invoke"); + if (GraalServices.hasLookupReferencedType()) { + assertTrue(invoke.callTarget().referencedType() != null, "Invoke should have a reference class set"); + assertTrue(invoke.callTarget().invokeKind() == CallTargetNode.InvokeKind.Interface, "Should still be an interface call"); + assertTrue(invoke.callTarget().referencedType().equals(expectedReferencedType)); + } else { + assertTrue(invoke.callTarget().invokeKind() == CallTargetNode.InvokeKind.Interface, "Should not be devirtualized"); + } + } + + @Test + public void testSingleImplementorInterfaceInlining2() { + ResolvedJavaType singleImplementorType = getMetaAccess().lookupJavaType(SingleImplementor1.class); + ResolvedJavaType expectedReferencedType = getMetaAccess().lookupJavaType(Interface1.class); + singleImplementorType.initialize(); + StructuredGraph graph = parseEager("singleImplementorInterfaceSnippet2", StructuredGraph.AllowAssumptions.YES); + HighTierContext context = new HighTierContext(getProviders(), getDefaultGraphBuilderSuite(), OptimisticOptimizations.ALL); + new InliningPhase(new CanonicalizerPhase()).apply(graph, context); + + // Right now inlining will not do anything, but if it starts doing devirtualization of + // interface calls + // in the future there should be a subtype check. + InvokeNode invoke = graph.getNodes().filter(InvokeNode.class).first(); + if (invoke != null) { + assertTrue(invoke.callTarget().invokeKind() == CallTargetNode.InvokeKind.Interface, "Should still be an interface call"); + if (GraalServices.hasLookupReferencedType()) { + assertTrue(invoke.callTarget().referencedType() != null, "Invoke should have a reference class set"); + assertTrue(invoke.callTarget().referencedType().equals(expectedReferencedType)); + } + } else { + InstanceOfNode instanceOfNode = graph.getNodes().filter(InstanceOfNode.class).first(); + assertTrue(instanceOfNode != null, "Missing the subtype check"); + assertTrue(instanceOfNode.getCheckedStamp().type().equals(singleImplementorType), "Checking against a wrong type"); + } + } +} diff --git a/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.core.test/src/org/graalvm/compiler/core/test/inlining/InliningTest.java b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.core.test/src/org/graalvm/compiler/core/test/inlining/InliningTest.java index f4fa65d68f..630bcac416 100644 --- a/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.core.test/src/org/graalvm/compiler/core/test/inlining/InliningTest.java +++ b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.core.test/src/org/graalvm/compiler/core/test/inlining/InliningTest.java @@ -43,6 +43,7 @@ import org.graalvm.compiler.phases.common.CanonicalizerPhase; import org.graalvm.compiler.phases.common.DeadCodeEliminationPhase; import org.graalvm.compiler.phases.common.inlining.InliningPhase; import org.graalvm.compiler.phases.tiers.HighTierContext; +import org.graalvm.compiler.serviceprovider.GraalServices; import org.junit.Assert; import org.junit.Ignore; import org.junit.Test; @@ -184,7 +185,9 @@ public class InliningTest extends GraalCompilerTest { public void testClassHierarchyAnalysis() { assertInlined(getGraph("invokeLeafClassMethodSnippet", false)); assertInlined(getGraph("invokeConcreteMethodSnippet", false)); - assertInlined(getGraph("invokeSingleImplementorInterfaceSnippet", false)); + if (GraalServices.hasLookupReferencedType()) { + assertInlined(getGraph("invokeSingleImplementorInterfaceSnippet", false)); + } // assertInlined(getGraph("invokeConcreteInterfaceMethodSnippet", false)); assertNotInlined(getGraph("invokeOverriddenPublicMethodSnippet", false)); @@ -196,7 +199,9 @@ public class InliningTest extends GraalCompilerTest { public void testClassHierarchyAnalysisIP() { assertManyMethodInfopoints(assertInlined(getGraph("invokeLeafClassMethodSnippet", true))); assertManyMethodInfopoints(assertInlined(getGraph("invokeConcreteMethodSnippet", true))); - assertManyMethodInfopoints(assertInlined(getGraph("invokeSingleImplementorInterfaceSnippet", true))); + if (GraalServices.hasLookupReferencedType()) { + assertManyMethodInfopoints(assertInlined(getGraph("invokeSingleImplementorInterfaceSnippet", true))); + } //@formatter:off // assertInlineInfopoints(assertInlined(getGraph("invokeConcreteInterfaceMethodSnippet", true))); //@formatter:on diff --git a/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.java/src/org/graalvm/compiler/java/BytecodeParser.java b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.java/src/org/graalvm/compiler/java/BytecodeParser.java index e0f178e65e..6fcb6ea4cb 100644 --- a/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.java/src/org/graalvm/compiler/java/BytecodeParser.java +++ b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.java/src/org/graalvm/compiler/java/BytecodeParser.java @@ -424,6 +424,7 @@ import org.graalvm.compiler.nodes.util.GraphUtil; import org.graalvm.compiler.options.OptionValues; import org.graalvm.compiler.phases.OptimisticOptimizations; import org.graalvm.compiler.phases.util.ValueMergeUtil; +import org.graalvm.compiler.serviceprovider.GraalServices; import jdk.internal.vm.compiler.word.LocationIdentity; import jdk.vm.ci.code.BailoutException; @@ -1452,13 +1453,17 @@ public class BytecodeParser implements GraphBuilderContext { protected void genInvokeInterface(int cpi, int opcode) { JavaMethod target = lookupMethod(cpi, opcode); - genInvokeInterface(target); + JavaType referencedType = lookupReferencedTypeInPool(cpi, opcode); + genInvokeInterface(referencedType, target); } - protected void genInvokeInterface(JavaMethod target) { - if (callTargetIsResolved(target)) { + protected void genInvokeInterface(JavaType referencedType, JavaMethod target) { + if (callTargetIsResolved(target) && (referencedType == null || referencedType instanceof ResolvedJavaType)) { ValueNode[] args = frameState.popArguments(target.getSignature().getParameterCount(true)); - appendInvoke(InvokeKind.Interface, (ResolvedJavaMethod) target, args); + Invoke invoke = appendInvoke(InvokeKind.Interface, (ResolvedJavaMethod) target, args); + if (invoke != null) { + invoke.callTarget().setReferencedType((ResolvedJavaType) referencedType); + } } else { handleUnresolvedInvoke(target, InvokeKind.Interface); } @@ -3923,6 +3928,16 @@ public class BytecodeParser implements GraphBuilderContext { return result; } + protected JavaType lookupReferencedTypeInPool(int cpi, int opcode) { + if (GraalServices.hasLookupReferencedType()) { + return GraalServices.lookupReferencedType(constantPool, cpi, opcode); + } + // Returning null means that we should not attempt using CHA to devirtualize or inline + // interface calls. This is a normal behavior if the JVMCI doesn't support + // {@code ConstantPool.lookupReferencedType()}. + return null; + } + protected JavaField lookupField(int cpi, int opcode) { maybeEagerlyResolve(cpi, opcode); JavaField result = constantPool.lookupField(cpi, method, opcode); diff --git a/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/CallTargetNode.java b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/CallTargetNode.java index 1e1ed79c40..19e0cf047f 100644 --- a/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/CallTargetNode.java +++ b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/CallTargetNode.java @@ -79,6 +79,46 @@ public abstract class CallTargetNode extends ValueNode implements LIRLowerable { @Input protected NodeInputList arguments; protected ResolvedJavaMethod targetMethod; + + /** + * Receiver type referenced at the interface call site. + * + * We need to distinguish the declaring type from the type referenced at the call site. We must + * use the referenced type as lower type bound when doing CHA since interface calls must throw + * exception if the receiver type is not a subtype of the reference type. + * + * Example: + * + * 
+     * interface I1 {
+     *     void foo();
+     * }
+     *
+     * interface I2 extends I1 {
+     * }
+     *
+     * void bar(I2 o) {
+     *     o.foo();
+     * }
+     *
+ * + * Here at the call site the declaring type for {@code foo()} is {@code I1}, while the + * referenced type is {@code I2}. Only receivers of type {@code T} that is {@code T <: I2} + * should be allowed at the call site. If they are not - an exception should be thrown. + * + * Since the interface types are not verified, another way to think about this call site is to + * rewrite it as follows: + * + * 
+     * void bar(Object o) {
+     *     ((I2) o).foo();
+     * }
+     *
+ * + * So, in case the receiver is not a subtype of {@code I2} an exception is thrown. + */ + protected ResolvedJavaType referencedType; + protected InvokeKind invokeKind; protected final StampPair returnStamp; @@ -117,8 +157,8 @@ public abstract class CallTargetNode extends ValueNode implements LIRLowerable { // nop } - public void setTargetMethod(ResolvedJavaMethod method) { - targetMethod = method; + public void setTargetMethod(ResolvedJavaMethod targetMethod) { + this.targetMethod = targetMethod; } /** @@ -130,6 +170,14 @@ public abstract class CallTargetNode extends ValueNode implements LIRLowerable { return targetMethod; } + public void setReferencedType(ResolvedJavaType referencedType) { + this.referencedType = referencedType; + } + + public ResolvedJavaType referencedType() { + return referencedType; + } + public InvokeKind invokeKind() { return invokeKind; } diff --git a/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/java/MethodCallTargetNode.java b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/java/MethodCallTargetNode.java index 93cf4766a4..695d851ab0 100644 --- a/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/java/MethodCallTargetNode.java +++ b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/java/MethodCallTargetNode.java @@ -183,37 +183,38 @@ public class MethodCallTargetNode extends CallTargetNode implements IterableNode Assumptions assumptions = graph().getAssumptions(); /* * Even though we are not registering an assumption (see comment below), the optimization is - * only valid when speculative optimizations are enabled. + * only valid when speculative optimizations are enabled. We need to check the invoke kind + * to avoid recursive simplification for virtual interface methods calls. */ - if (invokeKind().isIndirect() && invokeKind().isInterface() && assumptions != null) { - + if (invokeKind().isInterface() && assumptions != null) { // check if the type of the receiver can narrow the result ValueNode receiver = receiver(); // try to turn a interface call into a virtual call ResolvedJavaType declaredReceiverType = targetMethod().getDeclaringClass(); - - /* - * We need to check the invoke kind to avoid recursive simplification for virtual - * interface methods calls. - */ - if (declaredReceiverType.isInterface()) { - ResolvedJavaType singleImplementor = declaredReceiverType.getSingleImplementor(); - if (singleImplementor != null && !singleImplementor.equals(declaredReceiverType)) { - TypeReference speculatedType = TypeReference.createTrusted(assumptions, singleImplementor); - if (tryCheckCastSingleImplementor(receiver, speculatedType)) { - return; + ResolvedJavaType referencedReceiverType = referencedType(); + if (referencedReceiverType != null) { + if (declaredReceiverType.isInterface()) { + ResolvedJavaType singleImplementor = referencedReceiverType.getSingleImplementor(); + // If singleImplementor is equal to declaredReceiverType it means that there are + // multiple implementors. + if (singleImplementor != null && !singleImplementor.equals(declaredReceiverType)) { + TypeReference speculatedType = TypeReference.createTrusted(assumptions, singleImplementor); + if (tryCheckCastSingleImplementor(receiver, speculatedType)) { + return; + } } } - } - if (receiver instanceof UncheckedInterfaceProvider) { - UncheckedInterfaceProvider uncheckedInterfaceProvider = (UncheckedInterfaceProvider) receiver; - Stamp uncheckedStamp = uncheckedInterfaceProvider.uncheckedStamp(); - if (uncheckedStamp != null) { - TypeReference speculatedType = StampTool.typeReferenceOrNull(uncheckedStamp); - if (speculatedType != null) { - tryCheckCastSingleImplementor(receiver, speculatedType); + if (receiver instanceof UncheckedInterfaceProvider) { + UncheckedInterfaceProvider uncheckedInterfaceProvider = (UncheckedInterfaceProvider) receiver; + Stamp uncheckedStamp = uncheckedInterfaceProvider.uncheckedStamp(); + if (uncheckedStamp != null) { + TypeReference speculatedType = StampTool.typeReferenceOrNull(uncheckedStamp); + // speculatedType must be related to the referencedReceiverType. + if (speculatedType != null && referencedReceiverType.isAssignableFrom(speculatedType.getType())) { + tryCheckCastSingleImplementor(receiver, speculatedType); + } } } } @@ -230,7 +231,7 @@ public class MethodCallTargetNode extends CallTargetNode implements IterableNode * with an invoke virtual. * * To do so we need to ensure two properties: 1) the receiver must implement the - * interface (declaredReceiverType). The verifier does not prove this so we need a + * interface (referencedReceiverType). The verifier does not prove this so we need a * dynamic check. 2) we need to ensure that there is still only one implementor of * this interface, i.e. that we are calling the right method. We could do this with * an assumption but as we need an instanceof check anyway we can verify both diff --git a/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.phases.common/src/org/graalvm/compiler/phases/common/inlining/walker/InliningData.java b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.phases.common/src/org/graalvm/compiler/phases/common/inlining/walker/InliningData.java index a9eb6d8e31..19248d5258 100644 --- a/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.phases.common/src/org/graalvm/compiler/phases/common/inlining/walker/InliningData.java +++ b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.phases.common/src/org/graalvm/compiler/phases/common/inlining/walker/InliningData.java @@ -45,6 +45,7 @@ import org.graalvm.compiler.debug.GraalError; import org.graalvm.compiler.graph.Graph; import org.graalvm.compiler.graph.Node; import org.graalvm.compiler.nodes.CallTargetNode; +import org.graalvm.compiler.nodes.CallTargetNode.InvokeKind; import org.graalvm.compiler.nodes.Invoke; import org.graalvm.compiler.nodes.NodeView; import org.graalvm.compiler.nodes.ParameterNode; @@ -189,11 +190,12 @@ public class InliningData { MethodCallTargetNode callTarget = (MethodCallTargetNode) invoke.callTarget(); ResolvedJavaMethod targetMethod = callTarget.targetMethod(); - if (callTarget.invokeKind() == CallTargetNode.InvokeKind.Special || targetMethod.canBeStaticallyBound()) { + InvokeKind invokeKind = callTarget.invokeKind(); + if (invokeKind == CallTargetNode.InvokeKind.Special || invokeKind == CallTargetNode.InvokeKind.Static || targetMethod.canBeStaticallyBound()) { return getExactInlineInfo(invoke, targetMethod); } - assert callTarget.invokeKind().isIndirect(); + assert invokeKind.isIndirect(); ResolvedJavaType holder = targetMethod.getDeclaringClass(); if (!(callTarget.receiver().stamp(NodeView.DEFAULT) instanceof ObjectStamp)) { @@ -229,21 +231,23 @@ public class InliningData { } } - AssumptionResult leafConcreteSubtype = holder.findLeafConcreteSubtype(); - if (leafConcreteSubtype != null) { - ResolvedJavaMethod resolvedMethod = leafConcreteSubtype.getResult().resolveConcreteMethod(targetMethod, contextType); - if (resolvedMethod != null) { - if (leafConcreteSubtype.canRecordTo(callTarget.graph().getAssumptions())) { - return getAssumptionInlineInfo(invoke, resolvedMethod, leafConcreteSubtype); - } else { - return getTypeCheckedAssumptionInfo(invoke, resolvedMethod, leafConcreteSubtype.getResult()); + if (invokeKind != InvokeKind.Interface) { + AssumptionResult leafConcreteSubtype = holder.findLeafConcreteSubtype(); + if (leafConcreteSubtype != null) { + ResolvedJavaMethod resolvedMethod = leafConcreteSubtype.getResult().resolveConcreteMethod(targetMethod, contextType); + if (resolvedMethod != null) { + if (leafConcreteSubtype.canRecordTo(callTarget.graph().getAssumptions())) { + return getAssumptionInlineInfo(invoke, resolvedMethod, leafConcreteSubtype); + } else { + return getTypeCheckedAssumptionInfo(invoke, resolvedMethod, leafConcreteSubtype.getResult()); + } } } - } - AssumptionResult concrete = holder.findUniqueConcreteMethod(targetMethod); - if (concrete != null && concrete.canRecordTo(callTarget.graph().getAssumptions())) { - return getAssumptionInlineInfo(invoke, concrete.getResult(), concrete); + AssumptionResult concrete = holder.findUniqueConcreteMethod(targetMethod); + if (concrete != null && concrete.canRecordTo(callTarget.graph().getAssumptions())) { + return getAssumptionInlineInfo(invoke, concrete.getResult(), concrete); + } } // type check based inlining diff --git a/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.replacements/src/org/graalvm/compiler/replacements/classfile/ClassfileConstantPool.java b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.replacements/src/org/graalvm/compiler/replacements/classfile/ClassfileConstantPool.java index c6a6ee8dbe..bd3c8c1fb7 100644 --- a/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.replacements/src/org/graalvm/compiler/replacements/classfile/ClassfileConstantPool.java +++ b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.replacements/src/org/graalvm/compiler/replacements/classfile/ClassfileConstantPool.java @@ -45,12 +45,15 @@ import jdk.vm.ci.meta.JavaType; import jdk.vm.ci.meta.ResolvedJavaMethod; import jdk.vm.ci.meta.Signature; -class ClassfileConstantPool implements ConstantPool { +class ClassfileConstantPool implements ConstantPool, ConstantPoolPatch { final ClassfileConstant[] entries; final ClassfileBytecodeProvider context; public static class Bytecodes { + public static final int LDC = 18; // 0x12 + public static final int LDC_W = 19; // 0x13 + public static final int LDC2_W = 20; // 0x14 public static final int GETSTATIC = 178; // 0xB2 public static final int PUTSTATIC = 179; // 0xB3 public static final int GETFIELD = 180; // 0xB4 @@ -60,6 +63,12 @@ class ClassfileConstantPool implements ConstantPool { public static final int INVOKESTATIC = 184; // 0xB8 public static final int INVOKEINTERFACE = 185; // 0xB9 public static final int INVOKEDYNAMIC = 186; // 0xBA + public static final int NEW = 187; // 0xBB + public static final int NEWARRAY = 188; // 0xBC + public static final int ANEWARRAY = 189; // 0xBD + public static final int CHECKCAST = 192; // 0xC0 + public static final int INSTANCEOF = 193; // 0xC1 + public static final int MULTIANEWARRAY = 197; // 0xC5 } ClassfileConstantPool(DataInputStream stream, ClassfileBytecodeProvider context) throws IOException { @@ -160,6 +169,35 @@ class ClassfileConstantPool implements ConstantPool { return get(ClassRef.class, index).resolve(this); } + @Override + public JavaType lookupReferencedType(int index, int opcode) { + switch (opcode) { + case Bytecodes.CHECKCAST: + case Bytecodes.INSTANCEOF: + case Bytecodes.NEW: + case Bytecodes.ANEWARRAY: + case Bytecodes.MULTIANEWARRAY: + case Bytecodes.LDC: + case Bytecodes.LDC_W: + case Bytecodes.LDC2_W: + return get(ClassRef.class, index).resolve(this); + case Bytecodes.GETSTATIC: + case Bytecodes.PUTSTATIC: + case Bytecodes.GETFIELD: + case Bytecodes.PUTFIELD: + FieldRef f = get(FieldRef.class, index); + return get(ClassRef.class, f.classIndex).resolve(this); + case Bytecodes.INVOKEVIRTUAL: + case Bytecodes.INVOKESPECIAL: + case Bytecodes.INVOKESTATIC: + case Bytecodes.INVOKEINTERFACE: + ExecutableRef e = get(ExecutableRef.class, index); + return get(ClassRef.class, e.classIndex).resolve(this); + default: + throw GraalError.shouldNotReachHere("Unexpected opcode: " + opcode); + } + } + @Override public String lookupUtf8(int index) { return ((Utf8) entries[index]).value; diff --git a/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.replacements/src/org/graalvm/compiler/replacements/classfile/ConstantPoolPatch.java b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.replacements/src/org/graalvm/compiler/replacements/classfile/ConstantPoolPatch.java new file mode 100644 index 0000000000..9de7f89949 --- /dev/null +++ b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.replacements/src/org/graalvm/compiler/replacements/classfile/ConstantPoolPatch.java @@ -0,0 +1,31 @@ +/* + * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ +package org.graalvm.compiler.replacements.classfile; + +import jdk.vm.ci.meta.JavaType; + +public interface ConstantPoolPatch { + JavaType lookupReferencedType(int index, int opcode); +} diff --git a/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.serviceprovider/src/org/graalvm/compiler/serviceprovider/GraalServices.java b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.serviceprovider/src/org/graalvm/compiler/serviceprovider/GraalServices.java index 65fc285dcd..a79df424ac 100644 --- a/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.serviceprovider/src/org/graalvm/compiler/serviceprovider/GraalServices.java +++ b/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.serviceprovider/src/org/graalvm/compiler/serviceprovider/GraalServices.java @@ -28,12 +28,15 @@ import static java.lang.Thread.currentThread; import java.io.IOException; import java.io.InputStream; +import java.lang.reflect.Method; import java.util.Iterator; import java.util.List; import java.util.ServiceConfigurationError; import java.util.ServiceLoader; import java.util.concurrent.atomic.AtomicLong; +import jdk.vm.ci.meta.ConstantPool; +import jdk.vm.ci.meta.JavaType; import jdk.vm.ci.services.JVMCIPermission; import jdk.vm.ci.services.Services; @@ -331,4 +334,33 @@ public final class GraalServices { } return jmx.getInputArguments(); } + + private static final Method constantPoolLookupReferencedType; + + static { + Method lookupReferencedType = null; + Class constantPool = ConstantPool.class; + try { + lookupReferencedType = constantPool.getDeclaredMethod("lookupReferencedType", Integer.TYPE, Integer.TYPE); + } catch (NoSuchMethodException e) { + } + constantPoolLookupReferencedType = lookupReferencedType; + } + + public static JavaType lookupReferencedType(ConstantPool constantPool, int cpi, int opcode) { + if (constantPoolLookupReferencedType != null) { + try { + return (JavaType) constantPoolLookupReferencedType.invoke(constantPool, cpi, opcode); + } catch (Error e) { + throw e; + } catch (Throwable throwable) { + throw new InternalError(throwable); + } + } + throw new InternalError("This JVMCI version doesn't support ConstantPool.lookupReferencedType()"); + } + + public static boolean hasLookupReferencedType() { + return constantPoolLookupReferencedType != null; + } } -- GitLab