daniellansun commented on code in PR #2707:
URL: https://github.com/apache/groovy/pull/2707#discussion_r3604142551
##########
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']
Review Comment:
Two separate points:
1. **ALOAD → RETURN:** NOP flushes the load, so ALOAD is already committed;
the
single-window pass can’t reclaim it. Pulling that back needs multi-block
DCE
/ a second pass — follow-up.
2. **CHECKCAST drop:** per Paul’s review we no longer drop a discarded
CHECKCAST (POP and void-RETURN now both keep the cast for CCE). So this
sequence becomes `ALOAD; NOP; CHECKCAST; POP; RETURN`, not bare RETURN.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
