Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.99k stars 557 forks source link

PL_rpeepp doesn't pass the prevailing COP #18775

Open leonerd opened 3 years ago

leonerd commented 3 years ago

Modules which wish to install custom recursive peephole optimizers will wrap their hook function around PL_rpeepp.

As it would be rude to apply optimizations globally, any polite optimizing module should inspect the prevailing lexical hints-hash before applying their changes. An example of such can be seen in my Faster::Maths module at:

https://metacpan.org/source/PEVANS/Faster-Maths-0.02/lib%2FFaster%2FMaths.xs#L253-257

    if(o->op_type == OP_NEXTSTATE) {
      SV *sv = cop_hints_fetch_pvs(cCOPo, "Faster::Maths/faster", 0);
      enabled = sv && sv != &PL_sv_placeholder && SvTRUE(sv);
      goto next_o;
    }

This only works if the first op we encounter is an OP_NEXTSTATE, which while true of function bodies isn't always true in general. Perl core will take care to invoke PL_rpeepp for the main op_next chain of every function, but also in lots of special cases, such as the op_other of LOGOPs like OP_AND. In such a case, the first op we see on the righthand side of an and operator is not an OP_NEXTSTATE. This means that we don't see the prevailing lexical hints (via the COP) when optimizing this branch.

Faster::Maths has to work around this by manually recursing into these cases, passing in values obtained from the prevailing COP itself:

https://metacpan.org/source/PEVANS/Faster-Maths-0.02/lib%2FFaster%2FMaths.xs#L290

      case OP_OR:
      case OP_AND:
      case OP_DOR:
      ...
        if(cLOGOPo->op_other && cLOGOPo->op_other->op_type != OP_NEXTSTATE)
          rpeep_for_fastermaths(aTHX_ cLOGOPo->op_other, enabled);
        goto next_o;
leonerd commented 3 years ago

To solve this I could imagine two possibilities.

Both involve keeping track of the prevailing COP in effect for deferred recursive calls, and somehow ensuring that the invoked function is aware of it. For toplevel functions and code this would be the first op itself because that will be a COP, but for anything else (e.g. these LOGOP op_other branches) we can instead pass the prevailing COP we encountered at the time.

1) Change the signature of the PL_rpeepp function to pass a COP as well.

2) Keep the signature but ensure that PL_curcop itself is set to the correct value before invoking the function.

Option 1 is conceptually neater, but more disruptive in causing a code-level breakage of existing code. Option 2 avoids sourcecode breakage, but then code written presuming the new style will silently give the wrong results on older perls, rather than failing to compile as it would under option 1.

iabyn commented 3 years ago

On Sat, May 08, 2021 at 09:06:17AM -0700, Paul Evans wrote:

To solve this I could imagine two possibilities.

Both involve keeping track of the prevailing COP in effect for deferred recursive calls, and somehow ensuring that the invoked function is aware of it. For toplevel functions and code this would be the first op itself because that will be a COP, but for anything else (e.g. these LOGOP op_other branches) we can instead pass the prevailing COP we encountered at the time.

1) Change the signature of the PL_rpeepp function to pass a COP as well.

2) Keep the signature but ensure that PL_curcop itself is set to the correct value before invoking the function.

Option 1 is conceptually neater, but more disruptive in causing a code-level breakage of existing code. Option 2 avoids sourcecode breakage, but then code written presuming the new style will silently give the wrong results on older perls, rather than failing to compile as it would under option 1.

In general, isn't PL_curcop already localised to the current COP when calling rpeep?

-- Dave's first rule of Opera: If something needs saying, say it: don't warble it.