mirror/perl
1
0
Fork 0
mirror of https://github.com/Perl/perl5.git synced 2026-01-26 08:38:23 +00:00
Code Issues Packages Projects Releases Wiki Activity

peep.c: Only remove expected empty if/else blocks structures

Browse Source 
The expected OP tree structures for empty blocks fit for removal was:
```
  +-- stub
```
or
```
  +-- scope
    |
    +-- stub
```

GH #23505 showed that `{ BEGIN {} }` blocks will also generate the likes of:
```
  +--scope
    |
    +--stub
    |
    +--null (ex-nextstate)
```

This commit does the minimal possible to fix this bug - the peephole optimizer
now checks for the expected cases and ignores anything else.

Future commits may build on this to remove this additional type of structure.
Additional tests have been added now and give a useful structure for future
commit tests to fit into.
This commit is contained in:
Richard Leach 2025-07-30 21:16:15 +00:00 committed by Karl Williamson
parent 7cdc183e0e
commit 73c046c733
2 changed files with 107 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

117
peep.c
View File

@ -3590,41 +3590,110 @@ Perl_rpeep(pTHX_ OP *o)
case OP_COND_EXPR:
if (o->op_type == OP_COND_EXPR) {
OP *stub = cLOGOP->op_other;
OP *trueop = OpSIBLING( cLOGOP->op_first );
OP *falseop = OpSIBLING(trueop);
/* Is there an empty "if" block or ternary true branch?
If so, optimise away the OP_STUB if safe to do so. */
if (stub->op_type == OP_STUB &&
((stub->op_flags & OPf_WANT) != OPf_WANT_SCALAR)
) {
OP *trueop = OpSIBLING( cLOGOP->op_first );
if (stub == trueop) {
/* This is very unlikely:
* cond_expr
* -condition-
* stub
* -else-
*/
assert(!(stub->op_flags & OPf_KIDS));
cLOGOP->op_other = stub->op_next;
op_sibling_splice(o, cLOGOP->op_first, 1, NULL);
op_free(stub);
break;
} else if (OP_TYPE_IS(trueop, OP_SCOPE) &&
(stub == cUNOPx(trueop)->op_first) ) {
assert(!(stub->op_flags & OPf_KIDS));
assert((stub == trueop ) || (OP_TYPE_IS(trueop, OP_SCOPE) &&
((stub == cUNOPx(trueop)->op_first)) && !OpSIBLING(stub))
);
assert(!(stub->op_flags & OPf_KIDS));
cLOGOP->op_other = (stub->op_next == trueop) ?
stub->op_next->op_next :
stub->op_next;
op_sibling_splice(o, cLOGOP->op_first, 1, NULL);
if (stub != trueop) op_free(stub);
op_free(trueop);
} else
OP *stubsib = OpSIBLING(stub);
if (!stubsib) {
/* cond_expr
* -condition-
* scope
* stub
* -else-
*/
cLOGOP->op_other = trueop->op_next;
op_sibling_splice(o, cLOGOP->op_first, 1, NULL);
op_free(stub);
op_free(trueop);
break;
} else {
/* Could be something like this:
* -condition-
* scope
* stub
* null
* -else-
* But it may be more desirable (but is less
* straightforward) to transform this earlier
* in the compiler. Ignoring it for now,
* pending further exploration. */
}
}
}
/* Is there an empty "else" block or ternary false branch?
If so, optimise away the OP_STUB if safe to do so. */
stub = o->op_next;
if (stub->op_type == OP_STUB &&
((stub->op_flags & OPf_WANT) != OPf_WANT_SCALAR)
) {
assert(stub == OpSIBLING(OpSIBLING( cLOGOP->op_first )));
assert(!(stub->op_flags & OPf_KIDS));
o->op_flags |= OPf_SPECIAL; /* For B::Deparse */
o->op_next = stub->op_next;
op_sibling_splice(o, OpSIBLING(cLOGOP->op_first), 1, NULL);
op_free(stub);
if ((stub->op_flags & OPf_WANT) != OPf_WANT_SCALAR) {
if (stub->op_type == OP_STUB && !OpSIBLING(stub) ){
OP *stubsib = OpSIBLING(stub);
if ((stub == falseop) && !stubsib) {
/* cond_expr
* -condition-
* - if -
* stub
*/
assert(!(stub->op_flags & OPf_KIDS));
o->op_flags |= OPf_SPECIAL; /* For B::Deparse */
o->op_next = stub->op_next;
op_sibling_splice(o, OpSIBLING(cLOGOP->op_first), 1, NULL);
op_free(stub);
} else { /* Unexpected */ }
} else if (OP_TYPE_IS(stub,OP_ENTER) &&
OP_TYPE_IS(falseop, OP_LEAVE)) {
OP *enter = stub;
OP *stub = OpSIBLING(enter);
if (stub && OP_TYPE_IS(stub, OP_STUB) ){
assert(!(stub->op_flags & OPf_KIDS));
OP *stubsib = OpSIBLING(stub);
assert(stubsib);
if (OP_TYPE_IS(stubsib, OP_NULL) &&
!OpSIBLING(stubsib) &&
!(stubsib->op_flags & OPf_KIDS) ) {
/* cond_expr
* -condition-
* - if -
* leave
* enter
* stub
* null
*/
/* Ignoring it for now, pending further exploration.*/
/*
o->op_flags |= OPf_SPECIAL; // For B::Deparse
o->op_next = falseop->op_next;
op_sibling_splice(o, OpSIBLING(cLOGOP->op_first), 1, NULL);
op_free(enter);
op_free(stub);
op_free(stubsib);
op_free(falseop);
*/
}
}
}
}
}
/* FALLTHROUGH */
case OP_MAPWHILE:

15
t/perf/opcount.t
View File

@ -1290,13 +1290,26 @@ test_opcount(0, "defined(ABC) gets constant folded",
defined => 0,
});
# Empty condop other/next branch optimizations
test_opcount(0, "Empty if{} blocks are optimised away",
sub { my $x; if ($x) { } else { 1 } },
{
stub => 0
});
test_opcount(0, "Empty else{} blocks are optimised away",
sub { my $x; if ($x) { 1 } else { } },
{
stub => 0
});
test_opcount(0, "Empty ternary true blocks are optimised away",
sub { my $x; ($x) ? () : 1 },
{
stub => 0
});
test_opcount(0, "Empty else{} blocks are optimised away",
test_opcount(0, "Empty ternary false blocks are optimised away",
sub { my $x; ($x) ? 1 : () },
{
stub => 0