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.



_______________________________________________
fpc-devel maillist  -  [email protected]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Reply via email to