[ 
https://issues.apache.org/jira/browse/GROOVY-12065?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18087792#comment-18087792
 ] 

ASF GitHub Bot commented on GROOVY-12065:
-----------------------------------------

blackdrag commented on code in PR #2591:
URL: https://github.com/apache/groovy/pull/2591#discussion_r3384712327


##########
src/main/java/org/codehaus/groovy/classgen/asm/OperandStack.java:
##########
@@ -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);

Review Comment:
   Maybe it was different when most of the bytecode code was written, but 
current version of asm actually handle the special case internally: 
https://asm.ow2.io/javadoc/org/objectweb/asm/MethodVisitor.html#visitLdcInsn(java.lang.Object)
 - AFAIK





> Implement peephole optimization for bytecode generation
> -------------------------------------------------------
>
>                 Key: GROOVY-12065
>                 URL: https://issues.apache.org/jira/browse/GROOVY-12065
>             Project: Groovy
>          Issue Type: Improvement
>            Reporter: Daniel Sun
>            Assignee: Daniel Sun
>            Priority: Major
>
> h2. Overview
> This improvement introduces a dedicated, single-pass bytecode compaction 
> layer via {{{}PeepholeOptimizingMethodVisitor{}}}. This adapter wraps the 
> underlying ASM {{MethodVisitor}} during class generation 
> ({{{}AsmClassGenerator{}}}), intercepting instruction streams within local 
> basic blocks to eliminate redundant operations, rewrite conditional branches, 
> and narrow constant instructions to their most optimal forms.
> Crucially, by decoupling peephole optimization from structural code 
> generation, this approach significantly simplifies the constant emission 
> logic inside {{{}OperandStack{}}}. Instead of maintaining verbose, duplicate 
> routing logic for specialized primitive opcodes (such as {{{}ICONST_x{}}}, 
> {{{}BIPUSH{}}}, or {{{}FCONST_x{}}}), {{OperandStack}} now delegates constant 
> pushing uniformly via {{{}visitLdcInsn{}}}. The stateful peephole layer then 
> transparently condenses these instructions into their tightest bytecode 
> representations under the hood.
> h2. Key Optimizations to Implement
> h3. 1. Redundant Instruction & Dead Code Elimination
>  * *Discarded Assignment Values:* Eliminates wasteful {{DUP}} -> {{[X]STORE}} 
> -> {{POP}} patterns typically produced during assignments where the 
> expression result on the operand stack is unused. The optimizer flattens 
> these directly into a single {{{}[X]STORE{}}}.
>  * *Redundant Loads:* Detects situations where a local variable or constant 
> is loaded ({{{}[X]LOAD{}}} / {{{}LDC{}}}) but immediately discarded via 
> {{{}POP{}}}/{{{}POP2{}}}, or followed immediately by a void {{{}RETURN{}}}. 
> The optimizer safely drops both operations while preserving interleaved local 
> increments ({{{}IINC{}}}).
> h3. 2. Conditional Jump & Zero-Comparison Rewriting
>  * Optimizes integer comparisons against zero by transforming expensive 
> binary comparison sequences (e.g., {{ILOAD}} -> {{ICONST_0}} -> 
> {{{}IF_ICMPxx{}}}) into compact unary zero-comparison instructions 
> ({{{}IFxx{}}}). For instance, an {{IF_ICMPEQ}} branch against a buffered 
> {{0}} constant is cleanly rewritten directly to {{{}IFEQ{}}}.
> h3. 3. Instruction Narrowing & Constant Compaction
>  * Intercepts standard literal definitions and transparently narrows them 
> down to the smallest possible specialized opcodes ({{{}ICONST_M1{}}} through 
> {{{}ICONST_5{}}}, {{{}BIPUSH{}}}, {{{}SIPUSH{}}}, {{{}LCONST_0/1{}}}, 
> {{{}FCONST_0/1/2{}}}, and {{{}DCONST_0/1{}}}).
>  * Explicitly safeguards signed floating-point zeros ({{{}-0.0f{}}} and 
> {{{}-0.0d{}}}) from accidental flattening to guarantee strict IEEE 754 
> runtime compliance.
> h3. 4. Big Number Literal Lowering
>  * Centralizes string-constructed object instantiations for {{BigDecimal}} 
> and {{BigInteger}} literals by rewriting buffered constants into inline 
> {{NEW}} -> {{DUP}} -> {{LDC [string]}} -> {{INVOKESPECIAL <init>}} pipelines 
> seamlessly before stack emission.
> h2. Implementation Strategy & Safety Guardrails
> The {{PeepholeOptimizingMethodVisitor}} operates on a lightweight, 
> stack-local sliding window. To fully preserve runtime semantics, debugging 
> capabilities, and catch-block structures, the lookahead window is 
> automatically flushed to the delegate visitor when encountering non-local 
> boundaries or frame-altering instructions, including:
>  * Control flow jumps and basic block {{Label}} targets.
>  * Stack map frames ({{{}visitFrame{}}}).
>  * Line numbers and local variable debug maps.
>  * Method invocations and {{InvokeDynamic}} instructions.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to