[fpc-devel] optimizing the peephole optimizer?

[Martin Frb]

On 22/01/2014 21:22, Florian Klämpfl wrote:
> Am 22.01.2014 00:06, schrieb Martin Frb:
>> On 21/01/2014 21:20, Florian Klämpfl wrote:
>>> Am 19.01.2014 18:01, schrieb Martin:
>>>> In each of them GetNextInstruction(p, hp1) could be executed.
>>>>
>>>> If I counted correct, it can be called up to 6 times for each a_mov.
>>>> On the other hand:
>>>> - no code in this case block, is ever executed, if it does not succeed.
>>>> - It will always be executed at least once
>>>>
>>>> So it would probably be better to have this once at the start of the
>>>> A_MOV block.
>>>>
>>>>
>>>> If that makes sense, I can check other blocks for similar issues.
>>> I guess the idea is to catch typical code sequences in one go, see also
>>> the comment in 1243:
>>> { Next instruction is also a MOV ? }
>> So then a patch would be accepted to move the check for
>>     GetNextInstruction(p, hp1)
>> to the start, and have one single such condition, in one IF block, that
>> will contain all the others?
> Might p not be changed by if blocks before?
>

See my other mail, statement can be inserted/deleted, each block that 
makes modifications, must restore the state. That is it must set hp1 to 
the correct value again. Of course that could be nil, and that would 
mean another check on some of the other IF.

But yes, in terms of extending the code later it is a step back: Today 
all code would be changed to test GetNextInstruction again, if a change 
was made.
But if someone adds code in future without doing this test, then it breaks.

This is a risk that you/ the fpc team have to decide, if it was 
acceptable or not.

I could play with the code, and see how much of the IFs would be 
affected, and provide an actual patch, so it could be compared side by 
side (which would probably be better, than the current theoretical 
discussion (even if the patch was discarded it be a good exercise to get 
to know the code better)

I will defer that though, for
a) got other stuff
b) currently only have one checkout, and other patches pending. That 
makes it harder to keep track of which change is what.