[
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. Make the single-pass peephole layer apply to *every*
method body, restore classgen diagnostics after visitor wrapping, and extend
safe stack-local rewrites so generated bytecode stays compact without changing
runtime semantics.
h2. Background
After GROOVY-12065, {{OperandStack}} emits many constants uniformly via
{{visitLdcInsn}} and relies on {{PeepholeOptimizingMethodVisitor}} to narrow
them ({{ICONST_*}}, {{BIPUSH}}, {{SIPUSH}}, etc.).
Two practical 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. Goals
* One installation point so *all* methods (user + synthetic) get the same
compaction.
* Correct classgen logging / failure dumps through the wrapper chain.
* More rewrites that stay single-pass, stack-local, and semantics-preserving.
* Strong unit coverage for edge cases (CHECKCAST side effects, void {{RETURN}},
signed zeros, box/unbox mismatch).
h2. Approach
h3. Uniform installation
* Add {{PeepholeOptimizingClassVisitor}}, installed once from
{{WriterController}}, wrapping every method visitor.
* Remove ad-hoc {{new PeepholeOptimizingMethodVisitor(...)}} from
{{AsmClassGenerator}} and {{CallSiteWriter}}.
* {{PeepholeOptimizingMethodVisitor.wrap(null)}} propagates {{null}} unchanged
(ASM may skip a method body); wrap is idempotent.
h3. Diagnostics
* Add {{PeepholeOptimizingMethodVisitor.printTraceBytecode}} to walk nested
peephole layers and print an inner {{TraceMethodVisitor}} when present.
* {{AsmClassGenerator}} uses it on {{visitMaxs}} failure so classgen logging
still shows the last known bytecode.
h3. Extended rewrites (single basic-block window)
Still at most one pending load, one pending box, and one pending {{DUP}}/store:
* *CHECKCAST attachment* — attach to pending {{ALOAD}} or reference constant so
{{load}}; {{CHECKCAST}}; {{POP}} can collapse when unused; keep the cast when
the value is used.
* *Null compare* — {{ACONST_NULL}}; {{IF_ACMPEQ}}/{{IF_ACMPNE}} →
{{IFNULL}}/{{IFNONNULL}}.
* *Zero compare* — {{ICONST_0}}; {{IF_ICMP*}} → {{IF*}} via the fixed JVM
opcode offset ({{IF_ICMPEQ}}…{{IF_ICMPLE}} ↔ {{IFEQ}}…{{IFLE}}).
* *Bare DUP / DUP+store+pop* — matching {{POP}}/{{POP2}} collapses to a plain
store, or eliminates the dup 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* — integers re-emitted via {{BytecodeHelper.pushConstant}} on the
*delegate* (no re-buffering); {{ConstantDynamic}} is never buffered (bootstrap
may have side effects); signed FP zeros still compared by raw bits
(GROOVY-9797).
* *Box/unbox cancellation* (gjit-inspired) — matching {{Wrapper.valueOf}} +
{{xxxValue}}, or {{DefaultTypeTransformation.box}} + {{xxxUnbox}}, cancel to a
no-op; mismatched types are left intact.
* *Dead box* — {{valueOf}}/{{box}} + {{POP}}/{{POP2}} drops the box and pops
the original primitive ({{POP2}} when wide).
* *Boolean constant folding* — {{GETSTATIC Boolean.TRUE/FALSE}} +
{{booleanValue}}/{{booleanUnbox}} → {{ICONST_1}}/{{ICONST_0}}.
h2. Safety
* Flush pending state before labels, frames, line numbers, invokes (except
matched box/unbox), switches, maxs/end, and other non-local boundaries.
* No second compilation pass; no full data-flow analysis.
* Observable behaviour unchanged; only denser equivalent bytecode within a
basic block.
was:
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.
> 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. Make the single-pass peephole layer apply to
> *every* method body, restore classgen diagnostics after visitor wrapping, and
> extend safe stack-local rewrites so generated bytecode stays compact without
> changing runtime semantics.
> h2. Background
> After GROOVY-12065, {{OperandStack}} emits many constants uniformly via
> {{visitLdcInsn}} and relies on {{PeepholeOptimizingMethodVisitor}} to narrow
> them ({{ICONST_*}}, {{BIPUSH}}, {{SIPUSH}}, etc.).
> Two practical 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. Goals
> * One installation point so *all* methods (user + synthetic) get the same
> compaction.
> * Correct classgen logging / failure dumps through the wrapper chain.
> * More rewrites that stay single-pass, stack-local, and semantics-preserving.
> * Strong unit coverage for edge cases (CHECKCAST side effects, void
> {{RETURN}}, signed zeros, box/unbox mismatch).
> h2. Approach
> h3. Uniform installation
> * Add {{PeepholeOptimizingClassVisitor}}, installed once from
> {{WriterController}}, wrapping every method visitor.
> * Remove ad-hoc {{new PeepholeOptimizingMethodVisitor(...)}} from
> {{AsmClassGenerator}} and {{CallSiteWriter}}.
> * {{PeepholeOptimizingMethodVisitor.wrap(null)}} propagates {{null}}
> unchanged (ASM may skip a method body); wrap is idempotent.
> h3. Diagnostics
> * Add {{PeepholeOptimizingMethodVisitor.printTraceBytecode}} to walk nested
> peephole layers and print an inner {{TraceMethodVisitor}} when present.
> * {{AsmClassGenerator}} uses it on {{visitMaxs}} failure so classgen logging
> still shows the last known bytecode.
> h3. Extended rewrites (single basic-block window)
> Still at most one pending load, one pending box, and one pending
> {{DUP}}/store:
> * *CHECKCAST attachment* — attach to pending {{ALOAD}} or reference constant
> so {{load}}; {{CHECKCAST}}; {{POP}} can collapse when unused; keep the cast
> when the value is used.
> * *Null compare* — {{ACONST_NULL}}; {{IF_ACMPEQ}}/{{IF_ACMPNE}} →
> {{IFNULL}}/{{IFNONNULL}}.
> * *Zero compare* — {{ICONST_0}}; {{IF_ICMP*}} → {{IF*}} via the fixed JVM
> opcode offset ({{IF_ICMPEQ}}…{{IF_ICMPLE}} ↔ {{IFEQ}}…{{IFLE}}).
> * *Bare DUP / DUP+store+pop* — matching {{POP}}/{{POP2}} collapses to a plain
> store, or eliminates the dup 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* — integers re-emitted via {{BytecodeHelper.pushConstant}} on
> the *delegate* (no re-buffering); {{ConstantDynamic}} is never buffered
> (bootstrap may have side effects); signed FP zeros still compared by raw bits
> (GROOVY-9797).
> * *Box/unbox cancellation* (gjit-inspired) — matching {{Wrapper.valueOf}} +
> {{xxxValue}}, or {{DefaultTypeTransformation.box}} + {{xxxUnbox}}, cancel to
> a no-op; mismatched types are left intact.
> * *Dead box* — {{valueOf}}/{{box}} + {{POP}}/{{POP2}} drops the box and pops
> the original primitive ({{POP2}} when wide).
> * *Boolean constant folding* — {{GETSTATIC Boolean.TRUE/FALSE}} +
> {{booleanValue}}/{{booleanUnbox}} → {{ICONST_1}}/{{ICONST_0}}.
> h2. Safety
> * Flush pending state before labels, frames, line numbers, invokes (except
> matched box/unbox), switches, maxs/end, and other non-local boundaries.
> * No second compilation pass; no full data-flow analysis.
> * Observable behaviour unchanged; only denser equivalent bytecode within a
> basic block.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
