Skip to content

Commit 7015de1

Browse files
committed
op_scope/newCONDOP: Optimise away empty else blocks in OP_COND_EXPR
This commit comprises two main changes: * Not wrapping a bare OP_STUB in an enter/leave pair (Perl_op_scope) * Checking for a bare OP_STUB in the `falseop` argument when building an OP_COND_EXPR and then freeing it, rather than adding it as a sibling to the `first` and `trueop` branches. The main benefits of this are: * lower memory usage due to unnecessary OP removal * faster execution due to unnecessary OPs not being executed There are also some small changes to Deparse.pm and an additional Deparse.t test.
1 parent 6f83688 commit 7015de1

File tree

3 files changed

+39
-11
lines changed

3 files changed

+39
-11
lines changed

lib/B/Deparse.pm

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3966,13 +3966,22 @@ sub pp_cond_expr {
39663966
my $true = $cond->sibling;
39673967
my $false = $true->sibling;
39683968
my $cuddle = $self->{'cuddle'};
3969-
unless ($cx < 1 and (is_scope($true) and $true->name ne "null") and
3970-
(is_scope($false) || is_ifelse_cont($false))
3971-
and $self->{'expand'} < 7) {
3972-
$cond = $self->deparse($cond, 8);
3973-
$true = $self->deparse($true, 6);
3974-
$false = $self->deparse($false, 8);
3975-
return $self->maybe_parens("$cond ? $true : $false", $cx, 8);
3969+
3970+
if (class($false) eq "NULL") { # Empty else {} block was optimised away
3971+
unless ($cx < 1 and (is_scope($true) and $true->name ne "null")) {
3972+
$cond = $self->deparse($cond, 8);
3973+
$true = $self->deparse($true, 6);
3974+
return $self->maybe_parens("$cond ? $true : ()", $cx, 8);
3975+
}
3976+
} else { # Both true and false branches are present
3977+
unless ($cx < 1 and (is_scope($true) and $true->name ne "null")
3978+
and (is_scope($false) || is_ifelse_cont($false))
3979+
and $self->{'expand'} < 7) {
3980+
$cond = $self->deparse($cond, 8);
3981+
$true = $self->deparse($true, 6);
3982+
$false = $self->deparse($false, 8);
3983+
return $self->maybe_parens("$cond ? $true : $false", $cx, 8);
3984+
}
39763985
}
39773986

39783987
$cond = $self->deparse($cond, 1);

lib/B/Deparse.t

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3378,3 +3378,7 @@ $_ = (!$p) isa 'Some::Class';
33783378
$_ = (!$p) =~ tr/1//;
33793379
$_ = (!$p) =~ /1/;
33803380
$_ = (!$p) =~ s/1//r;
3381+
####
3382+
# Else block of a ternary is optimised away
3383+
my $x;
3384+
my(@y) = $x ? [1, 2] : ();

op.c

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4517,6 +4517,12 @@ Perl_op_scope(pTHX_ OP *o)
45174517
{
45184518
if (o) {
45194519
if (o->op_flags & OPf_PARENS || PERLDB_NOOPT || TAINTING_get) {
4520+
4521+
/* There's no need to wrap an empty stub in an enter/leave.
4522+
* This also makes eliding empty if/else blocks simpler. */
4523+
if (OP_TYPE_IS(o, OP_STUB) && (o->op_flags & OPf_PARENS))
4524+
return o;
4525+
45204526
o = op_prepend_elem(OP_LINESEQ,
45214527
newOP(OP_ENTER, (o->op_flags & OPf_WANT)), o);
45224528
OpTYPE_set(o, OP_LEAVE);
@@ -9268,7 +9274,6 @@ Perl_newCONDOP(pTHX_ I32 flags, OP *first, OP *trueop, OP *falseop)
92689274
logop = alloc_LOGOP(OP_COND_EXPR, first, LINKLIST(trueop));
92699275
logop->op_flags |= (U8)flags;
92709276
logop->op_private = (U8)(1 | (flags >> 8));
9271-
logop->op_next = LINKLIST(falseop);
92729277

92739278
CHECKOP(OP_COND_EXPR, /* that's logop->op_type */
92749279
logop);
@@ -9277,13 +9282,23 @@ Perl_newCONDOP(pTHX_ I32 flags, OP *first, OP *trueop, OP *falseop)
92779282
start = LINKLIST(first);
92789283
first->op_next = (OP*)logop;
92799284

9280-
/* make first, trueop, falseop siblings */
9285+
/* make first & trueop siblings */
9286+
/* falseop may also be a sibling, if not optimised away */
92819287
op_sibling_splice((OP*)logop, first, 0, trueop);
9282-
op_sibling_splice((OP*)logop, trueop, 0, falseop);
92839288

92849289
o = newUNOP(OP_NULL, 0, (OP*)logop);
92859290

9286-
trueop->op_next = falseop->op_next = o;
9291+
if (OP_TYPE_IS(falseop, OP_STUB) && (falseop->op_flags & OPf_PARENS)) {
9292+
/* The `else` block is empty. Optimise it away. */
9293+
logop->op_next = o;
9294+
op_free(falseop);
9295+
} else {
9296+
op_sibling_splice((OP*)logop, trueop, 0, falseop);
9297+
logop->op_next = LINKLIST(falseop);
9298+
falseop->op_next = o;
9299+
}
9300+
9301+
trueop->op_next = o;
92879302

92889303
o->op_next = start;
92899304
return o;

0 commit comments

Comments
 (0)