From 2a55a905fbd135cfa4b0415fe659853691457cb2 Mon Sep 17 00:00:00 2001 From: Daniel Sun Date: Sun, 7 Jun 2026 01:06:59 +0900 Subject: [PATCH] GROOVY-12065: Implement peephole optimization for bytecode generation --- .../groovy/classgen/AsmClassGenerator.java | 32 +- .../groovy/classgen/asm/BytecodeHelper.java | 4 + .../groovy/classgen/asm/MopWriter.java | 4 +- .../groovy/classgen/asm/OperandStack.java | 57 +- .../asm/PeepholeOptimizingMethodVisitor.java | 737 ++++++++++++++++++ .../classgen/asm/BinaryOperationsTest.groovy | 27 +- ...PeepholeOptimizingMethodVisitorTest.groovy | 240 ++++++ .../asm/sc/StaticCompilationTest.groovy | 4 - .../asm/sc/StaticCompileComparisonTest.groovy | 4 +- ...cCompileNullCompareOptimizationTest.groovy | 6 +- 10 files changed, 1041 insertions(+), 74 deletions(-) create mode 100644 src/main/java/org/codehaus/groovy/classgen/asm/PeepholeOptimizingMethodVisitor.java create mode 100644 src/test/groovy/org/codehaus/groovy/classgen/asm/PeepholeOptimizingMethodVisitorTest.groovy 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 methods, final bool Parameter[] parameters = method.getParameters(); String mopName = getMopMethodName(method, useThis); String signature = BytecodeHelper.getMethodDescriptor(returnType, parameters); - MethodVisitor mv = controller.getClassVisitor().visitMethod(ACC_PUBLIC | ACC_SYNTHETIC, mopName, signature, null, null); + MethodVisitor mv = new PeepholeOptimizingMethodVisitor( + controller.getClassVisitor().visitMethod( + ACC_PUBLIC | ACC_SYNTHETIC, mopName, signature, null, null)); controller.setMethodVisitor(mv); int stackIndex = 0; diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/OperandStack.java b/src/main/java/org/codehaus/groovy/classgen/asm/OperandStack.java index acd6f5d99f0..4f46a57a1e2 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/OperandStack.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/OperandStack.java @@ -49,13 +49,10 @@ import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveVoid; import static org.objectweb.asm.Opcodes.ACONST_NULL; import static org.objectweb.asm.Opcodes.ALOAD; -import static org.objectweb.asm.Opcodes.BIPUSH; import static org.objectweb.asm.Opcodes.CHECKCAST; import static org.objectweb.asm.Opcodes.D2F; import static org.objectweb.asm.Opcodes.D2I; import static org.objectweb.asm.Opcodes.D2L; -import static org.objectweb.asm.Opcodes.DCONST_0; -import static org.objectweb.asm.Opcodes.DCONST_1; import static org.objectweb.asm.Opcodes.DUP; import static org.objectweb.asm.Opcodes.DUP2; import static org.objectweb.asm.Opcodes.DUP2_X1; @@ -64,9 +61,6 @@ import static org.objectweb.asm.Opcodes.F2D; import static org.objectweb.asm.Opcodes.F2I; import static org.objectweb.asm.Opcodes.F2L; -import static org.objectweb.asm.Opcodes.FCONST_0; -import static org.objectweb.asm.Opcodes.FCONST_1; -import static org.objectweb.asm.Opcodes.FCONST_2; import static org.objectweb.asm.Opcodes.GETSTATIC; import static org.objectweb.asm.Opcodes.I2B; import static org.objectweb.asm.Opcodes.I2C; @@ -74,15 +68,11 @@ import static org.objectweb.asm.Opcodes.I2F; import static org.objectweb.asm.Opcodes.I2L; import static org.objectweb.asm.Opcodes.I2S; -import static org.objectweb.asm.Opcodes.ICONST_0; -import static org.objectweb.asm.Opcodes.ICONST_1; import static org.objectweb.asm.Opcodes.INVOKESPECIAL; import static org.objectweb.asm.Opcodes.INVOKEVIRTUAL; import static org.objectweb.asm.Opcodes.L2D; import static org.objectweb.asm.Opcodes.L2F; import static org.objectweb.asm.Opcodes.L2I; -import static org.objectweb.asm.Opcodes.LCONST_0; -import static org.objectweb.asm.Opcodes.LCONST_1; import static org.objectweb.asm.Opcodes.NEW; import static org.objectweb.asm.Opcodes.POP; import static org.objectweb.asm.Opcodes.POP2; @@ -165,11 +155,7 @@ public void castToBool(final int mark, final boolean emptyDefault) { MethodVisitor mv = controller.getMethodVisitor(); if (mark == size) { // no element, so use emptyDefault - if (emptyDefault) { - mv.visitIntInsn(BIPUSH, 1); - } else { - mv.visitIntInsn(BIPUSH, 0); - } + mv.visitLdcInsn(emptyDefault ? 1 : 0); stack.add(null); } else if (mark == size - 1) { ClassNode last = stack.get(size - 1); @@ -585,42 +571,11 @@ private static void pushPrimitiveConstant(final MethodVisitor mv, final Object v boolean isChar = isPrimitiveChar(type); if (isInt || isShort || isByte || isChar) { int val = isInt ? (Integer) value : isShort ? (Short) value : isChar ? (Character) value : (Byte) value; - BytecodeHelper.pushConstant(mv, val); - } else if (isPrimitiveLong(type)) { - if ((Long) value == 0L) { - mv.visitInsn(LCONST_0); - } else if ((Long) value == 1L) { - mv.visitInsn(LCONST_1); - } else { - mv.visitLdcInsn(value); - } - } else if (isPrimitiveFloat(type)) { - // GROOVY-9797: Use Float.equals to differentiate between positive and negative zero - if (value.equals(0f)) { - mv.visitInsn(FCONST_0); - } else if ((Float) value == 1f) { - mv.visitInsn(FCONST_1); - } else if ((Float) value == 2f) { - mv.visitInsn(FCONST_2); - } else { - mv.visitLdcInsn(value); - } - } else if (isPrimitiveDouble(type)) { - // GROOVY-9797: Use Double.equals to differentiate between positive and negative zero - if (value.equals(0d)) { - mv.visitInsn(DCONST_0); - } else if ((Double) value == 1d) { - mv.visitInsn(DCONST_1); - } else { - mv.visitLdcInsn(value); - } + mv.visitLdcInsn(val); + } else if (isPrimitiveLong(type) || isPrimitiveFloat(type) || isPrimitiveDouble(type)) { + mv.visitLdcInsn(value); } else if (isPrimitiveBoolean(type)) { - boolean b = (Boolean) value; - if (b) { - mv.visitInsn(ICONST_1); - } else { - mv.visitInsn(ICONST_0); - } + mv.visitLdcInsn((Boolean) value ? 1 : 0); } else { mv.visitLdcInsn(value); } @@ -729,7 +684,7 @@ public void load(final ClassNode type, final int idx) { */ public void pushBool(final boolean value) { MethodVisitor mv = controller.getMethodVisitor(); - mv.visitInsn(value ? ICONST_1 : ICONST_0); + mv.visitLdcInsn(value ? 1 : 0); push(ClassHelper.boolean_TYPE); } diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/PeepholeOptimizingMethodVisitor.java b/src/main/java/org/codehaus/groovy/classgen/asm/PeepholeOptimizingMethodVisitor.java new file mode 100644 index 00000000000..52b8eaa6f09 --- /dev/null +++ b/src/main/java/org/codehaus/groovy/classgen/asm/PeepholeOptimizingMethodVisitor.java @@ -0,0 +1,737 @@ +/* + * 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.objectweb.asm.AnnotationVisitor; +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.TypePath; + +import java.math.BigDecimal; +import java.math.BigInteger; + +import static org.objectweb.asm.Opcodes.ACONST_NULL; +import static org.objectweb.asm.Opcodes.ALOAD; +import static org.objectweb.asm.Opcodes.ASTORE; +import static org.objectweb.asm.Opcodes.BIPUSH; +import static org.objectweb.asm.Opcodes.CHECKCAST; +import static org.objectweb.asm.Opcodes.DCONST_0; +import static org.objectweb.asm.Opcodes.DCONST_1; +import static org.objectweb.asm.Opcodes.DLOAD; +import static org.objectweb.asm.Opcodes.DSTORE; +import static org.objectweb.asm.Opcodes.DUP; +import static org.objectweb.asm.Opcodes.DUP2; +import static org.objectweb.asm.Opcodes.FCONST_0; +import static org.objectweb.asm.Opcodes.FCONST_1; +import static org.objectweb.asm.Opcodes.FCONST_2; +import static org.objectweb.asm.Opcodes.FLOAD; +import static org.objectweb.asm.Opcodes.FSTORE; +import static org.objectweb.asm.Opcodes.ICONST_0; +import static org.objectweb.asm.Opcodes.ICONST_1; +import static org.objectweb.asm.Opcodes.ICONST_2; +import static org.objectweb.asm.Opcodes.ICONST_3; +import static org.objectweb.asm.Opcodes.ICONST_4; +import static org.objectweb.asm.Opcodes.ICONST_5; +import static org.objectweb.asm.Opcodes.ICONST_M1; +import static org.objectweb.asm.Opcodes.IFEQ; +import static org.objectweb.asm.Opcodes.IFGE; +import static org.objectweb.asm.Opcodes.IFGT; +import static org.objectweb.asm.Opcodes.IFLE; +import static org.objectweb.asm.Opcodes.IFLT; +import static org.objectweb.asm.Opcodes.IFNE; +import static org.objectweb.asm.Opcodes.IF_ICMPEQ; +import static org.objectweb.asm.Opcodes.IF_ICMPGE; +import static org.objectweb.asm.Opcodes.IF_ICMPGT; +import static org.objectweb.asm.Opcodes.IF_ICMPLE; +import static org.objectweb.asm.Opcodes.IF_ICMPLT; +import static org.objectweb.asm.Opcodes.IF_ICMPNE; +import static org.objectweb.asm.Opcodes.ILOAD; +import static org.objectweb.asm.Opcodes.INVOKESPECIAL; +import static org.objectweb.asm.Opcodes.ISTORE; +import static org.objectweb.asm.Opcodes.LCONST_0; +import static org.objectweb.asm.Opcodes.LCONST_1; +import static org.objectweb.asm.Opcodes.LLOAD; +import static org.objectweb.asm.Opcodes.LSTORE; +import static org.objectweb.asm.Opcodes.NEW; +import static org.objectweb.asm.Opcodes.POP; +import static org.objectweb.asm.Opcodes.POP2; +import static org.objectweb.asm.Opcodes.PUTSTATIC; +import static org.objectweb.asm.Opcodes.RETURN; +import static org.objectweb.asm.Opcodes.SIPUSH; + +/** + * Single-pass bytecode compaction inspired by Groovy++'s peephole adapters. + * The visitor only buffers the current stack-local candidate and flushes before + * labels, frames, debug metadata, and other non-local boundaries. + */ +public final class PeepholeOptimizingMethodVisitor extends MethodVisitor { + + private static final String BIG_DECIMAL_TYPE = "java/math/BigDecimal"; + private static final String BIG_INTEGER_TYPE = "java/math/BigInteger"; + private static final String STRING_CTOR_DESCRIPTOR = "(Ljava/lang/String;)V"; + private static final int NO_OPCODE = -1; + + private enum PendingLoadKind { + NONE, + CONSTANT, + VARIABLE, + CHECKCAST + } + + private enum PendingStoreKind { + NONE, + VARIABLE, + STATIC_FIELD + } + + private PendingLoadKind pendingLoadKind = PendingLoadKind.NONE; + private Object pendingConstant; + private int pendingLoadOpcode = NO_OPCODE; + private int pendingLoadVar = NO_OPCODE; + private boolean pendingLoadHasIinc; + private int pendingLoadIncrement; + private String pendingCheckcastDescriptor; + + private int pendingDupOpcode = NO_OPCODE; + private PendingStoreKind pendingStoreKind = PendingStoreKind.NONE; + private int pendingStoreOpcode = NO_OPCODE; + private int pendingStoreVar = NO_OPCODE; + private String pendingStoreOwner; + private String pendingStoreName; + private String pendingStoreDescriptor; + + public PeepholeOptimizingMethodVisitor(final MethodVisitor delegate) { + super(CompilerConfiguration.ASM_API_VERSION, delegate); + } + + @Override + public void visitAttribute(final Attribute attribute) { + flushPending(); + super.visitAttribute(attribute); + } + + @Override + public void visitFrame(final int type, final int numLocal, final Object[] local, final int numStack, final Object[] stack) { + flushPending(); + super.visitFrame(type, numLocal, local, numStack, stack); + } + + @Override + public void visitInsn(final int opcode) { + if (tryRemovePendingLoad(opcode) || tryDropPendingLoadOnReturn(opcode) || tryRemovePendingDupStore(opcode)) { + return; + } + + flushPendingLoad(); + if (opcode == DUP || opcode == DUP2) { + flushPendingDupStore(); + pendingDupOpcode = opcode; + return; + } + + flushPendingDupStore(); + switch (opcode) { + case ACONST_NULL: + bufferConstant(null); + return; + case ICONST_M1: + bufferConstant(-1); + return; + case ICONST_0: + bufferConstant(0); + return; + case ICONST_1: + bufferConstant(1); + return; + case ICONST_2: + bufferConstant(2); + return; + case ICONST_3: + bufferConstant(3); + return; + case ICONST_4: + bufferConstant(4); + return; + case ICONST_5: + bufferConstant(5); + return; + case LCONST_0: + bufferConstant(0L); + return; + case LCONST_1: + bufferConstant(1L); + return; + case FCONST_0: + bufferConstant(0f); + return; + case FCONST_1: + bufferConstant(1f); + return; + case FCONST_2: + bufferConstant(2f); + return; + case DCONST_0: + bufferConstant(0d); + return; + case DCONST_1: + bufferConstant(1d); + return; + default: + super.visitInsn(opcode); + } + } + + @Override + public void visitIntInsn(final int opcode, final int operand) { + flushPendingLoad(); + flushPendingDupStore(); + if (opcode == BIPUSH || opcode == SIPUSH) { + bufferConstant(operand); + return; + } + super.visitIntInsn(opcode, operand); + } + + @Override + public void visitVarInsn(final int opcode, final int varIndex) { + flushPendingLoad(); + if (pendingDupOpcode != NO_OPCODE && pendingStoreKind == PendingStoreKind.NONE && isStoreOpcode(opcode)) { + bufferVariableStore(opcode, varIndex); + return; + } + + flushPendingDupStore(); + if (isLoadOpcode(opcode)) { + bufferVariableLoad(opcode, varIndex); + return; + } + super.visitVarInsn(opcode, varIndex); + } + + @Override + public void visitTypeInsn(final int opcode, final String descriptor) { + flushPendingLoad(); + flushPendingDupStore(); + if (opcode == CHECKCAST) { + bufferCheckcast(descriptor); + return; + } + super.visitTypeInsn(opcode, descriptor); + } + + @Override + public void visitFieldInsn(final int opcode, final String owner, final String name, final String descriptor) { + flushPendingLoad(); + if (pendingDupOpcode != NO_OPCODE && pendingStoreKind == PendingStoreKind.NONE && opcode == PUTSTATIC) { + bufferStaticStore(opcode, owner, name, descriptor); + return; + } + + flushPendingDupStore(); + super.visitFieldInsn(opcode, owner, name, descriptor); + } + + @Override + public void visitMethodInsn(final int opcode, final String owner, final String name, final String descriptor, final boolean isInterface) { + flushPending(); + super.visitMethodInsn(opcode, owner, name, descriptor, isInterface); + } + + @Override + public void visitInvokeDynamicInsn(final String name, final String descriptor, final Handle bootstrapMethodHandle, final Object... bootstrapMethodArguments) { + flushPending(); + super.visitInvokeDynamicInsn(name, descriptor, bootstrapMethodHandle, bootstrapMethodArguments); + } + + @Override + public void visitJumpInsn(final int opcode, final Label label) { + flushPendingDupStore(); + if (tryRewriteZeroCompare(opcode, label)) { + return; + } + + flushPendingLoad(); + super.visitJumpInsn(opcode, label); + } + + @Override + public void visitLabel(final Label label) { + flushPending(); + super.visitLabel(label); + } + + @Override + public void visitLdcInsn(final Object value) { + flushPendingLoad(); + flushPendingDupStore(); + if (value instanceof ConstantDynamic) { + super.visitLdcInsn(value); + return; + } + bufferConstant(value); + } + + @Override + public void visitIincInsn(final int varIndex, final int increment) { + if (pendingLoadKind == PendingLoadKind.VARIABLE + && pendingLoadOpcode == ILOAD + && pendingLoadVar == varIndex + && !pendingLoadHasIinc) { + pendingLoadHasIinc = true; + pendingLoadIncrement = increment; + return; + } + + flushPending(); + super.visitIincInsn(varIndex, increment); + } + + @Override + public void visitTableSwitchInsn(final int min, final int max, final Label dflt, final Label... labels) { + flushPending(); + super.visitTableSwitchInsn(min, max, dflt, labels); + } + + @Override + public void visitLookupSwitchInsn(final Label dflt, final int[] keys, final Label[] labels) { + flushPending(); + super.visitLookupSwitchInsn(dflt, keys, labels); + } + + @Override + public void visitMultiANewArrayInsn(final String descriptor, final int dims) { + flushPending(); + super.visitMultiANewArrayInsn(descriptor, dims); + } + + @Override + public AnnotationVisitor visitInsnAnnotation(final int typeRef, final TypePath typePath, final String descriptor, final boolean visible) { + flushPending(); + return super.visitInsnAnnotation(typeRef, typePath, descriptor, visible); + } + + @Override + public void visitTryCatchBlock(final Label start, final Label end, final Label handler, final String type) { + flushPending(); + super.visitTryCatchBlock(start, end, handler, type); + } + + @Override + public AnnotationVisitor visitTryCatchAnnotation(final int typeRef, final TypePath typePath, final String descriptor, final boolean visible) { + flushPending(); + return super.visitTryCatchAnnotation(typeRef, typePath, descriptor, visible); + } + + @Override + public void visitLocalVariable(final String name, final String descriptor, final String signature, final Label start, final Label end, final int index) { + flushPending(); + super.visitLocalVariable(name, descriptor, signature, start, end, index); + } + + @Override + public AnnotationVisitor visitLocalVariableAnnotation(final int typeRef, final TypePath typePath, final Label[] start, final Label[] end, final int[] index, final String descriptor, final boolean visible) { + flushPending(); + return super.visitLocalVariableAnnotation(typeRef, typePath, start, end, index, descriptor, visible); + } + + @Override + public void visitLineNumber(final int line, final Label start) { + flushPending(); + super.visitLineNumber(line, start); + } + + @Override + public void visitMaxs(final int maxStack, final int maxLocals) { + flushPending(); + super.visitMaxs(maxStack, maxLocals); + } + + @Override + public void visitEnd() { + flushPending(); + super.visitEnd(); + } + + private void flushPending() { + flushPendingLoad(); + flushPendingDupStore(); + } + + private void flushPendingLoad() { + switch (pendingLoadKind) { + case CONSTANT: + emitConstant(pendingConstant); + break; + case VARIABLE: + super.visitVarInsn(pendingLoadOpcode, pendingLoadVar); + if (pendingLoadHasIinc) { + super.visitIincInsn(pendingLoadVar, pendingLoadIncrement); + } + break; + case CHECKCAST: + super.visitTypeInsn(CHECKCAST, pendingCheckcastDescriptor); + break; + case NONE: + default: + } + clearPendingLoad(); + } + + private void flushPendingDupStore() { + if (pendingDupOpcode == NO_OPCODE) { + return; + } + + super.visitInsn(pendingDupOpcode); + if (pendingStoreKind != PendingStoreKind.NONE) { + emitPendingStore(); + } + clearPendingDupStore(); + } + + private boolean tryRewriteZeroCompare(final int opcode, final Label label) { + if (!(pendingLoadKind == PendingLoadKind.CONSTANT && pendingConstant instanceof Integer intValue && intValue == 0)) { + return false; + } + + int replacement = switch (opcode) { + case IF_ICMPEQ -> 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/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/PeepholeOptimizingMethodVisitorTest.groovy b/src/test/groovy/org/codehaus/groovy/classgen/asm/PeepholeOptimizingMethodVisitorTest.groovy new file mode 100644 index 00000000000..9a38498d3cd --- /dev/null +++ b/src/test/groovy/org/codehaus/groovy/classgen/asm/PeepholeOptimizingMethodVisitorTest.groovy @@ -0,0 +1,240 @@ +/* + * 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.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.MethodInsnNode +import org.objectweb.asm.tree.MethodNode +import org.objectweb.asm.tree.TypeInsnNode +import org.objectweb.asm.tree.VarInsnNode +import org.objectweb.asm.util.Printer + +import java.math.BigDecimal +import java.math.BigInteger + +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 instructions = collectInstructions { + visitLdcInsn(-1) + visitLdcInsn(5) + visitLdcInsn(6) + visitIntInsn(org.objectweb.asm.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 instructions == [ + 'ICONST_M1', + 'ICONST_5', + 'BIPUSH 6', + 'BIPUSH 120', + 'LCONST_0', + 'LCONST_1', + 'LDC 2', + 'FCONST_0', + 'FCONST_1', + 'FCONST_2', + 'DCONST_0', + 'DCONST_1', + 'DSTORE 0', + 'RETURN', + ] + } + + @Test + void preservesSignedZeroConstants() { + def instructions = collectInsnNodes { + visitLdcInsn(-0.0f) + visitVarInsn(Opcodes.FSTORE, 0) + visitLdcInsn(-0.0d) + visitVarInsn(Opcodes.DSTORE, 1) + visitInsn(RETURN) + } + + assert instructions[0] instanceof LdcInsnNode + assert instructions[0].cst.equals(-0.0f) + assert render(instructions[1]) == 'FSTORE 0' + assert instructions[2] instanceof LdcInsnNode + assert instructions[2].cst.equals(-0.0d) + assert render(instructions[3]) == 'DSTORE 1' + assert render(instructions[4]) == '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 instructions = collectInstructions('(I)V') { + visitVarInsn(Opcodes.ILOAD, 0) + visitLdcInsn(0) + visitJumpInsn(opcode, label) + visitInsn(RETURN) + visitLabel(label) + visitInsn(RETURN) + } + + assert instructions == ['ILOAD 0', expected, 'RETURN', 'RETURN'] + } + } + + @Test + void removesRedundantLoadsAndConstantPops() { + def instructions = collectInstructions('(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 instructions == ['RETURN'] + } + + @Test + void preservesIincWhenDroppingRedundantLoads() { + def instructions = collectInstructions('(I)V') { + visitVarInsn(Opcodes.ILOAD, 0) + visitIincInsn(0, 1) + visitInsn(Opcodes.POP) + visitInsn(RETURN) + } + + assert instructions == ['IINC 0 1', 'RETURN'] + } + + @Test + void removesRedundantDupStorePops() { + def instructions = collectInstructions { + 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 instructions == [ + 'ICONST_1', + 'ISTORE 0', + 'LDC 2', + 'PUTSTATIC Owner.VALUE : J', + 'RETURN', + ] + } + + @Test + void lowersBigNumberConstantsBeforeEmission() { + def instructions = collectInstructions { + 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 instructions == [ + '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', + ] + } + + private static List collectInstructions(String descriptor = '()V', @DelegatesTo(MethodVisitor) Closure emitter) { + collectInsnNodes(descriptor, emitter).collect { AbstractInsnNode insn -> render(insn) } + } + + private static List collectInsnNodes(String descriptor = '()V', @DelegatesTo(MethodVisitor) Closure emitter) { + def sink = new MethodNode(CompilerConfiguration.ASM_API_VERSION, ACC_PUBLIC | ACC_STATIC, 'sample', descriptor, null, null) + def visitor = new PeepholeOptimizingMethodVisitor(sink) + visitor.visitCode() + emitter.delegate = visitor + emitter.resolveStrategy = Closure.DELEGATE_FIRST + emitter.call() + visitor.visitMaxs(0, 0) + visitor.visitEnd() + sink.instructions.toArray() + .findAll { AbstractInsnNode insn -> insn.opcode >= 0 } + } + + 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 ${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] + } + } +} 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 f2450f94b37..832f9502ed2 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 @@ -536,9 +536,7 @@ final class StaticCompilationTest extends AbstractBytecodeTestCase { 'ILOAD 1', 'BIPUSH 13', 'IADD', -/*TODO*/ 'DUP', 'ISTORE 1', -/*TODO*/ 'POP', 'L2', 'LINENUMBER 5', 'ILOAD 1', @@ -558,9 +556,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