diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java index 5a74c2cd24f..2e996fe91f8 100644 --- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java +++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java @@ -32,6 +32,7 @@ import org.codehaus.groovy.ast.GenericsType; import org.codehaus.groovy.ast.InnerClassNode; import org.codehaus.groovy.ast.InterfaceHelperClassNode; +import org.codehaus.groovy.ast.IntersectionTypeClassNode; import org.codehaus.groovy.ast.MethodNode; import org.codehaus.groovy.ast.ModuleNode; import org.codehaus.groovy.ast.PackageNode; @@ -56,7 +57,6 @@ import org.codehaus.groovy.ast.expr.Expression; import org.codehaus.groovy.ast.expr.FieldExpression; import org.codehaus.groovy.ast.expr.GStringExpression; -import org.codehaus.groovy.ast.IntersectionTypeClassNode; import org.codehaus.groovy.ast.expr.LambdaExpression; import org.codehaus.groovy.ast.expr.ListExpression; import org.codehaus.groovy.ast.expr.MapEntryExpression; @@ -103,6 +103,7 @@ import org.codehaus.groovy.classgen.asm.MopWriter; import org.codehaus.groovy.classgen.asm.OperandStack; import org.codehaus.groovy.classgen.asm.OptimizingStatementWriter; +import org.codehaus.groovy.classgen.asm.PeepholeOptimizingMethodVisitor; import org.codehaus.groovy.classgen.asm.WriterController; import org.codehaus.groovy.classgen.asm.WriterControllerFactory; import org.codehaus.groovy.control.SourceUnit; @@ -622,11 +623,15 @@ protected void visitConstructorOrMethod(final MethodNode node, final boolean isC receiver = parameters[0]; // non-static method or inner class ctor parameters = Arrays.copyOfRange(parameters, 1, parameters.length); } - MethodVisitor mv = classVisitor.visitMethod( - node.getModifiers() | (isVargs(parameters) ? ACC_VARARGS : 0), node.getName(), - BytecodeHelper.getMethodDescriptor(node.getReturnType(), parameters), - BytecodeHelper.getGenericsMethodSignature(node), - buildExceptions(node.getExceptions())); + + MethodVisitor mv = new PeepholeOptimizingMethodVisitor( + classVisitor.visitMethod( + node.getModifiers() | (isVargs(parameters) ? ACC_VARARGS : 0), + node.getName(), + BytecodeHelper.getMethodDescriptor(node.getReturnType(), parameters), + BytecodeHelper.getGenericsMethodSignature(node), + buildExceptions(node.getExceptions()))); + controller.setMethodVisitor(mv); controller.resetLineNumber(); @@ -678,10 +683,10 @@ protected void visitConstructorOrMethod(final MethodNode node, final boolean isC mv.visitMaxs(0, 0); } catch (Throwable t) { Writer writer = null; - if (mv instanceof TraceMethodVisitor) { + if (mv instanceof TraceMethodVisitor tmv) { writer = new StringBuilderWriter(); PrintWriter p = new PrintWriter(writer); - ((TraceMethodVisitor) mv).p.print(p); + tmv.p.print(p); p.flush(); } StringBuilder message = new StringBuilder(64); @@ -2329,11 +2334,12 @@ public void visitListExpression(final ListExpression expression) { while (index IFEQ; + case IF_ICMPNE -> IFNE; + case IF_ICMPGE -> IFGE; + case IF_ICMPGT -> IFGT; + case IF_ICMPLE -> IFLE; + case IF_ICMPLT -> IFLT; + default -> NO_OPCODE; + }; + if (replacement == NO_OPCODE) { + return false; + } + + clearPendingLoad(); + super.visitJumpInsn(replacement, label); + return true; + } + + private boolean tryRemovePendingLoad(final int opcode) { + if (pendingLoadKind == PendingLoadKind.NONE) { + return false; + } + + int popSize = stackSizeForPop(opcode); + if (popSize == 0) { + return false; + } + + if (pendingLoadKind == PendingLoadKind.CHECKCAST) { + if (opcode != POP) { + return false; + } + clearPendingLoad(); + super.visitInsn(POP); + return true; + } + + if (stackSizeForPendingLoad() != popSize) { + return false; + } + + if (pendingLoadKind == PendingLoadKind.VARIABLE && pendingLoadHasIinc) { + super.visitIincInsn(pendingLoadVar, pendingLoadIncrement); + } + clearPendingLoad(); + return true; + } + + private boolean tryDropPendingLoadOnReturn(final int opcode) { + if (opcode != RETURN || pendingLoadKind == PendingLoadKind.NONE) { + return false; + } + + if (pendingLoadKind == PendingLoadKind.VARIABLE && pendingLoadHasIinc) { + super.visitIincInsn(pendingLoadVar, pendingLoadIncrement); + } + clearPendingLoad(); + super.visitInsn(RETURN); + return true; + } + + private boolean tryRemovePendingDupStore(final int opcode) { + if (pendingStoreKind == PendingStoreKind.NONE) { + return false; + } + + int popSize = stackSizeForPop(opcode); + if (popSize == 0 || stackSizeForDup() != popSize || stackSizeForPendingStore() != popSize) { + return false; + } + + emitPendingStore(); + clearPendingDupStore(); + return true; + } + + private void bufferConstant(final Object value) { + clearPendingLoad(); + pendingLoadKind = PendingLoadKind.CONSTANT; + pendingConstant = value; + } + + private void bufferVariableLoad(final int opcode, final int varIndex) { + clearPendingLoad(); + pendingLoadKind = PendingLoadKind.VARIABLE; + pendingLoadOpcode = opcode; + pendingLoadVar = varIndex; + } + + private void bufferCheckcast(final String descriptor) { + clearPendingLoad(); + pendingLoadKind = PendingLoadKind.CHECKCAST; + pendingCheckcastDescriptor = descriptor; + } + + private void bufferVariableStore(final int opcode, final int varIndex) { + pendingStoreKind = PendingStoreKind.VARIABLE; + pendingStoreOpcode = opcode; + pendingStoreVar = varIndex; + } + + private void bufferStaticStore(final int opcode, final String owner, final String name, final String descriptor) { + pendingStoreKind = PendingStoreKind.STATIC_FIELD; + pendingStoreOpcode = opcode; + pendingStoreOwner = owner; + pendingStoreName = name; + pendingStoreDescriptor = descriptor; + } + + private void emitPendingStore() { + switch (pendingStoreKind) { + case VARIABLE: + super.visitVarInsn(pendingStoreOpcode, pendingStoreVar); + break; + case STATIC_FIELD: + super.visitFieldInsn(pendingStoreOpcode, pendingStoreOwner, pendingStoreName, pendingStoreDescriptor); + break; + case NONE: + default: + } + } + + private void clearPendingLoad() { + pendingLoadKind = PendingLoadKind.NONE; + pendingConstant = null; + pendingLoadOpcode = NO_OPCODE; + pendingLoadVar = NO_OPCODE; + pendingLoadHasIinc = false; + pendingLoadIncrement = 0; + pendingCheckcastDescriptor = null; + } + + private void clearPendingDupStore() { + pendingDupOpcode = NO_OPCODE; + pendingStoreKind = PendingStoreKind.NONE; + pendingStoreOpcode = NO_OPCODE; + pendingStoreVar = NO_OPCODE; + pendingStoreOwner = null; + pendingStoreName = null; + pendingStoreDescriptor = null; + } + + private int stackSizeForPendingLoad() { + return switch (pendingLoadKind) { + case VARIABLE -> stackSizeForLoadOpcode(pendingLoadOpcode); + case CONSTANT -> (pendingConstant instanceof Long || pendingConstant instanceof Double) ? 2 : 1; + case CHECKCAST -> 1; + case NONE -> 0; + }; + } + + private int stackSizeForDup() { + return switch (pendingDupOpcode) { + case DUP -> 1; + case DUP2 -> 2; + default -> 0; + }; + } + + private int stackSizeForPendingStore() { + return switch (pendingStoreKind) { + case VARIABLE -> stackSizeForStoreOpcode(pendingStoreOpcode); + case STATIC_FIELD -> stackSizeForFieldDescriptor(pendingStoreDescriptor); + case NONE -> 0; + }; + } + + private static int stackSizeForPop(final int opcode) { + return switch (opcode) { + case POP -> 1; + case POP2 -> 2; + default -> 0; + }; + } + + private static int stackSizeForLoadOpcode(final int opcode) { + return switch (opcode) { + case LLOAD, DLOAD -> 2; + case ILOAD, FLOAD, ALOAD -> 1; + default -> 0; + }; + } + + private static int stackSizeForStoreOpcode(final int opcode) { + return switch (opcode) { + case LSTORE, DSTORE -> 2; + case ISTORE, FSTORE, ASTORE -> 1; + default -> 0; + }; + } + + private static int stackSizeForFieldDescriptor(final String descriptor) { + char first = descriptor.charAt(0); + return (first == 'J' || first == 'D') ? 2 : 1; + } + + private static boolean isLoadOpcode(final int opcode) { + return switch (opcode) { + case ILOAD, LLOAD, FLOAD, DLOAD, ALOAD -> true; + default -> false; + }; + } + + private static boolean isStoreOpcode(final int opcode) { + return switch (opcode) { + case ISTORE, LSTORE, FSTORE, DSTORE, ASTORE -> true; + default -> false; + }; + } + + private void emitConstant(final Object value) { + if (value == null) { + super.visitInsn(ACONST_NULL); + return; + } + + if (value instanceof BigDecimal || value instanceof BigInteger) { + emitStringConstructedConstant(value); + return; + } + + if (value instanceof Integer intValue) { + emitIntConstant(intValue); + return; + } + + if (value instanceof Long longValue) { + emitLongConstant(longValue); + return; + } + + if (value instanceof Float floatValue) { + emitFloatConstant(floatValue); + return; + } + + if (value instanceof Double doubleValue) { + emitDoubleConstant(doubleValue); + return; + } + + super.visitLdcInsn(value); + } + + private void emitStringConstructedConstant(final Object value) { + String type = (value instanceof BigDecimal ? BIG_DECIMAL_TYPE : BIG_INTEGER_TYPE); + super.visitTypeInsn(NEW, type); + super.visitInsn(DUP); + super.visitLdcInsn(value.toString()); + super.visitMethodInsn(INVOKESPECIAL, type, "", STRING_CTOR_DESCRIPTOR, false); + } + + private void emitIntConstant(final int value) { + switch (value) { + case -1: + super.visitInsn(ICONST_M1); + return; + case 0: + super.visitInsn(ICONST_0); + return; + case 1: + super.visitInsn(ICONST_1); + return; + case 2: + super.visitInsn(ICONST_2); + return; + case 3: + super.visitInsn(ICONST_3); + return; + case 4: + super.visitInsn(ICONST_4); + return; + case 5: + super.visitInsn(ICONST_5); + return; + default: + } + + if (value >= Byte.MIN_VALUE && value <= Byte.MAX_VALUE) { + super.visitIntInsn(BIPUSH, value); + } else if (value >= Short.MIN_VALUE && value <= Short.MAX_VALUE) { + super.visitIntInsn(SIPUSH, value); + } else { + super.visitLdcInsn(value); + } + } + + private void emitLongConstant(final long value) { + if (value == 0L) { + super.visitInsn(LCONST_0); + } else if (value == 1L) { + super.visitInsn(LCONST_1); + } else { + super.visitLdcInsn(value); + } + } + + private void emitFloatConstant(final float value) { + int rawBits = Float.floatToRawIntBits(value); + if (rawBits == Float.floatToRawIntBits(0f)) { + super.visitInsn(FCONST_0); + } else if (rawBits == Float.floatToRawIntBits(1f)) { + super.visitInsn(FCONST_1); + } else if (rawBits == Float.floatToRawIntBits(2f)) { + super.visitInsn(FCONST_2); + } else { + super.visitLdcInsn(value); + } + } + + private void emitDoubleConstant(final double value) { + long rawBits = Double.doubleToRawLongBits(value); + if (rawBits == Double.doubleToRawLongBits(0d)) { + super.visitInsn(DCONST_0); + } else if (rawBits == Double.doubleToRawLongBits(1d)) { + super.visitInsn(DCONST_1); + } else { + super.visitLdcInsn(value); + } + } +} diff --git a/src/test/groovy/org/codehaus/groovy/classgen/BytecodeHelperTest.groovy b/src/test/groovy/org/codehaus/groovy/classgen/BytecodeHelperTest.groovy index 150437beeda..b6c579f183d 100644 --- a/src/test/groovy/org/codehaus/groovy/classgen/BytecodeHelperTest.groovy +++ b/src/test/groovy/org/codehaus/groovy/classgen/BytecodeHelperTest.groovy @@ -23,12 +23,20 @@ import org.codehaus.groovy.ast.ClassNode import org.codehaus.groovy.ast.MethodNode import org.codehaus.groovy.ast.Parameter import org.codehaus.groovy.ast.stmt.EmptyStatement +import org.codehaus.groovy.classgen.asm.AbstractBytecodeTestCase import org.codehaus.groovy.classgen.asm.BytecodeHelper import org.junit.jupiter.api.Test +import org.objectweb.asm.ClassWriter +import org.objectweb.asm.MethodVisitor +import org.objectweb.asm.util.Printer +import static org.objectweb.asm.Opcodes.ACC_PUBLIC +import static org.objectweb.asm.Opcodes.ACC_STATIC +import static org.objectweb.asm.Opcodes.RETURN +import static org.objectweb.asm.Opcodes.V17 import static org.junit.jupiter.api.Assertions.assertEquals -class BytecodeHelperTest { +class BytecodeHelperTest extends AbstractBytecodeTestCase { @Test void testTypeName() { @@ -72,4 +80,49 @@ class BytecodeHelperTest { assertEquals("()Z", BytecodeHelper.getMethodDescriptor(new MethodNode('test', 0, ClassHelper.boolean_TYPE, Parameter.EMPTY_ARRAY, [] as ClassNode[], EmptyStatement.INSTANCE))) } + + @Test + void testPushConstantUsesCompactIntegerInstructions() { + def bytecode = emitBytecode { + BytecodeHelper.pushConstant(it, -1) + BytecodeHelper.pushConstant(it, 6) + BytecodeHelper.pushConstant(it, 200) + BytecodeHelper.pushConstant(it, 70_000) + it.visitInsn(RETURN) + } + + assert opcodeLines(bytecode) == [ + 'ICONST_M1', + 'BIPUSH 6', + 'SIPUSH 200', + 'LDC 70000', + 'RETURN', + ] + } + + private emitBytecode(@DelegatesTo(MethodVisitor) Closure emitter) { + def writer = new ClassWriter(ClassWriter.COMPUTE_MAXS) + writer.visit(V17, ACC_PUBLIC, 'BytecodeHelperTestSupport', null, 'java/lang/Object', null) + + MethodVisitor mv = writer.visitMethod(ACC_PUBLIC | ACC_STATIC, 'sample', '()V', null, null) + mv.visitCode() + emitter.delegate = mv + emitter.resolveStrategy = Closure.DELEGATE_FIRST + if (emitter.maximumNumberOfParameters == 0) { + emitter.call() + } else { + emitter.call(mv) + } + mv.visitMaxs(0, 0) + mv.visitEnd() + + writer.visitEnd() + extractSequence(writer.toByteArray(), [method: 'sample']) + } + + private static List opcodeLines(bytecode) { + bytecode.instructions.findAll { line -> + Printer.OPCODES.any { opcode -> opcode != null && line.startsWith(opcode) } + } + } } diff --git a/src/test/groovy/org/codehaus/groovy/classgen/asm/BinaryOperationsTest.groovy b/src/test/groovy/org/codehaus/groovy/classgen/asm/BinaryOperationsTest.groovy index 2e8f751783d..cc857c57615 100644 --- a/src/test/groovy/org/codehaus/groovy/classgen/asm/BinaryOperationsTest.groovy +++ b/src/test/groovy/org/codehaus/groovy/classgen/asm/BinaryOperationsTest.groovy @@ -87,7 +87,12 @@ final class BinaryOperationsTest extends AbstractBytecodeTestCase { "ICONST_$it", ]) } - [-1, 6,Byte.MIN_VALUE,Byte.MAX_VALUE].each { + assert compile("""\ + int a = -1 + """).hasStrictSequence([ + "ICONST_M1", + ]) + [6,Byte.MIN_VALUE,Byte.MAX_VALUE].each { assert compile("""\ int a = $it """).hasStrictSequence([ @@ -110,6 +115,26 @@ final class BinaryOperationsTest extends AbstractBytecodeTestCase { } } + @Test + void testPrimitiveZeroConstants() { + assumeFalse(config.indyEnabled) + assert compile("""\ + long a = 0L + """).hasStrictSequence([ + 'LCONST_0', + ]) + assert compile("""\ + float a = 0f + """).hasStrictSequence([ + 'FCONST_0', + ]) + assert compile("""\ + double a = 0d + """).hasStrictSequence([ + 'DCONST_0', + ]) + } + @Test void testCharXor() { assumeFalse(config.indyEnabled) diff --git a/src/test/groovy/org/codehaus/groovy/classgen/asm/PeepholeBytecodeIntegrationTest.groovy b/src/test/groovy/org/codehaus/groovy/classgen/asm/PeepholeBytecodeIntegrationTest.groovy new file mode 100644 index 00000000000..022e6db99be --- /dev/null +++ b/src/test/groovy/org/codehaus/groovy/classgen/asm/PeepholeBytecodeIntegrationTest.groovy @@ -0,0 +1,104 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.codehaus.groovy.classgen.asm + +import org.codehaus.groovy.control.CompilationUnit +import org.codehaus.groovy.control.CompilerConfiguration +import org.codehaus.groovy.control.Phases +import org.junit.jupiter.api.Test + +final class PeepholeBytecodeIntegrationTest extends AbstractBytecodeTestCase { + + @Test + void testListHelperMethodsUseCompactIndexes() { + def constants = (0..1000).join(', ') + def bytecode = compile(method: '$createListEntry_1', """\ + def values = [$constants] + """) + + assert bytecode.hasStrictSequence([ + 'ALOAD 0', + 'ICONST_0', + 'ICONST_0', + 'INVOKESTATIC java/lang/Integer.valueOf', + ]) + assert bytecode.hasSequence([ + 'ALOAD 0', + 'BIPUSH 6', + 'BIPUSH 6', + 'INVOKESTATIC java/lang/Integer.valueOf', + ]) + } + + @Test + void testCallSiteArrayHelpersUseCompactIndexes() { + def bytecode = compileClassic(method: '$createCallSiteArray_1', '''\ + def text = 'x' + text.toUpperCase() + text.toLowerCase() + text.trim() + ''') + + assert clazz.declaredMethods*.name.contains('$createCallSiteArray_1') + assert bytecode.hasStrictSequence([ + 'ALOAD 0', + 'ICONST_0', + 'LDC', + ]) + assert bytecode.hasSequence([ + 'ALOAD 0', + 'ICONST_1', + 'LDC', + ]) + } + + protected InstructionSequence compileClassic(Map options = [:], final String scriptText) { + options = [method: 'run', classNamePattern: '.*script', *: options] + sequence = null + clazz = null + classBytes = null + + def cu = new CompilationUnit(new CompilerConfiguration(optimizationOptions: [indy: false])) + def su = cu.addSource('script', scriptText) + cu.compile(Phases.CONVERSION) + if (options.conversionAction != null) { + options.conversionAction(su) + } + cu.compile(Phases.CLASS_GENERATION) + + for (gc in cu.classes) { + if (gc.name ==~ options.classNamePattern) { + classBytes = gc.bytes + sequence = extractSequence(gc.bytes, options) + } + } + if (sequence == null && cu.classes) { + def gc = cu.classes.find { it.name == cu.firstClassNode.name } + classBytes = gc.bytes + sequence = extractSequence(gc.bytes, options) + } + for (gc in cu.classes) { + Class c = cu.classLoader.defineClass(gc.name, gc.bytes) + if (Script.isAssignableFrom(c)) { clazz = c } + c.isInterface() + } + + sequence + } +} diff --git a/src/test/groovy/org/codehaus/groovy/classgen/asm/PeepholeOptimizingMethodVisitorTest.groovy b/src/test/groovy/org/codehaus/groovy/classgen/asm/PeepholeOptimizingMethodVisitorTest.groovy new file mode 100644 index 00000000000..241aaff00c5 --- /dev/null +++ b/src/test/groovy/org/codehaus/groovy/classgen/asm/PeepholeOptimizingMethodVisitorTest.groovy @@ -0,0 +1,436 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.codehaus.groovy.classgen.asm + +import org.codehaus.groovy.control.CompilerConfiguration +import org.junit.jupiter.api.Test +import org.objectweb.asm.Attribute +import org.objectweb.asm.ConstantDynamic +import org.objectweb.asm.Handle +import org.objectweb.asm.Label +import org.objectweb.asm.MethodVisitor +import org.objectweb.asm.Opcodes +import org.objectweb.asm.tree.AbstractInsnNode +import org.objectweb.asm.tree.FieldInsnNode +import org.objectweb.asm.tree.IincInsnNode +import org.objectweb.asm.tree.IntInsnNode +import org.objectweb.asm.tree.LdcInsnNode +import org.objectweb.asm.tree.LookupSwitchInsnNode +import org.objectweb.asm.tree.MethodInsnNode +import org.objectweb.asm.tree.MethodNode +import org.objectweb.asm.tree.MultiANewArrayInsnNode +import org.objectweb.asm.tree.TableSwitchInsnNode +import org.objectweb.asm.tree.TypeInsnNode +import org.objectweb.asm.tree.VarInsnNode +import org.objectweb.asm.util.Printer +import org.objectweb.asm.util.Textifier +import org.objectweb.asm.util.TraceMethodVisitor + +import static org.objectweb.asm.Opcodes.ACC_PUBLIC +import static org.objectweb.asm.Opcodes.ACC_STATIC +import static org.objectweb.asm.Opcodes.RETURN + +final class PeepholeOptimizingMethodVisitorTest { + + @Test + void compactsNumericConstants() { + def bytecode = sequenceFor { + visitLdcInsn(-1) + visitLdcInsn(5) + visitLdcInsn(6) + visitIntInsn(Opcodes.SIPUSH, 120) + visitLdcInsn(0L) + visitLdcInsn(1L) + visitLdcInsn(2L) + visitLdcInsn(0f) + visitLdcInsn(1f) + visitLdcInsn(2f) + visitLdcInsn(0d) + visitLdcInsn(1d) + visitVarInsn(Opcodes.DSTORE, 0) + visitInsn(RETURN) + } + + assert opcodeLines(bytecode) == [ + 'ICONST_M1', + 'ICONST_5', + 'BIPUSH 6', + 'BIPUSH 120', + 'LCONST_0', + 'LCONST_1', + 'LDC 2L', + 'FCONST_0', + 'FCONST_1', + 'FCONST_2', + 'DCONST_0', + 'DCONST_1', + 'DSTORE 0', + 'RETURN', + ] + } + + @Test + void preservesSignedZeroConstants() { + def methodNode = methodNodeFor { + visitLdcInsn(-0.0f) + visitVarInsn(Opcodes.FSTORE, 0) + visitLdcInsn(-0.0d) + visitVarInsn(Opcodes.DSTORE, 1) + visitInsn(RETURN) + } + def constants = executableInstructions(methodNode).findAll { it instanceof LdcInsnNode }*.cst + def bytecode = traceSequence(methodNode) + + assert constants.size() == 2 + assert constants[0].equals(-0.0f) + assert constants[1].equals(-0.0d) + assert opcodeLines(bytecode) == ['LDC -0.0F', 'FSTORE 0', 'LDC -0.0D', 'DSTORE 1', 'RETURN'] + } + + @Test + void rewritesIntegerComparisonsAgainstZero() { + def zeroComparisons = [ + (Opcodes.IF_ICMPEQ): 'IFEQ', + (Opcodes.IF_ICMPNE): 'IFNE', + (Opcodes.IF_ICMPGE): 'IFGE', + (Opcodes.IF_ICMPGT): 'IFGT', + (Opcodes.IF_ICMPLE): 'IFLE', + (Opcodes.IF_ICMPLT): 'IFLT', + ] + + zeroComparisons.each { opcode, expected -> + def label = new Label() + def bytecode = sequenceFor('(I)V') { + visitVarInsn(Opcodes.ILOAD, 0) + visitLdcInsn(0) + visitJumpInsn(opcode, label) + visitInsn(RETURN) + visitLabel(label) + visitInsn(RETURN) + } + def lines = opcodeLines(bytecode) + + assert lines.size() == 4 + assert lines[0] == 'ILOAD 0' + assert lines[1].startsWith(expected) + assert lines[2..3] == ['RETURN', 'RETURN'] + assert !lines[1].startsWith(Printer.OPCODES[opcode]) + } + } + + @Test + void removesRedundantLoadsAndConstantPops() { + def bytecode = sequenceFor('(Ljava/lang/Object;J)V') { + visitVarInsn(Opcodes.ALOAD, 0) + visitInsn(Opcodes.POP) + visitVarInsn(Opcodes.LLOAD, 1) + visitInsn(Opcodes.POP2) + visitLdcInsn('unused') + visitInsn(Opcodes.POP) + visitInsn(RETURN) + } + + assert opcodeLines(bytecode) == ['RETURN'] + } + + @Test + void preservesIincWhenDroppingRedundantLoads() { + def bytecode = sequenceFor('(I)V') { + visitVarInsn(Opcodes.ILOAD, 0) + visitIincInsn(0, 1) + visitInsn(Opcodes.POP) + visitInsn(RETURN) + } + + assert opcodeLines(bytecode) == ['IINC 0 1', 'RETURN'] + } + + @Test + void removesRedundantDupStorePops() { + def bytecode = sequenceFor { + visitInsn(Opcodes.ICONST_1) + visitInsn(Opcodes.DUP) + visitVarInsn(Opcodes.ISTORE, 0) + visitInsn(Opcodes.POP) + visitLdcInsn(2L) + visitInsn(Opcodes.DUP2) + visitFieldInsn(Opcodes.PUTSTATIC, 'Owner', 'VALUE', 'J') + visitInsn(Opcodes.POP2) + visitInsn(RETURN) + } + + assert opcodeLines(bytecode) == [ + 'ICONST_1', + 'ISTORE 0', + 'LDC 2L', + 'PUTSTATIC Owner.VALUE : J', + 'RETURN', + ] + } + + @Test + void lowersBigNumberConstantsBeforeEmission() { + def bytecode = sequenceFor { + visitLdcInsn(new BigDecimal('123.45')) + visitFieldInsn(Opcodes.PUTSTATIC, 'Owner', 'DECIMAL', 'Ljava/math/BigDecimal;') + visitLdcInsn(new BigInteger('42')) + visitFieldInsn(Opcodes.PUTSTATIC, 'Owner', 'INTEGER', 'Ljava/math/BigInteger;') + visitInsn(RETURN) + } + + assert opcodeLines(bytecode) == [ + 'NEW java/math/BigDecimal', + 'DUP', + 'LDC "123.45"', + 'INVOKESPECIAL java/math/BigDecimal. (Ljava/lang/String;)V', + 'PUTSTATIC Owner.DECIMAL : Ljava/math/BigDecimal;', + 'NEW java/math/BigInteger', + 'DUP', + 'LDC "42"', + 'INVOKESPECIAL java/math/BigInteger. (Ljava/lang/String;)V', + 'PUTSTATIC Owner.INTEGER : Ljava/math/BigInteger;', + 'RETURN', + ] + } + + @Test + void compactsBufferedPrimitiveOpcodesAndLargeFallbacks() { + def bytecode = sequenceFor { + visitInsn(Opcodes.ICONST_M1) + visitInsn(Opcodes.ICONST_3) + visitInsn(Opcodes.ICONST_4) + visitInsn(Opcodes.ICONST_5) + visitInsn(Opcodes.LCONST_1) + visitInsn(Opcodes.FCONST_1) + visitInsn(Opcodes.FCONST_2) + visitInsn(Opcodes.DCONST_1) + visitLdcInsn(300) + visitLdcInsn(70_000) + visitVarInsn(Opcodes.ISTORE, 0) + visitInsn(RETURN) + } + + assert opcodeLines(bytecode) == [ + 'ICONST_M1', + 'ICONST_3', + 'ICONST_4', + 'ICONST_5', + 'LCONST_1', + 'FCONST_1', + 'FCONST_2', + 'DCONST_1', + 'SIPUSH 300', + 'LDC 70000', + 'ISTORE 0', + 'RETURN', + ] + } + + @Test + void flushesPendingStateBeforeStructuralVisitors() { + def defaultLabel = new Label() + def caseLabel = new Label() + def methodNode = methodNodeFor { + visitLdcInsn(1) + visitAttribute(new Attribute('Test') { }) + visitLdcInsn(new ConstantDynamic('dyn', 'I', new Handle(Opcodes.H_INVOKESTATIC, 'Owner', 'bootstrap', '()V', false))) + visitLdcInsn(2) + visitFrame(Opcodes.F_SAME, 0, null, 0, null) + visitLdcInsn(3) + visitTableSwitchInsn(0, 0, defaultLabel, caseLabel) + visitLabel(caseLabel) + visitLdcInsn(4) + visitLookupSwitchInsn(defaultLabel, [1] as int[], [caseLabel] as Label[]) + visitLabel(defaultLabel) + visitLdcInsn(1) + visitMultiANewArrayInsn('[[I', 1) + visitInsn(RETURN) + } + def instructions = executableInstructions(methodNode) + def bytecode = traceSequence(methodNode) + + assert render(instructions[0]) == 'ICONST_1' + assert instructions[1] instanceof LdcInsnNode && ((LdcInsnNode) instructions[1]).cst instanceof ConstantDynamic + assert bytecode.hasStrictSequence(['ICONST_2', 'FRAME SAME', 'ICONST_3', 'TABLESWITCH']) + assert bytecode.hasSequence(['ICONST_4', 'LOOKUPSWITCH']) + assert bytecode.hasSequence(['ICONST_1', 'MULTIANEWARRAY']) + assert instructions[4] instanceof TableSwitchInsnNode + assert instructions[6] instanceof LookupSwitchInsnNode + assert instructions[8] instanceof MultiANewArrayInsnNode + assert render(instructions[9]) == 'RETURN' + assert methodNode.attrs*.type == ['Test'] + } + + @Test + void flushesPendingStateBeforeTypeAnnotationVisitors() { + def start = new Label() + def end = new Label() + def handler = new Label() + def methodNode = methodNodeFor('(Ljava/lang/Object;)V') { + visitLabel(start) + visitVarInsn(Opcodes.ALOAD, 0) + visitTypeInsn(Opcodes.CHECKCAST, 'java/lang/String') + visitInsnAnnotation(0, null, 'Ljava/lang/Deprecated;', true)?.visitEnd() + visitLabel(end) + visitTryCatchBlock(start, end, handler, 'java/lang/RuntimeException') + visitTryCatchAnnotation(1, null, 'Ljava/lang/Deprecated;', true)?.visitEnd() + visitLocalVariable('value', 'Ljava/lang/Object;', null, start, end, 0) + visitLocalVariableAnnotation(2, null, [start] as Label[], [end] as Label[], [0] as int[], 'Ljava/lang/Deprecated;', true)?.visitEnd() + visitLabel(handler) + visitInsn(RETURN) + } + def bytecode = traceSequence(methodNode) + def checkcast = executableInstructions(methodNode).find { it instanceof TypeInsnNode } + + assert opcodeLines(bytecode) == ['ALOAD 0', 'CHECKCAST java/lang/String', 'RETURN'] + assert checkcast.visibleTypeAnnotations?.size() == 1 + assert methodNode.tryCatchBlocks.size() == 1 + assert methodNode.tryCatchBlocks[0].visibleTypeAnnotations?.size() == 1 + assert methodNode.visibleLocalVariableAnnotations?.size() == 1 + } + + @Test + void flushesIncrementedLoadsWhenAnotherInstructionNeedsThem() { + def bytecode = sequenceFor('(I)V') { + visitVarInsn(Opcodes.ILOAD, 0) + visitIincInsn(0, 1) + visitInsn(Opcodes.ICONST_1) + visitInsn(Opcodes.POP) + visitInsn(RETURN) + } + + assert opcodeLines(bytecode) == ['ILOAD 0', 'IINC 0 1', 'RETURN'] + } + + @Test + void dropsIncrementedLoadsOnReturn() { + def bytecode = sequenceFor('(I)V', { -> + visitVarInsn(Opcodes.ILOAD, 0) + visitIincInsn(0, 1) + visitInsn(RETURN) + }) + + assert opcodeLines(bytecode) == ['IINC 0 1', 'RETURN'] + } + + @Test + void removesStandaloneCheckcastBeforePop() { + def bytecode = sequenceFor('(Ljava/lang/Object;)V') { + visitVarInsn(Opcodes.ALOAD, 0) + visitTypeInsn(Opcodes.CHECKCAST, 'java/lang/String') + visitInsn(Opcodes.POP) + visitInsn(RETURN) + } + + assert opcodeLines(bytecode) == ['ALOAD 0', 'POP', 'RETURN'] + } + + @Test + void flushesPendingDupStoresWhenTheValueIsKept() { + def bytecode = sequenceFor { + visitInsn(Opcodes.ICONST_1) + visitInsn(Opcodes.DUP) + visitVarInsn(Opcodes.ISTORE, 0) + visitInsn(Opcodes.NOP) + visitLdcInsn(3L) + visitInsn(Opcodes.DUP2) + visitVarInsn(Opcodes.LSTORE, 1) + visitInsn(Opcodes.POP2) + visitInsn(RETURN) + } + + assert opcodeLines(bytecode) == [ + 'ICONST_1', + 'DUP', + 'ISTORE 0', + 'NOP', + 'LDC 3L', + 'LSTORE 1', + 'RETURN', + ] + } + + private InstructionSequence sequenceFor(String descriptor = '()V', @DelegatesTo(MethodVisitor) Closure emitter) { + traceSequence(methodNodeFor(descriptor, emitter)) + } + + private static InstructionSequence traceSequence(MethodNode methodNode) { + def out = new StringWriter() + def printer = new TraceMethodVisitor(new Textifier()) + methodNode.accept(printer) + def writer = new PrintWriter(out) + printer.p.print(writer) + writer.flush() + new InstructionSequence(instructions: out.toString().split('\n')*.trim().findAll()) + } + + private static MethodNode methodNodeFor(String descriptor = '()V', @DelegatesTo(MethodVisitor) Closure emitter) { + def methodNode = new MethodNode(CompilerConfiguration.ASM_API_VERSION, ACC_PUBLIC | ACC_STATIC, 'sample', descriptor, null, null) + MethodVisitor visitor = new PeepholeOptimizingMethodVisitor(methodNode) + visitor.visitCode() + emitter.delegate = visitor + emitter.resolveStrategy = Closure.DELEGATE_FIRST + if (emitter.maximumNumberOfParameters == 0) { + emitter.call() + } else { + emitter.call(visitor) + } + visitor.visitMaxs(0, 0) + visitor.visitEnd() + methodNode + } + + private static List executableInstructions(MethodNode methodNode) { + methodNode.instructions.toArray().findAll { AbstractInsnNode insn -> insn.opcode >= 0 } + } + + private static List opcodeLines(InstructionSequence bytecode) { + bytecode.instructions.findAll { line -> + Printer.OPCODES.any { opcode -> opcode != null && line.startsWith(opcode) } + } + } + + private static String render(AbstractInsnNode insn) { + switch (insn) { + case FieldInsnNode: + return "${Printer.OPCODES[insn.opcode]} ${insn.owner}.${insn.name} : ${insn.desc}" + case IincInsnNode: + return "IINC ${insn.var} ${insn.incr}" + case IntInsnNode: + return "${Printer.OPCODES[insn.opcode]} ${insn.operand}" + case LdcInsnNode: + return "LDC ${formatConstant(insn.cst)}" + case MethodInsnNode: + return "${Printer.OPCODES[insn.opcode]} ${insn.owner}.${insn.name} ${insn.desc}" + case TypeInsnNode: + return "${Printer.OPCODES[insn.opcode]} ${insn.desc}" + case VarInsnNode: + return "${Printer.OPCODES[insn.opcode]} ${insn.var}" + default: + return Printer.OPCODES[insn.opcode] + } + } + + private static String formatConstant(Object constant) { + if (constant instanceof Long) return "${constant}L" + if (constant instanceof Float) return "${constant}f" + if (constant instanceof Double) return "${constant}d" + constant.toString() + } +} diff --git a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompilationTest.groovy b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompilationTest.groovy index 6b3e1ecae1e..0b256584246 100644 --- a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompilationTest.groovy +++ b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompilationTest.groovy @@ -541,9 +541,7 @@ final class StaticCompilationTest extends AbstractBytecodeTestCase { 'ILOAD 1', 'BIPUSH 13', 'IADD', -/*TODO*/ 'DUP', 'ISTORE 1', -/*TODO*/ 'POP', 'L2', 'LINENUMBER 5', 'ILOAD 1', @@ -563,9 +561,7 @@ final class StaticCompilationTest extends AbstractBytecodeTestCase { 'ILOAD 1', 'ILOAD 2', 'IADD', -/*TODO*/ 'DUP', 'ISTORE 1', -/*TODO*/ 'POP', 'L1', 'LINENUMBER 4', 'RETURN' diff --git a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileComparisonTest.groovy b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileComparisonTest.groovy index 324977392fc..eac814b2ca4 100644 --- a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileComparisonTest.groovy +++ b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileComparisonTest.groovy @@ -71,7 +71,7 @@ final class StaticCompileComparisonTest extends AbstractBytecodeTestCase { } ''') assert bytecode.hasStrictSequence( - ['ALOAD','ARRAYLENGTH', 'ICONST_0','IF_ICMPLE'] + ['ALOAD','ARRAYLENGTH', 'IFLE'] ) } @@ -88,7 +88,7 @@ final class StaticCompileComparisonTest extends AbstractBytecodeTestCase { } ''') assert bytecode.hasStrictSequence( - ['ALOAD','ARRAYLENGTH', 'ICONST_0','IF_ICMPLE'] + ['ALOAD','ARRAYLENGTH', 'IFLE'] ) } diff --git a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileNullCompareOptimizationTest.groovy b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileNullCompareOptimizationTest.groovy index e87c3d04c2b..eb73678bd90 100644 --- a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileNullCompareOptimizationTest.groovy +++ b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/StaticCompileNullCompareOptimizationTest.groovy @@ -78,7 +78,8 @@ final class StaticCompileNullCompareOptimizationTest extends AbstractBytecodeTes null == x } ''') - assert bytecode.hasStrictSequence(['ICONST_0', 'POP']) + assert !bytecode.hasSequence(['ICONST_0', 'POP']) + assert bytecode.hasSequence(['RETURN']) } @Test @@ -89,7 +90,8 @@ final class StaticCompileNullCompareOptimizationTest extends AbstractBytecodeTes x == null } ''') - assert bytecode.hasStrictSequence(['ICONST_0', 'POP']) + assert !bytecode.hasSequence(['ICONST_0', 'POP']) + assert bytecode.hasSequence(['RETURN']) } @Test