This is an automated email from the ASF dual-hosted git repository. asf-gitbox-commits pushed a commit to branch GROOVY-12162 in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 0ce2212b8abfa9526d1f166896bd10ce4761b858 Author: Daniel Sun <[email protected]> AuthorDate: Tue Jul 14 01:32:24 2026 +0900 GROOVY-12162: Harden and extend bytecode peephole optimization --- .../groovy/classgen/AsmClassGenerator.java | 37 ++-- .../groovy/classgen/asm/CallSiteWriter.java | 6 +- .../asm/PeepholeOptimizingClassVisitor.java | 44 +++++ .../asm/PeepholeOptimizingMethodVisitor.java | 209 ++++++++++++++++----- .../groovy/classgen/asm/WriterController.java | 10 +- .../asm/PeepholeOptimizingMethodVisitorTest.groovy | 99 +++++++++- 6 files changed, 327 insertions(+), 78 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java index 6f92fece54..5d57f66831 100644 --- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java +++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java @@ -118,7 +118,6 @@ import org.objectweb.asm.RecordComponentVisitor; import org.objectweb.asm.Type; import org.objectweb.asm.TypePath; import org.objectweb.asm.TypeReference; -import org.objectweb.asm.util.TraceMethodVisitor; import java.io.PrintWriter; import java.io.Writer; @@ -624,13 +623,12 @@ public class AsmClassGenerator extends ClassGenerator { parameters = Arrays.copyOfRange(parameters, 1, parameters.length); } - 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()))); + MethodVisitor mv = 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(); @@ -683,11 +681,13 @@ public class AsmClassGenerator extends ClassGenerator { mv.visitMaxs(0, 0); } catch (Throwable t) { Writer writer = null; - if (mv instanceof TraceMethodVisitor tmv) { - writer = new StringBuilderWriter(); - PrintWriter p = new PrintWriter(writer); - tmv.p.print(p); - p.flush(); + // Method visitors are wrapped by PeepholeOptimizingClassVisitor; unwrap to + // reach a TraceMethodVisitor when classgen logging is enabled. + StringBuilderWriter buffer = new StringBuilderWriter(); + PrintWriter printer = new PrintWriter(buffer); + if (PeepholeOptimizingMethodVisitor.printTraceBytecode(mv, printer)) { + printer.flush(); + writer = buffer; } StringBuilder message = new StringBuilder(64); message.append("ASM reporting processing error for "); @@ -2336,12 +2336,11 @@ public class AsmClassGenerator extends ClassGenerator { while (index<size) { String methodName = "$createListEntry_" + controller.getNextHelperMethodIndex(); methods.add(methodName); - mv = new PeepholeOptimizingMethodVisitor( - controller.getClassVisitor().visitMethod( - ACC_PRIVATE + ACC_STATIC + ACC_SYNTHETIC, - methodName, - "([Ljava/lang/Object;)V", - null, null)); + mv = controller.getClassVisitor().visitMethod( + ACC_PRIVATE + ACC_STATIC + ACC_SYNTHETIC, + methodName, + "([Ljava/lang/Object;)V", + null, null); controller.setMethodVisitor(mv); mv.visitCode(); int methodBlockSize = Math.min(size-index, maxInit); diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/CallSiteWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/CallSiteWriter.java index 8196cb0030..9f1b52fcf3 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/CallSiteWriter.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/CallSiteWriter.java @@ -188,8 +188,7 @@ public class CallSiteWriter { methodIndex++; String methodName = "$createCallSiteArray_" + methodIndex; callSiteInitMethods.add(methodName); - MethodVisitor mv = new PeepholeOptimizingMethodVisitor( - controller.getClassVisitor().visitMethod(MOD_PRIVSS, methodName, "([Ljava/lang/String;)V", null, null)); + MethodVisitor mv = controller.getClassVisitor().visitMethod(MOD_PRIVSS, methodName, "([Ljava/lang/String;)V", null, null); controller.setMethodVisitor(mv); mv.visitCode(); int methodLimit = size; @@ -208,8 +207,7 @@ public class CallSiteWriter { mv.visitEnd(); } // create base createCallSiteArray method - MethodVisitor mv = new PeepholeOptimizingMethodVisitor( - controller.getClassVisitor().visitMethod(MOD_PRIVSS, CREATE_CSA_METHOD, GET_CALLSITEARRAY_DESC, null, null)); + MethodVisitor mv = controller.getClassVisitor().visitMethod(MOD_PRIVSS, CREATE_CSA_METHOD, GET_CALLSITEARRAY_DESC, null, null); controller.setMethodVisitor(mv); mv.visitCode(); mv.visitLdcInsn(size); diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/PeepholeOptimizingClassVisitor.java b/src/main/java/org/codehaus/groovy/classgen/asm/PeepholeOptimizingClassVisitor.java new file mode 100644 index 0000000000..9cd55f1e19 --- /dev/null +++ b/src/main/java/org/codehaus/groovy/classgen/asm/PeepholeOptimizingClassVisitor.java @@ -0,0 +1,44 @@ +/* + * 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.ClassVisitor; +import org.objectweb.asm.MethodVisitor; + +/** + * {@link ClassVisitor} that routes every method body through + * {@link PeepholeOptimizingMethodVisitor}, so all synthetic helpers (MOP bridges, + * call-site initialisers, {@code class$} resolvers, list-init chunks, \ldots) receive + * the same single-pass compaction as user methods without ad-hoc wrapping at each + * {@code visitMethod} site. + * + * @since 6.0.0 + */ +public final class PeepholeOptimizingClassVisitor extends ClassVisitor { + + public PeepholeOptimizingClassVisitor(final ClassVisitor classVisitor) { + super(CompilerConfiguration.ASM_API_VERSION, classVisitor); + } + + @Override + public MethodVisitor visitMethod(final int access, final String name, final String descriptor, final String signature, final String[] exceptions) { + return PeepholeOptimizingMethodVisitor.wrap(super.visitMethod(access, name, descriptor, signature, exceptions)); + } +} diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/PeepholeOptimizingMethodVisitor.java b/src/main/java/org/codehaus/groovy/classgen/asm/PeepholeOptimizingMethodVisitor.java index 6acdc28738..5620592336 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/PeepholeOptimizingMethodVisitor.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/PeepholeOptimizingMethodVisitor.java @@ -25,8 +25,11 @@ import org.objectweb.asm.ConstantDynamic; import org.objectweb.asm.Handle; import org.objectweb.asm.Label; import org.objectweb.asm.MethodVisitor; +import org.objectweb.asm.Type; import org.objectweb.asm.TypePath; +import org.objectweb.asm.util.TraceMethodVisitor; +import java.io.PrintWriter; import java.math.BigDecimal; import java.math.BigInteger; @@ -59,6 +62,10 @@ 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.IFNONNULL; +import static org.objectweb.asm.Opcodes.IFNULL; +import static org.objectweb.asm.Opcodes.IF_ACMPEQ; +import static org.objectweb.asm.Opcodes.IF_ACMPNE; import static org.objectweb.asm.Opcodes.IF_ICMPEQ; import static org.objectweb.asm.Opcodes.IF_ICMPGE; import static org.objectweb.asm.Opcodes.IF_ICMPGT; @@ -81,8 +88,26 @@ 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. + * <p> + * The visitor buffers only the current stack-local candidate and flushes before + * labels, frames, debug metadata, and other non-local control-flow boundaries. + * Recognised patterns include: + * <ul> + * <li>primitive / {@code null} constant forms ({@code ICONST_*}, {@code BIPUSH}, + * {@code SIPUSH}, {@code LCONST_*}, {@code FCONST_*}, {@code DCONST_*}, + * {@code ACONST_NULL})</li> + * <li>dead loads / constants removed before matching {@code POP}/{@code POP2} + * or void {@code RETURN} (preserving a buffered {@code IINC} side effect)</li> + * <li>{@code CHECKCAST} attached to a pending reference load so + * {@code ALOAD}/{@code LDC}; {@code CHECKCAST}; {@code POP} collapses fully</li> + * <li>standalone {@code CHECKCAST}; {@code POP} drops the cast</li> + * <li>{@code ICONST_0}; {@code IF_ICMP*} rewritten to {@code IF*}</li> + * <li>{@code ACONST_NULL}; {@code IF_ACMP*} rewritten to {@code IFNULL}/{@code IFNONNULL}</li> + * <li>{@code DUP}/{@code DUP2}; store; matching {@code POP}/{@code POP2} rewritten + * to a plain store, and bare {@code DUP}/{@code DUP2}; matching pop eliminated</li> + * <li>{@link BigDecimal}/{@link BigInteger} constants lowered to + * {@code new Type(String)} construction before emission</li> + * </ul> * * @since 6.0.0 */ @@ -117,6 +142,12 @@ public final class PeepholeOptimizingMethodVisitor extends MethodVisitor { private int pendingLoadVar = NO_OPCODE; private boolean pendingLoadHasIinc; private int pendingLoadIncrement; + /** + * Descriptor for a pending {@code CHECKCAST}. When {@link #pendingLoadKind} is + * {@link PendingLoadKind#VARIABLE} or {@link PendingLoadKind#CONSTANT}, this is an + * optional cast attached to that load; when the kind is {@link PendingLoadKind#CHECKCAST}, + * it is a standalone cast of a value already on the operand stack. + */ private String pendingCheckcastDescriptor; private int pendingDupOpcode = NO_OPCODE; @@ -131,6 +162,34 @@ public final class PeepholeOptimizingMethodVisitor extends MethodVisitor { super(CompilerConfiguration.ASM_API_VERSION, delegate); } + /** + * Returns {@code delegate} when it is already a peephole visitor; otherwise wraps it. + */ + public static MethodVisitor wrap(final MethodVisitor delegate) { + if (delegate instanceof PeepholeOptimizingMethodVisitor) { + return delegate; + } + return new PeepholeOptimizingMethodVisitor(delegate); + } + + /** + * If {@code visitor} (or its chain) wraps a {@link TraceMethodVisitor}, prints that + * visitor's buffered bytecode to {@code out}. Used for class-generation diagnostics. + * + * @return {@code true} when a trace visitor was found and printed + */ + public static boolean printTraceBytecode(final MethodVisitor visitor, final PrintWriter out) { + MethodVisitor current = visitor; + while (current instanceof PeepholeOptimizingMethodVisitor peephole) { + current = peephole.mv; + } + if (current instanceof TraceMethodVisitor tmv) { + tmv.p.print(out); + return true; + } + return false; + } + @Override public void visitAttribute(final Attribute attribute) { flushPending(); @@ -145,7 +204,7 @@ public final class PeepholeOptimizingMethodVisitor extends MethodVisitor { @Override public void visitInsn(final int opcode) { - if (tryRemovePendingLoad(opcode) || tryDropPendingLoadOnReturn(opcode) || tryRemovePendingDupStore(opcode)) { + if (tryRemovePendingLoad(opcode) || tryDropPendingLoadOnReturn(opcode) || tryCollapsePendingDup(opcode)) { return; } @@ -237,12 +296,18 @@ public final class PeepholeOptimizingMethodVisitor extends MethodVisitor { @Override public void visitTypeInsn(final int opcode, final String descriptor) { - flushPendingLoad(); flushPendingDupStore(); if (opcode == CHECKCAST) { + if (canAttachCheckcastToPendingLoad()) { + pendingCheckcastDescriptor = descriptor; + return; + } + flushPendingLoad(); bufferCheckcast(descriptor); return; } + + flushPendingLoad(); super.visitTypeInsn(opcode, descriptor); } @@ -273,7 +338,7 @@ public final class PeepholeOptimizingMethodVisitor extends MethodVisitor { @Override public void visitJumpInsn(final int opcode, final Label label) { flushPendingDupStore(); - if (tryRewriteZeroCompare(opcode, label)) { + if (tryRewriteZeroCompare(opcode, label) || tryRewriteNullCompare(opcode, label)) { return; } @@ -303,7 +368,8 @@ public final class PeepholeOptimizingMethodVisitor extends MethodVisitor { if (pendingLoadKind == PendingLoadKind.VARIABLE && pendingLoadOpcode == ILOAD && pendingLoadVar == varIndex - && !pendingLoadHasIinc) { + && !pendingLoadHasIinc + && pendingCheckcastDescriptor == null) { pendingLoadHasIinc = true; pendingLoadIncrement = increment; return; @@ -388,12 +454,14 @@ public final class PeepholeOptimizingMethodVisitor extends MethodVisitor { switch (pendingLoadKind) { case CONSTANT: emitConstant(pendingConstant); + emitAttachedCheckcast(); break; case VARIABLE: super.visitVarInsn(pendingLoadOpcode, pendingLoadVar); if (pendingLoadHasIinc) { super.visitIincInsn(pendingLoadVar, pendingLoadIncrement); } + emitAttachedCheckcast(); break; case CHECKCAST: super.visitTypeInsn(CHECKCAST, pendingCheckcastDescriptor); @@ -404,6 +472,12 @@ public final class PeepholeOptimizingMethodVisitor extends MethodVisitor { clearPendingLoad(); } + private void emitAttachedCheckcast() { + if (pendingCheckcastDescriptor != null) { + super.visitTypeInsn(CHECKCAST, pendingCheckcastDescriptor); + } + } + private void flushPendingDupStore() { if (pendingDupOpcode == NO_OPCODE) { return; @@ -417,7 +491,10 @@ public final class PeepholeOptimizingMethodVisitor extends MethodVisitor { } private boolean tryRewriteZeroCompare(final int opcode, final Label label) { - if (!(pendingLoadKind == PendingLoadKind.CONSTANT && pendingConstant instanceof Integer intValue && intValue == 0)) { + if (!(pendingLoadKind == PendingLoadKind.CONSTANT + && pendingCheckcastDescriptor == null + && pendingConstant instanceof Integer intValue + && intValue == 0)) { return false; } @@ -439,6 +516,27 @@ public final class PeepholeOptimizingMethodVisitor extends MethodVisitor { return true; } + private boolean tryRewriteNullCompare(final int opcode, final Label label) { + if (!(pendingLoadKind == PendingLoadKind.CONSTANT + && pendingCheckcastDescriptor == null + && pendingConstant == null)) { + return false; + } + + int replacement = switch (opcode) { + case IF_ACMPEQ -> IFNULL; + case IF_ACMPNE -> IFNONNULL; + 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; @@ -450,6 +548,7 @@ public final class PeepholeOptimizingMethodVisitor extends MethodVisitor { } if (pendingLoadKind == PendingLoadKind.CHECKCAST) { + // Value already on stack: drop the dead cast, keep the pop. if (opcode != POP) { return false; } @@ -469,8 +568,15 @@ public final class PeepholeOptimizingMethodVisitor extends MethodVisitor { return true; } + /** + * Drops a dead buffered load/constant immediately before void {@code RETURN}. + * Standalone {@code CHECKCAST} is left alone so a type-check side effect is not + * silently discarded when the value was already committed to the stack. + */ private boolean tryDropPendingLoadOnReturn(final int opcode) { - if (opcode != RETURN || pendingLoadKind == PendingLoadKind.NONE) { + if (opcode != RETURN + || pendingLoadKind == PendingLoadKind.NONE + || pendingLoadKind == PendingLoadKind.CHECKCAST) { return false; } @@ -482,36 +588,69 @@ public final class PeepholeOptimizingMethodVisitor extends MethodVisitor { return true; } - private boolean tryRemovePendingDupStore(final int opcode) { - if (pendingStoreKind == PendingStoreKind.NONE) { + /** + * Collapses {@code DUP}/{@code DUP2} with an optional store and a matching pop: + * <ul> + * <li>{@code DUP}; {@code xSTORE}; {@code POP} → {@code xSTORE}</li> + * <li>{@code DUP2}; {@code PUTSTATIC J/D}; {@code POP2} → {@code PUTSTATIC}</li> + * <li>{@code DUP}/{@code DUP2}; matching pop with no store → eliminated entirely</li> + * </ul> + */ + private boolean tryCollapsePendingDup(final int opcode) { + if (pendingDupOpcode == NO_OPCODE) { return false; } int popSize = stackSizeForPop(opcode); - if (popSize == 0 || stackSizeForDup() != popSize || stackSizeForPendingStore() != popSize) { + if (popSize == 0 || stackSizeForDup() != popSize) { return false; } - emitPendingStore(); + if (pendingStoreKind != PendingStoreKind.NONE) { + if (stackSizeForPendingStore() != popSize) { + return false; + } + emitPendingStore(); + } clearPendingDupStore(); return true; } + private boolean canAttachCheckcastToPendingLoad() { + if (pendingCheckcastDescriptor != null) { + return false; + } + if (pendingLoadKind == PendingLoadKind.VARIABLE + && pendingLoadOpcode == ALOAD + && !pendingLoadHasIinc) { + return true; + } + return pendingLoadKind == PendingLoadKind.CONSTANT && isReferenceConstant(pendingConstant); + } + + private static boolean isReferenceConstant(final Object value) { + return value == null + || value instanceof String + || value instanceof Type + || value instanceof Handle + || value instanceof ConstantDynamic; + } + private void bufferConstant(final Object value) { - clearPendingLoad(); + flushPendingLoad(); pendingLoadKind = PendingLoadKind.CONSTANT; pendingConstant = value; } private void bufferVariableLoad(final int opcode, final int varIndex) { - clearPendingLoad(); + flushPendingLoad(); pendingLoadKind = PendingLoadKind.VARIABLE; pendingLoadOpcode = opcode; pendingLoadVar = varIndex; } private void bufferCheckcast(final String descriptor) { - clearPendingLoad(); + flushPendingLoad(); pendingLoadKind = PendingLoadKind.CHECKCAST; pendingCheckcastDescriptor = descriptor; } @@ -643,7 +782,8 @@ public final class PeepholeOptimizingMethodVisitor extends MethodVisitor { } if (value instanceof Integer intValue) { - emitIntConstant(intValue); + // Emit through the delegate so BytecodeHelper's forms are not re-buffered. + BytecodeHelper.pushConstant(mv, intValue); return; } @@ -673,41 +813,6 @@ public final class PeepholeOptimizingMethodVisitor extends MethodVisitor { super.visitMethodInsn(INVOKESPECIAL, type, "<init>", 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); @@ -719,6 +824,7 @@ public final class PeepholeOptimizingMethodVisitor extends MethodVisitor { } private void emitFloatConstant(final float value) { + // Compare raw bits so +0.0f and -0.0f stay distinct (GROOVY-9797). int rawBits = Float.floatToRawIntBits(value); if (rawBits == FLOAT_TO_RAW_INT_BITS_0) { super.visitInsn(FCONST_0); @@ -732,6 +838,7 @@ public final class PeepholeOptimizingMethodVisitor extends MethodVisitor { } private void emitDoubleConstant(final double value) { + // Compare raw bits so +0.0d and -0.0d stay distinct (GROOVY-9797). long rawBits = Double.doubleToRawLongBits(value); if (rawBits == DOUBLE_TO_RAW_LONG_BITS_0) { super.visitInsn(DCONST_0); diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/WriterController.java b/src/main/java/org/codehaus/groovy/classgen/asm/WriterController.java index d69cb00d61..6178523f01 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/WriterController.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/WriterController.java @@ -161,10 +161,14 @@ public class WriterController { } private static ClassVisitor createClassVisitor(final ClassVisitor cv, final CompilerConfiguration config) { - if (!config.isLogClassgen() || cv instanceof LoggableClassVisitor) { - return cv; + ClassVisitor visitor = cv; + if (config.isLogClassgen() && !(cv instanceof LoggableClassVisitor)) { + visitor = new LoggableClassVisitor(cv, config); } - return new LoggableClassVisitor(cv, config); + // Apply peephole compaction to every method, including synthetic helpers. + return visitor instanceof PeepholeOptimizingClassVisitor + ? visitor + : new PeepholeOptimizingClassVisitor(visitor); } //-------------------------------------------------------------------------- diff --git a/src/test/groovy/org/codehaus/groovy/classgen/asm/PeepholeOptimizingMethodVisitorTest.groovy b/src/test/groovy/org/codehaus/groovy/classgen/asm/PeepholeOptimizingMethodVisitorTest.groovy index 241aaff00c..d8489bd6e7 100644 --- a/src/test/groovy/org/codehaus/groovy/classgen/asm/PeepholeOptimizingMethodVisitorTest.groovy +++ b/src/test/groovy/org/codehaus/groovy/classgen/asm/PeepholeOptimizingMethodVisitorTest.groovy @@ -329,16 +329,91 @@ final class PeepholeOptimizingMethodVisitorTest { assert opcodeLines(bytecode) == ['IINC 0 1', 'RETURN'] } + @Test + void removesLoadAndAttachedCheckcastBeforePop() { + def bytecode = sequenceFor('(Ljava/lang/Object;)V') { + visitVarInsn(Opcodes.ALOAD, 0) + visitTypeInsn(Opcodes.CHECKCAST, 'java/lang/String') + visitInsn(Opcodes.POP) + visitInsn(RETURN) + } + + // ALOAD + CHECKCAST are both dead once the value is discarded. + assert opcodeLines(bytecode) == ['RETURN'] + } + @Test void removesStandaloneCheckcastBeforePop() { def bytecode = sequenceFor('(Ljava/lang/Object;)V') { visitVarInsn(Opcodes.ALOAD, 0) + visitInsn(Opcodes.NOP) // force the load to flush so CHECKCAST is standalone visitTypeInsn(Opcodes.CHECKCAST, 'java/lang/String') visitInsn(Opcodes.POP) visitInsn(RETURN) } - assert opcodeLines(bytecode) == ['ALOAD 0', 'POP', 'RETURN'] + assert opcodeLines(bytecode) == ['ALOAD 0', 'NOP', 'POP', 'RETURN'] + } + + @Test + void rewritesNullComparisonsAgainstAconstNull() { + def nullComparisons = [ + (Opcodes.IF_ACMPEQ): 'IFNULL', + (Opcodes.IF_ACMPNE): 'IFNONNULL', + ] + + nullComparisons.each { opcode, expected -> + def label = new Label() + def bytecode = sequenceFor('(Ljava/lang/Object;)V') { + visitVarInsn(Opcodes.ALOAD, 0) + visitInsn(Opcodes.ACONST_NULL) + visitJumpInsn(opcode, label) + visitInsn(RETURN) + visitLabel(label) + visitInsn(RETURN) + } + def lines = opcodeLines(bytecode) + + assert lines.size() == 4 + assert lines[0] == 'ALOAD 0' + assert lines[1].startsWith(expected) + assert lines[2..3] == ['RETURN', 'RETURN'] + assert !lines[1].startsWith(Printer.OPCODES[opcode]) + } + } + + @Test + void removesBareDupBeforeMatchingPop() { + def bytecode = sequenceFor { + visitInsn(Opcodes.ICONST_1) + visitInsn(Opcodes.DUP) + visitInsn(Opcodes.POP) + visitLdcInsn(2L) + visitInsn(Opcodes.DUP2) + visitInsn(Opcodes.POP2) + visitInsn(RETURN) + } + + assert opcodeLines(bytecode) == [ + 'ICONST_1', + 'LDC 2L', + 'RETURN', + ] + } + + @Test + void preservesAttachedCheckcastWhenTheValueIsUsed() { + def bytecode = sequenceFor('(Ljava/lang/Object;)Ljava/lang/String;') { + visitVarInsn(Opcodes.ALOAD, 0) + visitTypeInsn(Opcodes.CHECKCAST, 'java/lang/String') + visitInsn(Opcodes.ARETURN) + } + + assert opcodeLines(bytecode) == [ + 'ALOAD 0', + 'CHECKCAST java/lang/String', + 'ARETURN', + ] } @Test @@ -366,6 +441,28 @@ final class PeepholeOptimizingMethodVisitorTest { ] } + @Test + void preservesStandaloneCheckcastBeforeReturn() { + def bytecode = sequenceFor('(Ljava/lang/Object;)V') { + visitVarInsn(Opcodes.ALOAD, 0) + visitInsn(Opcodes.NOP) + visitTypeInsn(Opcodes.CHECKCAST, 'java/lang/String') + visitInsn(Opcodes.POP) + visitInsn(RETURN) + } + + // Standalone CHECKCAST before POP is dropped; before RETURN it is flushed. + def returnOnly = sequenceFor('(Ljava/lang/Object;)V') { + visitVarInsn(Opcodes.ALOAD, 0) + visitInsn(Opcodes.NOP) + visitTypeInsn(Opcodes.CHECKCAST, 'java/lang/String') + visitInsn(RETURN) + } + + assert opcodeLines(bytecode) == ['ALOAD 0', 'NOP', 'POP', 'RETURN'] + assert opcodeLines(returnOnly) == ['ALOAD 0', 'NOP', 'CHECKCAST java/lang/String', 'RETURN'] + } + private InstructionSequence sequenceFor(String descriptor = '()V', @DelegatesTo(MethodVisitor) Closure emitter) { traceSequence(methodNodeFor(descriptor, emitter)) }
