Skip to content

Optimise away empty if{}, elsif{}. and else {} blocks #23367

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: blead
Choose a base branch
from

Conversation

richardleach
Copy link
Contributor

Note: this is an (extended) replacement for #22745

These commits:

  • Change Perl_op_scope so that a bare OP_STUB is not wrapped in an OP_ENTER/OP_LEAVE pair.
  • Change Perl_rpeep to remove the that OP_STUB when it is safe to do so (not in scalar context).

Tests and B::Deparse support are included.

For example, given my $x; if ($x) {print 1} else {}, the relevant piece of the optree looked like this before:

4     <;> nextstate(main 2 -e:1) v:{ ->5
-     <1> null vK/1 ->a
6        <|> cond_expr(other->7) vK/1 ->b
5           <0> padsv[$x:1,8] s ->6
-           <@> scope vK ->-
-              <;> ex-nextstate(main 4 -e:1) v ->7
9              <@> print vK ->a
7                 <0> pushmark s ->8
8                 <$> const[IV 1] s ->9
d           <@> leave vK ->a
b              <0> enter v ->c
c              <0> stub vP ->d

With these commits, it looks like:

4     <;> nextstate(main 2 -e:1) v:{ ->5
-     <1> null vK/1 ->a
6        <|> cond_expr(other->7) vK*/1 ->a
5           <0> padsv[$x:1,8] s ->6
-           <@> scope vK ->-
-              <;> ex-nextstate(main 4 -e:1) v ->7
9              <@> print vK ->a
7                 <0> pushmark s ->8
8                 <$> const[IV 1] s ->9

  • I'm not sure if this set of changes requires a perldelta entry.

This is a replacement for Perl#22745, which
aimed to prevent the associated `OP_STUB` from being created in
`Perl_newCONDOP` and subsequently removed in peep.c. However, doing that
turned out to be unavoidable, since it is not safe to omit the `OP_STUB`
if it is in scalar context, but `Perl_newCONDOP` runs before context is
applied.

This commit:
 * Re-adds the logic in `Perl_op_scope` to not wrap a bare `OP_STUB` in an
   `ENTER/LEAVE` pair.
 * Removes any relevant `OP_STUB`s in non-scalar context in `Perl_rpeep`.
 * Adds B::Deparse support
 * Adds a test based on the failures seen in Perl#22866
@richardleach richardleach added the defer-next-dev This PR should not be merged yet, but await the next development cycle label Jun 11, 2025
This commit optimises away the OP_STUB associated with an empty `true`
branch of an `OP_COND_EXPR`.

The `OPf_SPECIAL` flag has been brought into use on the `OP_COND_EXPR`
to indicate that it is the `else`, not the `if`, block that has been
optimised away. This is purely for the benefit of B::Deparse.
@richardleach richardleach force-pushed the hydahy/omit_empty_else2 branch from e6e5fef to 326e2e2 Compare June 12, 2025 22:13
@bulk88
Copy link
Contributor

bulk88 commented Jun 12, 2025

troll argument: will this "break" or change flaky PP single stepping to extra crispy flaky PP single stepping?

I don't have the time to compile this PR and do a deep dive on how Komodo's red dots and yellow arrow behave B4/AF when Im rapid tapping F11. The yellow arrow has never been perfect for me, 5.10 thru blead, its good enough for me, its not perfect. The yellow arrow has always gotten weird around constant folded if elsif {} braces and loop open loop close {} braces since I wrote PP hello world.

Update:

BBQ glazed step rolls argument: Will this PR break https://metacpan.org/pod/Enbugger ?

@bulk88 last used https://metacpan.org/pod/Enbugger in 2016. After 2016 he reverse engineered Komodo's PP DBR's PP DBR injector code, along with rev engineering and synthesizing cmd line -D in PP code. Its really easy, just keep a strategic copy paste in a .txt you add to whatever strict.pm your perl of the day is executing, and obviously its #ed off 99% of the time inside strict.pm, but no .t or perl.exe in the world will escape my little trick. Shell vars %PERL5OPT% and %PERL5DB% are useless IMO. There are entire frameworks on CPAN, Where the root class/root .pm in a BEGIN{}, think of huge frameworks like Alien:: or Moose::, I forgot the exact names of what CPAN base class modules did this to me. But %PERL5OPT% and %PERL5DB% are utterly useless, since the .t or PolymorphicMetaProgramming::Base::Test.pm probes $ENV{PERL5OPT} and $ENV{PERL5DB} and then deletes the 2 env vars, then launches fresh_perl_like($0);

Even better is CPAN authors who have a secure coding policy of if (keys %ENV) {local(%ENV); exit(system($^X, $0));}. I've had to fix this 2 or 4 in the past atleast once involving CPAN modules. Hence time to edit strict.pm by hand and pretend $ENV{PERL5OPT} and $ENV{PERL5DB} and -D dont exist in Perl.

@bulk88
Copy link
Contributor

bulk88 commented Jun 12, 2025

Also, more seriously, would this optimization degrade or break NYTProf? ?

@richardleach
Copy link
Contributor Author

Also, more seriously, would this optimization degrade or break NYTProf? ?

I'll try to check at the weekend.

I haven't used Komodo in yonks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defer-next-dev This PR should not be merged yet, but await the next development cycle
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants