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
peephole optimiser could prune more dead code #10481
Comments
From @nwc10Created by @nwc10$ ./perl -Ilib -MO=Deparse -e 'if ("Pie" eq "Good") {print}' but $ ./perl -Ilib -MO=Deparse -e 'if ($a && "Pie" eq "Good") {print}' which demonstrates that "Pie" eq "Good" is constant folded, but that the The peephole optimiser is correct not to optimise this to nothing, as it However, it can know that the conditional to the if block is always false, $a and !1; or even the perl equivalent of (void) (bool) $a; Wishlist, because I've no idea how much real world perl code ends up with Nicholas Clark Perl Info
|
From james@mastros.bizOn 9 July 2010 16:56, Nicholas Clark <perlbug-followup@perl.org> wrote:
I do wonder, sometimes, if we worry entirely too much about just when -=- James Mastros / theorbtwo |
The RT System itself - Status changed from 'new' to 'open' |
From @jbenjoreOn Fri, Jul 9, 2010 at 8:56 AM, Nicholas Clark
Well actually, the 'bool' call was in scalar context, not void. Josh |
From @demerphqOn 10 July 2010 13:36, James Mastros <james@mastros.biz> wrote:
This came up in another thread. JIT compilation techniques combined I also proposed a "no magic" pragma/syntax that would allow the cheers, -- |
From @LeontOn Sat, Jul 10, 2010 at 4:17 PM, demerphq <demerphq@gmail.com> wrote:
The use of magic is too pervasive for that. Not only because many I don't think it's workable. Leon |
From ben@morrow.me.ukQuoth fawaka@gmail.com (Leon Timmermans):
Most of those forms of magic don't have meaningful side-effects, though, Ben |
From @LeontOn Sat, Jul 10, 2010 at 9:16 PM, Ben Morrow <ben@morrow.me.uk> wrote:
If you're only ignoring get magic and not set magic, I guess it's Leon |
From @druud62James Mastros wrote:
no tie; no overload; or use optimize qw( :no_overload :no_tie ); ? -- |
From ben@morrow.me.ukQuoth rvtol+usenet@isolution.nl ("Dr.Ruud"):
Obviously, this should be use less "magic"; :) Ben |
From james@mastros.bizOn 11 July 2010 00:25, Leon Timmermans <fawaka@gmail.com> wrote:
I think what Nick was getting at, and certainly what I was getting at, { is OK. We invoke the get magic, once, and assume the value hasn't @_ elements is probably a matter of "don't do that, then". However, -=- James Mastros / theorbtwo |
From @nwc10On Sun, Jul 11, 2010 at 10:35:05AM +0100, James Mastros wrote:
No, that's not what *I* was getting at. That's the entire thread that has gone $ ./perl -Ilib -MO=Deparse -e 'if ($a && "Pie" eq "Good") {print}' ie there is provably dead code still in the optree - the print statement. (Which could be removed by the optimiser, without any change to any semantic Nicholas Clark |
From @ikegamiOn Sun, Jul 11, 2010 at 5:35 AM, James Mastros <james@mastros.biz> wrote:
We're not instructing Perl to be less magical, we're promising Perl we won't |
From @xdgOn Sat, Jul 10, 2010 at 7:36 AM, James Mastros <james@mastros.biz> wrote:
I think explicitly clarifying that side effects (like magic) will not Then maybe we could use the less pragma when that isn't desired. ( -- David |
From @ikegamiOn Sun, Jul 11, 2010 at 3:40 PM, David Golden <xdaveg@gmail.com> wrote:
Sounds great to me, except there are backward compatibility issues to |
From @jbenjoreOn Sun, Jul 11, 2010 at 2:56 PM, Eric Brine <ikegami@adaelis.com> wrote:
Sounds like you want some instrumentation for where your magic is Josh |
From @xdgOn Sun, Jul 11, 2010 at 5:58 PM, Joshua ben Jore <twists@gmail.com> wrote:
(Insert xdg rant on backwards compatibility) I'm suggesting that we disclaim any implicit guarantee that the Whether any particular optimization is "worth it" is open for later Notwithstanding that Nicholas points out that this example isn't what Effectively: if ($a && 0) { ... } # could be optimized away entirely I don't know how much code *relies* on something like $a being tied warn "..." if $something && DEBUG; -- David |
From @ikegamiOn Sun, Jul 11, 2010 at 10:22 PM, David Golden <xdaveg@gmail.com> wrote:
Without that guarantee, my $x = f() would be buggy. Dunno if that matters |
From @xdgOn Mon, Jul 12, 2010 at 12:51 AM, Eric Brine <ikegami@adaelis.com> wrote:
I said "... have side effects when evaluated ..." but your example has David |
From @demerphqOn 12 July 2010 13:07, David Golden <xdaveg@gmail.com> wrote:
I think this is close to something i mentioned. My thought was: given that $b=$a++ + $a++; is not defined, that we could also assume that changing fetch magic $b=$a + $a; where $a is tied/overloaded and the magic changes on invocation of the Thus we could check for magic at the beginning of the expression, and But the problem with any of these changes is that it could/would break cheers, -- |
From @demerphqOn 12 July 2010 13:36, demerphq <demerphq@gmail.com> wrote:
Gah. That came out all wrong. I mean: given that $b=$a++ + $a++; is undefined, that is that an
Cheers, -- |
From @rurban2010/7/12 David Golden <xdaveg@gmail.com>:
That's not the point. perl -MO=Concise,-exec -e'$a and "cmp" eq "cc"' can be optimized to gvsv is just checking magic and doing the sideeffect, and there would So the question if we should assert for less magic is bogus, as gvsv
Not entirely. if ($a && 0) { ... } This would nullify a lot of ops if the {} block is large, where we Bad that the optimizer module needs some core support. op_seq is gone. |
From @iabynOn Mon, Jul 12, 2010 at 06:06:15PM +0200, Reini Urban wrote:
Huh? gvsv doesn't call magic. Also, just as a data point, note that pp_concat *explicitly* calls get if (left == right) -- |
From @rurban2010/7/12 Dave Mitchell <davem@iabyn.com>:
Oops, right.
|
From @ikegamiOn Mon, Jul 12, 2010 at 7:07 AM, David Golden <xdaveg@gmail.com> wrote:
I realise you knew it would break. What I was pointing out is that it breaks |
From @demerphqOn 12 July 2010 20:30, Dave Mitchell <davem@iabyn.com> wrote:
Id like to argue that this was misguided. I dont think we guarantee cheers, -- |
From ben@morrow.me.ukQuoth demerphq@gmail.com (demerphq):
Perl doesn't have undefined behaviour. No matter what weasel words Ben |
From @demerphqOn 13 July 2010 13:43, Ben Morrow <ben@morrow.me.uk> wrote:
Just so everyone can conveniently see: From perldoc perlop: Auto-increment and Auto-decrement "++" and "--" work as in C. That is, if placed before a $i = 0; $j = 0; Note that just as in C, Perl doesn’t define when the variable $i = $i ++; Perl will not guarantee what the result of the above statements is. If we are going to say that these statements are well defined then we Ill just say that in this case I would much prefer we dont change the cheers, -- |
From @avarOn Tue, Jul 13, 2010 at 11:43, Ben Morrow <ben@morrow.me.uk> wrote:
Undefined doesn't mean that the implementation doesn't act Of course we can't liberally change things that are documented to be |
From @nwc10On Tue, Jul 13, 2010 at 01:11:11PM +0000, Ævar Arnfjörð Bjarmason wrote:
http://www.lysator.liu.se/c/c-faq/c-5.html#5-23 Briefly: implementation-defined means that an implementation must choose I suspect that all of our documentation should say "unspecified" rather than
But whatever we call it, that's the key problem. There is only one Nicholas Clark |
From @demerphqOn 15 July 2010 01:12, Dave Mitchell <davem@iabyn.com> wrote:
I'm not sure what your point is? Simply because Hugo wrote/pushed a The documentation for ++ is pretty clear. If the concatenation of a tied variable that mutates is well Thus the onus is not on me to show why this is unspecified, as the I have to say that I'm struggling to see why what you just posted cheers, |
From @iabynOn Thu, Jul 15, 2010 at 10:31:51AM +0200, demerphq wrote:
I wasn't making a point, I was just providing information.
Just to make it clear, I didn't post that patch to prove a point one way However, for my opinions for the topic in hand... as regards tiedness, there are actually two orthogonal issues of On the other hand, it may not be documented or specified, but I think most Finally, my feeling is that any 'no magic;' scopes aren't really viable in -- |
From @rurban2010/7/15 demerphq <demerphq@gmail.com>:
I'm also with Yves here. $a . $a must evaluate $a twice, and not only once, In our case we need: This is would you expect from reading $a . $a. But we certainly need a testcase for this mg_get sideeffect.
|
From @iabynOn Thu, Jul 15, 2010 at 01:28:58PM +0200, Reini Urban wrote:
No I think you're reading it the wrong way. Hugo's patch ensures that $a -- |
From @rurban2010/7/15 Dave Mitchell <davem@iabyn.com>:
Great! I see it now in the two gmagic tests. I still have no time to finish my simple optimizer rule for Nicks |
From @rurban#! perl
=head1 DESCRIPTION
optimize (and ... NO) to null if no gvsv/padsv, else (dor $x) or do some SvGETMAGIC
(and NO) is always false, but all SVs must call their mg_get for all SVs before not
=head1 EXAMPLE1 gvsv
$ perl -MO=Concise,-exec -e'if ($a and "x" eq "y") { print $s;}'
1 <0> enter
2 <;> nextstate(main 3 -e:1) v:{
3 <$> gvsv(*a) s
4 <|> and(other->5) sK/1
5 <$> const(SPECIAL sv_no) s
6 <|> and(other->7) vK/1
7 <0> pushmark s
8 <$> gvsv(*s) s
9 <@> print vK
a <@> leave[1 ref] vKP/REFC
can be optimized to
1 <0> enter
2 <;> nextstate(main 3 -e:1) v:{
3 <$> gvsv(*a) s
4 <1> dor vK/1
a <@> leave[1 ref] vKP/REFC
=head1 EXAMPLE2 padsv
$ perl -MO=Concise,-exec -e'my $a; if ($a and "x" eq "y") { print $s;}'
1 <0> enter
2 <;> nextstate(main 1 -e:1) v:{
3 <0> padsv[$a:1,4] vM/LVINTRO
...
4 <;> nextstate(main 4 -e:1) v:{
5 <0> padsv[$a:1,4] s
6 <|> and(other->7) sK/1
7 <$> const[SPECIAL sv_no] s
8 <|> and(other->9) vK/1
9 <0> pushmark s
a <#> gvsv[*s] s
b <@> print vK
c <@> leave[1 ref] vKP/REFC
can be optimized to
1 <0> enter
2 <;> nextstate(main 1 -e:1) v:{
3 <0> padsv[$a:1,3] vM/LVINTRO
...
4 <;> nextstate(main 2 -e:1) v:{
5 <$> padsv([$a:1,3) s
6 <1> dor vK/1
7 <@> leave[1 ref] vKP/REFC
=head1 EXAMPLE3 ok
$ perl -MO=Concise,-exec -e'if ("x" eq "y" and $a) { print $s;}'
is already optimized to
1 <0> enter
2 <;> nextstate(main 3 -e:1) v:{
3 <@> leave[1 ref] vKP/REFC
=cut
use optimizer;
use B::Generate;
use optimizer callback => sub {
my $o = shift;
if (($o->name eq 'gvsv' or $o->name eq 'padsv')
and ${$o->next} and {$o->next}->name eq 'and'
and ${$o->next->next} and {$o->next->next}->name eq 'const'
and {$o->next->next}->sv == B::sv_no
)
{
# change o->next to dor and nullify the rest
}
};
|
From @demerphqOn 15 July 2010 13:24, Dave Mitchell <davem@iabyn.com> wrote:
My apologies for making the incorrect inference. Clarification understood.
Yes, agreed. i dont have any issue with calling tie twice, and would
What does this mean exactly.
Hmm. I don't know that I would. If we want this to be the case then
Ok, thanks. What is the main problem as you see it? Yves -- |
From @hvdsDave Mitchell <davem@iabyn.com> wrote: This agrees with my thinking - I do not care a jot about the order of Hugo |
From @janduboisOn Fri, 16 Jul 2010, hv@crypt.org wrote:
Could you explain _why_ you would care about invoking magic twice, but And could you also explain why it makes sense that $a.$a has to invoke Cheers, |
From @nwc10On Tue, Jul 20, 2010 at 01:12:29PM -0700, Jan Dubois wrote:
On this part, I believe that I agree with Hugo, because my answer is: I read $a . $a as equivalent to $x . $y, where it happens that $x and $y Whereas $a x 2 has $a *written* once by the programmer, so as there is only Basically, I view tie as active data, with an implied contract that it will Hence I don't view $a . $a and $a x 2 as identical and interchangeable - if (Overload, on the other hand, I view as should-be-idempotent. I see its role Nicholas Clark |
From @janduboisOn Tue, 20 Jul 2010, Nicholas Clark wrote:
In that case I think you'll find plenty of places where it is accessed flags = SvFLAGS(sv); It is easy to guarantee that each tied variable is fetched at least once I prefer to view this as an inefficiency, not as a bug, because I think Cheers, |
From @janduboisEirik Berg Hanssen wrote:
Yes, but f() may have side-effects, whereas $a shouldn't have any (IMO). As I wrote earlier to Nicholas, FETCH may be called more than you expect sub foo::TIESCALAR { bless \my $x => "foo" } tie $a, "foo"; print "NV\n"; $a = 1.; NV So FETCH is called twice when $a is a floating point number and only But even assuming there is a canonical number of times FETCH should perlop.pod implies that there should be 2 accesses: "++" and "--" work as in C. That is, if placed before a variable, they There is nothing there stating that the returned value can be re-used to Cheers, |
From ebhanssen@cpan.orgOn Tue, Jul 20, 2010 at 10:12 PM, Jan Dubois <jand@activestate.com> wrote:
For the same reason that f().f() calls &f twice, while f() x 2 will only Eirik |
From @AbigailOn Tue, Jul 13, 2010 at 06:55:33PM +0100, Ben Morrow wrote:
Actually, I added the explicite statement about undefinedness in the $i = $j = 1; Abigail |
From @AbigailOn Tue, Jul 20, 2010 at 02:41:34PM -0700, Jan Dubois wrote:
If FETCH is to be side-effect free, many of the interesting cases for using Abigail |
From @rgarciaOn 29 July 2010 15:42, Abigail <abigail@abigail.be> wrote:
At this point, someone usually mentions monads. |
From @ap* Eric Brine <ikegami@adaelis.com> [2010-07-12 06:55]:
Why? As far as I can tell, the compiler would statically determine It would also statically determine that an `or !1` clause in that That would leave the code looking like my $x = f(); when `DEBUG` is false. Which seems 100% on the mark to me. Am I missing something in your objection? Regards, |
From @ikegamiIndeed. I made a booboo when I wrote that. |
From @nthykierHi, Attached is a prototype branch that enables some branch elimination It can kill branches in if-else cases. Examples: $ ./perl -Ilib -MO=Deparse \ $ ./perl -Ilib -MO=Deparse \ The patch adds two op_private flags for OP_AND and OP_OR, which However, the newLOGOP does not use this flag itself to elimite dead $ ./perl -Ilib -MO=Deparse -e 'if ($a && "Pie" ne "Good") {print}' The only problem here is that folding the OP_AND/OP_OR expressions $a && "Pie" ne "Good" => $a Before newCONP has a change to see it (and thus lose the ability to ~Niels |
From @nthykierop-prune-cond-branch.diffdiff --git a/op.c b/op.c
index d5323a0..a5675d3 100644
--- a/op.c
+++ b/op.c
@@ -5976,6 +5976,39 @@ S_new_logop(pTHX_ I32 type, I32 flags, OP** firstp, OP** otherp)
first->op_next = (OP*)logop;
first->op_sibling = other;
+ if (type == OP_AND || type == OP_OR)
+ {
+ /* Look for stuff like: a() and (b() or 1)
+
+ The truth value of this expression is entirely decidable,
+ but we cannot eliminate the expression (a() and b() might
+ have side effects).
+ */
+ U8 rhs_value = 0;
+ cstop = search_const(other);
+ /* Check if the RHS is known to be FALSE */
+ if (cstop)
+ {
+ if (SvTRUE(((SVOP*)cstop)->op_sv))
+ rhs_value = OPpLOGOP_CONST_TRUE;
+ else
+ rhs_value = OPpLOGOP_CONST_FALSE;
+ }
+ if ((other->op_type == OP_AND || other->op_type == OP_OR)
+ && (other->op_private & OPpLOGOP_CONST_MASK))
+ {
+ rhs_value = other->op_private & OPpLOGOP_CONST_MASK;
+ }
+
+ if (rhs_value)
+ {
+ if (type == OP_AND && rhs_value == OPpLOGOP_CONST_FALSE)
+ logop->op_private |= rhs_value;
+ if (type == OP_OR && rhs_value == OPpLOGOP_CONST_TRUE)
+ logop->op_private |= rhs_value;
+ }
+ }
+
CHECKOP(type,logop);
o = newUNOP(prepend_not ? OP_NOT : OP_NULL, 0, (OP*)logop);
@@ -6043,6 +6076,47 @@ Perl_newCONDOP(pTHX_ I32 flags, OP *first, OP *trueop, OP *falseop)
live->op_private |= OPpCONST_FOLDED;
return live;
}
+ o = first;
+ while (o && o->op_type == OP_NULL && (o->op_flags & OPf_KIDS))
+ o = cUNOPo->op_first;
+
+ if (o && (o->op_type == OP_AND || o->op_type == OP_OR)
+ && (o->op_private & OPpLOGOP_CONST_MASK)) {
+ const bool left = (o->op_private & OPpLOGOP_CONST_MASK)
+ == OPpLOGOP_CONST_TRUE;
+ OP *live = left ? trueop : falseop;
+ OP *const dead = left ? falseop : trueop;
+ /* Rewrite:
+
+ if (a() and <false>) {<dead>} else { b() } =>
+ (a() and <false>) or { b() }
+
+ if (a() or <true>) { b() } else {<dead>} =>
+ (a() or <true>) and { b() }
+
+ (NB: <false> and <true> are not limited to simple OP_CONST)
+
+ */
+ OPCODE combine_op = o->op_type == OP_AND ? OP_OR : OP_AND;
+
+ if (PL_madskills) {
+ /* This is all dead code when PERL_MAD is not defined. */
+ live = newUNOP(OP_NULL, 0, live);
+ op_getmad(first, live, 'C');
+ op_getmad(dead, live, left ? 'e' : 't');
+ } else {
+ op_free(dead);
+ }
+ if (live->op_type == OP_LEAVE)
+ live = newUNOP(OP_NULL, OPf_SPECIAL, live);
+ else if (live->op_type == OP_MATCH || live->op_type == OP_SUBST
+ || live->op_type == OP_TRANS || live->op_type == OP_TRANSR)
+ /* Mark the op as being unbindable with =~ */
+ live->op_flags |= OPf_SPECIAL;
+ else if (live->op_type == OP_CONST)
+ live->op_private |= OPpCONST_FOLDED;
+ return newLOGOP(combine_op, 0, first, live);
+ }
NewOp(1101, logop, 1, LOGOP);
logop->op_type = OP_COND_EXPR;
logop->op_ppaddr = PL_ppaddr[OP_COND_EXPR];
diff --git a/op.h b/op.h
index 5d1a771..b055965 100644
--- a/op.h
+++ b/op.h
@@ -176,6 +176,12 @@ Deprecated. Use C<GIMME_V> instead.
#define OPpASSIGN_BACKWARDS 64 /* Left & right switched. */
#define OPpASSIGN_CV_TO_GV 128 /* Possible optimisation for constants. */
+
+/* Private for OP_AND and OP_OR */
+#define OPpLOGOP_CONST_TRUE 2 /* Result is always true, but op has side-effect*/
+#define OPpLOGOP_CONST_FALSE 4 /* Result is always false, but op has side-effect*/
+#define OPpLOGOP_CONST_MASK (2|4) /* Mask is always true, but op has side-effect*/
+
/* Private for OP_MATCH and OP_SUBST{,CONT} */
#define OPpRUNTIME 64 /* Pattern coming in on the stack */
|
Are we planning on applying this? If not should it be closed? |
On Tue, Feb 04, 2020 at 02:20:43PM -0800, Todd Rinaldo wrote:
Are we planning on applying this? If not should it be closed?
I doubt that it would apply cleanly any more.
But in any case, it just removes a bit of dead code from the optree; it
doesn't optimise runtime execution. Since also the whole area of boolean
and logical op optimisation is a bit complex, we'd run a real risk of
inadvertently pessimising something too.
So I don't think the gain is worth the effort to revive and scrutinise
this patch.
…--
"Do not dabble in paradox, Edward, it puts you in danger of fortuitous wit."
-- Lady Croom, "Arcadia"
|
Migrated from rt.perl.org#76438 (status was 'open')
Searchable as RT76438$
The text was updated successfully, but these errors were encountered: