On Fri, Jan 24, 2014 at 03:31:24PM -0800, Matthew Horsfall wrote: > First pass at optimisation for 'my $x; my $y;' -> 'my ($x, $y)'. > > This changes padop -> nextate -> padop -> nexstate into a padrange -> nextstate, > which allows further padrange optimisations to occur. Cool, thanks! It looks okay to me. One thing: I'd like more detailed comments at the top of the code that describes exactly what sequence of ops it matches, and how it transforms them, e.g. /* Optimise 'my $x; my $y;' into 'my ($x, $y);' * This latter form is then suitable for conversion into padrange * later on. Convert: * * nextstate1 -> padop1 -> nextstate2 -> padop2 -> nextstate3 * * into: * * nextstate1 -> listop -> nextstate3 * / \ * pushmark -> padop1 -> padop2 */ Also, you don't appear to op_free() nextstate2. > There's two differences here: > > The ranges of the padops [$x:1,4 ...] vs [$x1,2 ...], though I'm not > sure that matters. Shouldn't do. That's just the range of cops for which the lexical is in scope, so that for example in my $x; eval '$x'; my $x; eval '$x' each eval sees a different $x. > The flag 'M' (OPf_MOD) on vanilla blead's padrange doesn't show up > on my patched version. > I'm not sure if I should be setting this here, how to detect when to > set it, etc... The M on the vanilla padrange is inherited from the M on the pushmark, when that latter op is converted into the padrange. I think it's harmless (neither pp_pushmark nor pp_padrange use it), but it might be worth seeing where it gets set on a pushmark and under which circumstances, and duplicate that behaviour if possible. -- "I do not resent criticism, even when, for the sake of emphasis, it parts for the time with reality". -- Winston Churchill, House of Commons, 22nd Jan 1941.Thread Previous | Thread Next