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 58dcf7a33f674af711d1dada3ed22ba8cacf7318 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 | 74 +++++ .../asm/PeepholeOptimizingMethodVisitor.java | 354 ++++++++++++++++++--- .../groovy/classgen/asm/WriterController.java | 10 +- .../asm/PeepholeOptimizingMethodVisitorTest.groovy | 99 +++++- 6 files changed, 501 insertions(+), 79 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..e3bacd60e1 --- /dev/null +++ b/src/main/java/org/codehaus/groovy/classgen/asm/PeepholeOptimizingClassVisitor.java @@ -0,0 +1,74 @@ +/* + * 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; + +/** + * Class-level installation point for Groovy's single-pass bytecode peephole layer. + * <p> + * Wraps each {@link MethodVisitor} returned by the delegate's + * {@link #visitMethod(int, String, String, String, String[])} with + * {@link PeepholeOptimizingMethodVisitor}, so <em>every</em> method body receives + * the same stack-local compaction — user methods, constructors, static initializers, + * and synthetic helpers alike (MOP bridges, call-site array initializers, + * {@code class$}/{@code $get$} resolvers, large-list init chunks, and so on). + * <p> + * Centralizing the wrap here avoids ad-hoc + * {@code new PeepholeOptimizingMethodVisitor(...)} at individual emission sites and + * keeps compaction in lockstep with {@link OperandStack}, which emits integer and + * other primitive constants via {@code visitLdcInsn} and relies on the peephole + * visitor to narrow them to {@code ICONST_*}, {@code BIPUSH}, {@code SIPUSH}, etc. + * <p> + * Installed once by {@link WriterController} when the class visitor chain is built + * (optionally outside a {@code LoggableClassVisitor} / {@code TraceClassVisitor} + * pair used for classgen logging). Field and attribute visits are left unchanged; + * only method bodies are optimized. + * + * @see PeepholeOptimizingMethodVisitor + * @see WriterController + * @since 6.0.0 + */ +public final class PeepholeOptimizingClassVisitor extends ClassVisitor { + + /** + * Creates a visitor that peephole-optimizes every method written to + * {@code classVisitor}. + * + * @param classVisitor the next visitor in the class-generation chain + * (typically a {@link org.objectweb.asm.ClassWriter}, optionally + * preceded by logging / tracing adapters) + */ + public PeepholeOptimizingClassVisitor(final ClassVisitor classVisitor) { + super(CompilerConfiguration.ASM_API_VERSION, classVisitor); + } + + /** + * {@inheritDoc} + * <p> + * The returned visitor is always a {@link PeepholeOptimizingMethodVisitor} + * (or an existing one if the delegate already wrapped the method). + */ + @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..da2ee28a10 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; @@ -80,10 +87,64 @@ 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. + * Single-pass, stack-local bytecode compaction for methods emitted by the Groovy + * class generator. Inspired by Groovy++'s peephole adapters. + * <p> + * Upstream writers such as {@link OperandStack} and {@link BytecodeHelper} may emit + * a uniform, easy-to-generate form (for example {@code visitLdcInsn} for every + * integer). This visitor rewrites those sequences, within a single basic-block + * window, into the densest equivalent JVM opcodes without a second compilation + * pass or a full data-flow analysis. * + * <h2>Model</h2> + * At most one <em>pending load</em> (constant, variable load, or standalone + * {@code CHECKCAST}) and at most one <em>pending {@code DUP}/{@code DUP2}</em> + * (optionally followed by a buffered store) are held at a time. Pending state is + * flushed to the delegate before any non-local boundary so control flow, frames, + * and debug metadata stay correct: + * <ul> + * <li>labels, jump targets, table/lookup switches</li> + * <li>stack map frames</li> + * <li>line numbers, local-variable tables, and type annotations</li> + * <li>method and {@code invokedynamic} calls</li> + * <li>{@code visitMaxs} / {@code visitEnd}</li> + * </ul> + * + * <h2>Rewrites</h2> + * <ul> + * <li><b>Constant narrowing</b> — {@code LDC}/push forms become + * {@code ICONST_*}, {@code BIPUSH}, {@code SIPUSH}, {@code LCONST_*}, + * {@code FCONST_*}, {@code DCONST_*}, or {@code ACONST_NULL} when legal. + * Signed floating-point zeros ({@code -0.0f}/{@code -0.0d}) are preserved + * via raw-bit comparison (GROOVY-9797).</li> + * <li><b>Dead loads</b> — a buffered load or constant followed by a matching + * {@code POP}/{@code POP2}, or a void {@code RETURN}, is dropped. A + * buffered {@code IINC} paired with {@code ILOAD} is retained as a side + * effect when the loaded value itself is discarded.</li> + * <li><b>CHECKCAST</b> — may attach to a pending {@code ALOAD} or reference + * constant so {@code load}; {@code CHECKCAST}; {@code POP} collapses + * entirely. A standalone cast before {@code POP} is dropped; before void + * {@code RETURN} it is flushed (type-check side effect kept).</li> + * <li><b>Compare-to-zero / null</b> — + * {@code ICONST_0}; {@code IF_ICMP*} → {@code IF*}; + * {@code ACONST_NULL}; {@code IF_ACMP*} → {@code IFNULL}/{@code IFNONNULL}.</li> + * <li><b>DUP + store + pop</b> — {@code DUP}/{@code DUP2}; store; + * matching pop becomes a plain store; bare {@code DUP}/{@code DUP2} with a + * matching pop is eliminated.</li> + * <li><b>Big number lowering</b> — buffered {@link BigDecimal}/{@link BigInteger} + * constants become {@code new Type(String)} construction on flush.</li> + * </ul> + * + * <h2>Installation</h2> + * Prefer {@link PeepholeOptimizingClassVisitor} (wired from + * {@link WriterController}) so every method is covered. Use {@link #wrap(MethodVisitor)} + * only when constructing a method visitor outside that chain (for example unit tests). + * Integer constants are re-emitted through {@link BytecodeHelper#pushConstant} on the + * <em>delegate</em> so they are not re-buffered by this visitor. + * + * @see PeepholeOptimizingClassVisitor + * @see OperandStack + * @see BytecodeHelper#pushConstant(MethodVisitor, int) * @since 6.0.0 */ public final class PeepholeOptimizingMethodVisitor extends MethodVisitor { @@ -98,6 +159,7 @@ public final class PeepholeOptimizingMethodVisitor extends MethodVisitor { private static final long DOUBLE_TO_RAW_LONG_BITS_0 = Double.doubleToRawLongBits(0d); private static final long DOUBLE_TO_RAW_LONG_BITS_1 = Double.doubleToRawLongBits(1d); + /** Kind of value currently held in the single-slot load window, if any. */ private enum PendingLoadKind { NONE, CONSTANT, @@ -105,6 +167,7 @@ public final class PeepholeOptimizingMethodVisitor extends MethodVisitor { CHECKCAST } + /** Kind of store buffered after a pending {@code DUP}/{@code DUP2}, if any. */ private enum PendingStoreKind { NONE, VARIABLE, @@ -117,6 +180,16 @@ public final class PeepholeOptimizingMethodVisitor extends MethodVisitor { private int pendingLoadVar = NO_OPCODE; private boolean pendingLoadHasIinc; private int pendingLoadIncrement; + /** + * Internal name / type descriptor for a pending {@code CHECKCAST}. + * <ul> + * <li>When {@link #pendingLoadKind} is {@link PendingLoadKind#VARIABLE} or + * {@link PendingLoadKind#CONSTANT}: optional cast attached to that load + * (emitted after the load on flush, or dropped with the load on pop).</li> + * <li>When the kind is {@link PendingLoadKind#CHECKCAST}: standalone cast of + * a value already written to the operand stack.</li> + * </ul> + */ private String pendingCheckcastDescriptor; private int pendingDupOpcode = NO_OPCODE; @@ -127,10 +200,63 @@ public final class PeepholeOptimizingMethodVisitor extends MethodVisitor { private String pendingStoreName; private String pendingStoreDescriptor; + /** + * Creates a peephole visitor that forwards compacted instructions to + * {@code delegate}. + * + * @param delegate the next method visitor in the chain (for example a + * {@link org.objectweb.asm.MethodWriter} or {@link TraceMethodVisitor}) + */ public PeepholeOptimizingMethodVisitor(final MethodVisitor delegate) { super(CompilerConfiguration.ASM_API_VERSION, delegate); } + /** + * Idempotent factory: returns {@code delegate} unchanged when it is already a + * {@link PeepholeOptimizingMethodVisitor}, otherwise wraps it. + * <p> + * Used by {@link PeepholeOptimizingClassVisitor#visitMethod} so nested or + * repeated wrapping does not stack multiple peephole layers. + * + * @param delegate the visitor to wrap; must not be {@code null} when a new + * wrapper is created + * @return a peephole-optimizing method visitor + */ + public static MethodVisitor wrap(final MethodVisitor delegate) { + if (delegate instanceof PeepholeOptimizingMethodVisitor) { + return delegate; + } + return new PeepholeOptimizingMethodVisitor(delegate); + } + + /** + * Walks {@code visitor} and any nested {@link PeepholeOptimizingMethodVisitor} + * layers to find a {@link TraceMethodVisitor}, then prints that visitor's + * recorded instruction text to {@code out}. + * <p> + * {@link org.codehaus.groovy.classgen.AsmClassGenerator} uses this when + * {@code visitMaxs} fails under classgen logging: the outer visitor is a + * peephole wrapper, so a direct {@code instanceof TraceMethodVisitor} check + * would miss the tracer sitting further down the chain. + * + * @param visitor the method visitor active during class generation (may be + * a peephole wrapper) + * @param out destination for the traced bytecode listing + * @return {@code true} if a {@link TraceMethodVisitor} was found and printed; + * {@code false} if none was present in the chain + */ + 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 +271,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 +363,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 +405,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 +435,8 @@ public final class PeepholeOptimizingMethodVisitor extends MethodVisitor { if (pendingLoadKind == PendingLoadKind.VARIABLE && pendingLoadOpcode == ILOAD && pendingLoadVar == varIndex - && !pendingLoadHasIinc) { + && !pendingLoadHasIinc + && pendingCheckcastDescriptor == null) { pendingLoadHasIinc = true; pendingLoadIncrement = increment; return; @@ -379,21 +512,28 @@ public final class PeepholeOptimizingMethodVisitor extends MethodVisitor { super.visitEnd(); } + /** Flushes both the load window and any pending {@code DUP}/store pair. */ private void flushPending() { flushPendingLoad(); flushPendingDupStore(); } + /** + * Emits the buffered load/constant (and any attached {@code CHECKCAST} or + * {@code IINC}) to the delegate, then clears the load window. + */ private void flushPendingLoad() { 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 +544,17 @@ public final class PeepholeOptimizingMethodVisitor extends MethodVisitor { clearPendingLoad(); } + /** Emits a {@code CHECKCAST} that was attached to a pending variable/constant load. */ + private void emitAttachedCheckcast() { + if (pendingCheckcastDescriptor != null) { + super.visitTypeInsn(CHECKCAST, pendingCheckcastDescriptor); + } + } + + /** + * Emits a buffered {@code DUP}/{@code DUP2} and, when present, the following + * store that was waiting to see whether the duplicated value would be discarded. + */ private void flushPendingDupStore() { if (pendingDupOpcode == NO_OPCODE) { return; @@ -416,8 +567,17 @@ public final class PeepholeOptimizingMethodVisitor extends MethodVisitor { clearPendingDupStore(); } + /** + * Rewrites {@code …; ICONST_0; IF_ICMPxx L} to {@code …; IFxx L} when the + * pending constant is integer zero. + * + * @return {@code true} if the jump was rewritten and no further action is needed + */ 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 +599,42 @@ public final class PeepholeOptimizingMethodVisitor extends MethodVisitor { return true; } + /** + * Rewrites {@code …; ACONST_NULL; IF_ACMPEQ/NE L} to {@code …; IFNULL/IFNONNULL L}. + * + * @return {@code true} if the jump was rewritten and no further action is needed + */ + 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; + } + + /** + * Drops a dead buffered load/constant (or standalone {@code CHECKCAST}) when + * the next instruction is a matching {@code POP}/{@code POP2}. + * <p> + * For a standalone cast the value is already on the stack, so only the cast is + * removed and the pop is still emitted. For a buffered load, both the load and + * any attached cast are removed; a paired {@code IINC} is preserved. + * + * @return {@code true} if the pop was fully handled + */ private boolean tryRemovePendingLoad(final int opcode) { if (pendingLoadKind == PendingLoadKind.NONE) { return false; @@ -450,6 +646,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 +666,17 @@ 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. + * + * @return {@code true} if the return was emitted after dropping the pending load + */ 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 +688,94 @@ 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> + * + * @return {@code true} if the pop was fully handled + */ + 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; } + /** + * Whether the next {@code CHECKCAST} can be recorded as an adornment on the + * current pending reference load instead of flushing that load first. + * Restricted to {@code ALOAD} and reference-typed constants so primitive + * loads are never paired with an invalid cast. + */ + private boolean canAttachCheckcastToPendingLoad() { + if (pendingCheckcastDescriptor != null) { + return false; + } + if (pendingLoadKind == PendingLoadKind.VARIABLE + && pendingLoadOpcode == ALOAD + && !pendingLoadHasIinc) { + return true; + } + return pendingLoadKind == PendingLoadKind.CONSTANT && isReferenceConstant(pendingConstant); + } + + /** + * {@code true} for constant values that occupy a single reference slot on the + * operand stack ({@code null}, {@link String}, {@link Type}, {@link Handle}, + * {@link ConstantDynamic}). + */ + private static boolean isReferenceConstant(final Object value) { + return value == null + || value instanceof String + || value instanceof Type + || value instanceof Handle + || value instanceof ConstantDynamic; + } + + /** + * Replaces the load window with a constant candidate. Any previously buffered + * load is flushed first so candidates are never silently discarded. + */ private void bufferConstant(final Object value) { - clearPendingLoad(); + flushPendingLoad(); pendingLoadKind = PendingLoadKind.CONSTANT; pendingConstant = value; } + /** + * Replaces the load window with a variable-load candidate, flushing any prior + * pending load first. + */ private void bufferVariableLoad(final int opcode, final int varIndex) { - clearPendingLoad(); + flushPendingLoad(); pendingLoadKind = PendingLoadKind.VARIABLE; pendingLoadOpcode = opcode; pendingLoadVar = varIndex; } + /** + * Buffers a standalone {@code CHECKCAST} of a value already on the stack + * (the preceding producer was not held in the load window). + */ private void bufferCheckcast(final String descriptor) { - clearPendingLoad(); + flushPendingLoad(); pendingLoadKind = PendingLoadKind.CHECKCAST; pendingCheckcastDescriptor = descriptor; } @@ -631,6 +895,11 @@ public final class PeepholeOptimizingMethodVisitor extends MethodVisitor { }; } + /** + * Writes {@code value} to the delegate using the densest legal constant form. + * Integers go through {@link BytecodeHelper#pushConstant} on the ASM delegate + * so the specialized opcodes are not intercepted and re-buffered by this visitor. + */ private void emitConstant(final Object value) { if (value == null) { super.visitInsn(ACONST_NULL); @@ -643,7 +912,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; } @@ -665,6 +935,10 @@ public final class PeepholeOptimizingMethodVisitor extends MethodVisitor { super.visitLdcInsn(value); } + /** + * Lowers a {@link BigDecimal} or {@link BigInteger} constant to + * {@code new Type(value.toString())} on the delegate. + */ private void emitStringConstructedConstant(final Object value) { String type = (value instanceof BigDecimal ? BIG_DECIMAL_TYPE : BIG_INTEGER_TYPE); super.visitTypeInsn(NEW, type); @@ -673,41 +947,7 @@ 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); - } - } - + /** Emits {@code LCONST_0}/{@code LCONST_1} when possible, otherwise {@code LDC}. */ private void emitLongConstant(final long value) { if (value == 0L) { super.visitInsn(LCONST_0); @@ -718,7 +958,12 @@ public final class PeepholeOptimizingMethodVisitor extends MethodVisitor { } } + /** + * Emits {@code FCONST_0/1/2} when the raw bits match; otherwise {@code LDC}. + * Raw-bit comparison keeps {@code +0.0f} and {@code -0.0f} distinct (GROOVY-9797). + */ 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); @@ -731,7 +976,12 @@ public final class PeepholeOptimizingMethodVisitor extends MethodVisitor { } } + /** + * Emits {@code DCONST_0/1} when the raw bits match; otherwise {@code LDC}. + * Raw-bit comparison keeps {@code +0.0d} and {@code -0.0d} distinct (GROOVY-9797). + */ 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)) }
