[
https://issues.apache.org/jira/browse/GROOVY-12162?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Daniel Sun updated GROOVY-12162:
--------------------------------
Description:
h2. Summary
Follow-up to GROOVY-12065. Apply the single-pass peephole layer to *every*
method, restore classgen diagnostics after visitor wrapping, and extend safe
stack-local rewrites so generated bytecode stays compact without changing
runtime semantics.
h2. Motivation
After GROOVY-12065, {{OperandStack}} emits constants uniformly via
{{visitLdcInsn}} and relies on {{PeepholeOptimizingMethodVisitor}} to narrow
them ({{ICONST_*}}, {{BIPUSH}}, {{SIPUSH}}, etc.).
Two gaps remained:
# *Incomplete coverage* — the peephole wrapper was applied only at selected
{{visitMethod}} sites. Synthetic helpers (call-site array init, large-list
chunks, MOP bridges, …) often bypassed it and kept non-compact forms.
# *Broken diagnostics* — with the peephole visitor outside
{{TraceMethodVisitor}}, {{mv instanceof TraceMethodVisitor}} failed, so "Last
known generated bytecode" was never printed when {{visitMaxs}} failed under
classgen logging.
h2. Approach
h3. Uniform installation
* Introduce {{PeepholeOptimizingClassVisitor}}, installed once from
{{WriterController}}, so every method body is wrapped.
* Remove ad-hoc {{new PeepholeOptimizingMethodVisitor(...)}} from
{{AsmClassGenerator}} and {{CallSiteWriter}}.
* {{wrap(null)}} propagates {{null}} unchanged (ASM may skip a method body).
h3. Diagnostics
* Add {{PeepholeOptimizingMethodVisitor.printTraceBytecode}} to walk nested
peephole layers and print an inner {{TraceMethodVisitor}} when present (used by
{{AsmClassGenerator}} on {{visitMaxs}} failure).
h3. Extended rewrites (still single-pass, one pending load / one pending DUP
window)
* *CHECKCAST attachment* — attach to pending {{ALOAD}} or reference constant so
{{load}}; {{CHECKCAST}}; {{POP}} collapses entirely when the value is unused;
keep the cast when the value is used.
* *Null compare* — {{ACONST_NULL}}; {{IF_ACMPEQ}}/{{IF_ACMPNE}} →
{{IFNULL}}/{{IFNONNULL}}.
* *Bare DUP* — {{DUP}}/{{DUP2}} (+ optional store) + matching {{POP}}/{{POP2}}
→ store only, or eliminate entirely.
* *Void RETURN* — drop dead loads/constants (preserve paired {{IINC}}); for
pending {{CHECKCAST}} (standalone or attached), emit the cast then {{POP}} so
the type-check side effect is kept and the void return stays verifiable.
* *Constants* — integer emission reuses {{BytecodeHelper.pushConstant}} on the
*delegate* (no re-buffering); {{ConstantDynamic}} is not buffered (bootstrap
may have side effects).
* *Zero compare* — {{ICONST_0}}; {{IF_ICMP*}} → {{IF*}} via the fixed JVM
opcode offset ({{IF_ICMPEQ}}…{{IF_ICMPLE}} ↔ {{IFEQ}}…{{IFLE}}).
h2. Safety
* Single basic-block window only; flush before labels, frames, line numbers,
invokes, switches, maxs/end, and other non-local boundaries.
* Signed floating-point zeros still compared by raw bits (GROOVY-9797).
* No second compilation pass; no full data-flow analysis.
was:
h2. Overview
Follow-up to GROOVY-12065. The initial peephole layer
({{{}PeepholeOptimizingMethodVisitor{}}}) only wrapped selected
{{visitMethod}} call sites. After {{OperandStack}} started
emitting constants via {{visitLdcInsn}} and relying on the
peephole visitor for compaction, any method that was *not* wrapped
regressed to non-compact forms. Class-generation error diagnostics
that depended on {{TraceMethodVisitor}} were also broken once
the peephole wrapper sat in front of the trace visitor.
This improvement applies peephole compaction uniformly, restores
diagnostics, and adds several safe local-window rewrites that
further shrink generated bytecode without changing runtime
semantics.
h2. Problems Addressed
* *Inconsistent application:* MOP bridges, call-site helpers,
{{{}class${}}}, and other synthetic methods often bypassed the
peephole wrapper while user methods did not.
* *Broken classgen diagnostics:*
{{mv instanceof TraceMethodVisitor}} no longer matched after
wrapping, so "Last known generated bytecode" was never printed
when {{visitMaxs}} failed under classgen logging.
* *Incomplete dead-code patterns:*
{{{}ALOAD{}}}/reference {{{}LDC{}}}; {{{}CHECKCAST{}}}; {{POP}}
only dropped the cast (the load remained); bare
{{{}DUP{}}}/{{{}DUP2{}}}; matching {{{}POP{}}}/{{{}POP2{}}} was left
untouched; {{{}ACONST_NULL{}}}; {{IF_ACMP*}} was not rewritten
to {{{}IFNULL{}}}/{{{}IFNONNULL{}}}.
* *Side-effect caution:* dropping a standalone {{CHECKCAST}}
immediately before void {{RETURN}} could discard a type-check
side effect for a value already committed to the stack.
h2. Changes
h3. Uniform application
* Introduce {{{}PeepholeOptimizingClassVisitor{}}}, installed once
from {{{}WriterController{}}}, so every method body (including
synthetic helpers) is routed through
{{{}PeepholeOptimizingMethodVisitor{}}}.
* Remove ad-hoc {{new PeepholeOptimizingMethodVisitor(...)}}
wraps from {{AsmClassGenerator}} and {{{}CallSiteWriter{}}}.
h3. Diagnostics
* Add {{PeepholeOptimizingMethodVisitor.printTraceBytecode}} to
walk the visitor chain and print a nested
{{TraceMethodVisitor}} when present.
h3. Additional rewrites (still single-pass / stack-local)
* Attach {{CHECKCAST}} to a pending {{ALOAD}} or reference
constant so load + cast + {{POP}} collapses completely when
the value is unused; keep the cast when the value is used.
* Rewrite {{{}ACONST_NULL{}}}; {{{}IF_ACMPEQ{}}}/{{{}IF_ACMPNE{}}} to
{{{}IFNULL{}}}/{{{}IFNONNULL{}}}.
* Eliminate bare {{{}DUP{}}}/{{{}DUP2{}}} followed by a matching pop
(with or without an intervening store → store-only form).
* On void {{{}RETURN{}}}, drop only pending variable/constant loads
(preserving {{{}IINC{}}}); flush a standalone {{CHECKCAST}}
instead of discarding it.
* Buffer replacement flushes prior pending state instead of
silently clearing it; integer constant emission reuses
{{BytecodeHelper.pushConstant}} on the delegate.
h2. Safety
The visitor remains a single candidate window: pending state is
flushed at labels, frames, line numbers, invokes, and other
non-local boundaries. Signed zero constants continue to use raw-bit
comparisons (GROOVY-9797).
> Harden and extend bytecode peephole optimization
> ------------------------------------------------
>
> Key: GROOVY-12162
> URL: https://issues.apache.org/jira/browse/GROOVY-12162
> Project: Groovy
> Issue Type: Improvement
> Reporter: Daniel Sun
> Priority: Major
>
> h2. Summary
> Follow-up to GROOVY-12065. Apply the single-pass peephole layer to *every*
> method, restore classgen diagnostics after visitor wrapping, and extend safe
> stack-local rewrites so generated bytecode stays compact without changing
> runtime semantics.
> h2. Motivation
> After GROOVY-12065, {{OperandStack}} emits constants uniformly via
> {{visitLdcInsn}} and relies on {{PeepholeOptimizingMethodVisitor}} to narrow
> them ({{ICONST_*}}, {{BIPUSH}}, {{SIPUSH}}, etc.).
> Two gaps remained:
> # *Incomplete coverage* — the peephole wrapper was applied only at selected
> {{visitMethod}} sites. Synthetic helpers (call-site array init, large-list
> chunks, MOP bridges, …) often bypassed it and kept non-compact forms.
> # *Broken diagnostics* — with the peephole visitor outside
> {{TraceMethodVisitor}}, {{mv instanceof TraceMethodVisitor}} failed, so "Last
> known generated bytecode" was never printed when {{visitMaxs}} failed under
> classgen logging.
> h2. Approach
> h3. Uniform installation
> * Introduce {{PeepholeOptimizingClassVisitor}}, installed once from
> {{WriterController}}, so every method body is wrapped.
> * Remove ad-hoc {{new PeepholeOptimizingMethodVisitor(...)}} from
> {{AsmClassGenerator}} and {{CallSiteWriter}}.
> * {{wrap(null)}} propagates {{null}} unchanged (ASM may skip a method body).
> h3. Diagnostics
> * Add {{PeepholeOptimizingMethodVisitor.printTraceBytecode}} to walk nested
> peephole layers and print an inner {{TraceMethodVisitor}} when present (used
> by {{AsmClassGenerator}} on {{visitMaxs}} failure).
> h3. Extended rewrites (still single-pass, one pending load / one pending DUP
> window)
> * *CHECKCAST attachment* — attach to pending {{ALOAD}} or reference constant
> so {{load}}; {{CHECKCAST}}; {{POP}} collapses entirely when the value is
> unused; keep the cast when the value is used.
> * *Null compare* — {{ACONST_NULL}}; {{IF_ACMPEQ}}/{{IF_ACMPNE}} →
> {{IFNULL}}/{{IFNONNULL}}.
> * *Bare DUP* — {{DUP}}/{{DUP2}} (+ optional store) + matching
> {{POP}}/{{POP2}} → store only, or eliminate entirely.
> * *Void RETURN* — drop dead loads/constants (preserve paired {{IINC}}); for
> pending {{CHECKCAST}} (standalone or attached), emit the cast then {{POP}} so
> the type-check side effect is kept and the void return stays verifiable.
> * *Constants* — integer emission reuses {{BytecodeHelper.pushConstant}} on
> the *delegate* (no re-buffering); {{ConstantDynamic}} is not buffered
> (bootstrap may have side effects).
> * *Zero compare* — {{ICONST_0}}; {{IF_ICMP*}} → {{IF*}} via the fixed JVM
> opcode offset ({{IF_ICMPEQ}}…{{IF_ICMPLE}} ↔ {{IFEQ}}…{{IFLE}}).
> h2. Safety
> * Single basic-block window only; flush before labels, frames, line numbers,
> invokes, switches, maxs/end, and other non-local boundaries.
> * Signed floating-point zeros still compared by raw bits (GROOVY-9797).
> * No second compilation pass; no full data-flow analysis.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
