On 21/01/2014 23:37, Jonas Maebe wrote:
On 22 Jan 2014, at 00:06, Martin Frb wrote:
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?
The main issue with the peephole optimizer isn't its speed, but its
readability, maintainability and extensibility. So unless this significantly
improves either (or at the very least doesn't hurt them), I wouldn't modify it.
Looking at the below example, I would say it does not hurt, IMHO it is
slightly more readable. Give that currently each IF has several lines
connected with AND.
Reducing the AND allows to see where they really differ.
But, yes it is at bes a minor readability improvment. Of course
readability is dependent on the reader.
As for speed, the differenc in the overall compile time will probably be
tiny. Not even sure if noticeable. (I may test it, but maybe not)
I understand, that this is the change, someone with long standing commit
rights of its own may do. But it simply is not adding enough value to
really spent the time on reviewing it, if it is not your own work. So if
it is this then that is ok.
NEW:
If GetNextInstruction(p, hp1) then Begin
if ffo1 then
else
if ffo2 then
;
if ffo3 then
end;
OLD:
if ffo1 and
GetNextInstruction(p, hp1) then
else
if ffo2 and
GetNextInstruction(p, hp1) then
;
if ffo3 and
GetNextInstruction(p, hp1) then
_______________________________________________
fpc-devel maillist - [email protected]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel
