[fpc-devel] RemoveCurrentP Refactor

[J. Gareth Moreton]

Hi everyone,

I have made note of a preference to use RemoveCurrentP instead of 
removing the current instruction from the assembler list due to an 
optimisation that warrants its removal.  This is fair enough because 
there are a number of steps that have to be taken and the procedure is 
largely the same each time, and subtle errors or memory leaks arising if 
a step is missed.  The full process is rather expensive, however, with 
calls to both UpdateUsedRegs and GetNextInstruction, and there are cases 
where it is a waste to call them (for the former, when removing multiple 
adjacent instructions; for the latter, when the next instruction is 
already known).

https://bugs.freepascal.org/view.php?id=36797

This patch introduces a new version of RemoveCurrentP that takes a 
second parameter, but doesn't return a result - procedure 
RemoveCurrentP(var p: tai; const hp1: tai) - this is for situations 
where the next instruction is already known... or more simply, it simply 
sets p to hp1 once the original p is removed and freed, instead of 
calling GetNextInstruction.  Additionally, a number of calls to 
RemoveCurrentP where it's clear that hp1 came from GetNextInstruction 
have been changed to the new version. Note the old one still exists for 
when the next instruction is not known.

This patch is a pure refactor - it doesn't add any new features. As 
such, the prerequisites for its acceptance are that any binaries 
generated by the compilers do NOT change, and that the compiler runs 
slightly faster (which should be the case since the number of calls to 
GetNextInstruction have been reduced).

I like to think this draws a balance between performance and 
maintainability.

Gareth aka. Kit


-- 
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus