[fpc-devel] Looking for clarification on what I think are obviously missing "const" prefixes for parameters in the methods of TRect (the typshrdh.inc one)

[Ben Grasset]

TSize and TPoint, the other two advanced records defined there, have (well,
mostly, they're also missing a couple) all record parameters in their
method signatures declared "const", for what is presumably performance /
efficiency reasons.

That is to say, record parameters specified without "var", "const", or
"constref" have copies made of them 100% of the time regardless of any
optimization settings, which results in quite a bit more assembly code
being generated and of course slower runtime performance.

TRect on the other hand has literally no "const" of any kind for anything
except the array called Points in one of the Union method overloads.

It seems to me like the original author of this unit probably *intended* to
"constify" the methods for TRec the same way they did for TSize and TPoint,
but just never got around to it / forgot / something along those lines.

I did a little test wherein I extracted the interface and implementation
for TRect into a standalone unit, and compared the size of the generated
assembly file both as-is and with "const" added in what I believe are the
appropriate places.

Compiled with native Win64 trunk FPC at -O3, the no-const version was 28KB,
while the version with const was 23KB, confirming my expectation that
objectively slower / longer assembly is generated without const.

As an example, here's the no-const assembly for the "equal" operator
overload:

.section .text.n_unit1$_$tmyrect_$__$$_equal$tmyrect$tmyrect$$boolean,"x"
.balign 16,0x90
.globl UNIT1$_$TMYRECT_$__$$_equal$TMYRECT$TMYRECT$$BOOLEAN
UNIT1$_$TMYRECT_$__$$_equal$TMYRECT$TMYRECT$$BOOLEAN:
.Lc16:
.seh_proc UNIT1$_$TMYRECT_$__$$_equal$TMYRECT$TMYRECT$$BOOLEAN
leaq -40(%rsp),%rsp
.Lc18:
.seh_stackalloc 40
.seh_endprologue
movq (%rcx),%rax
movq %rax,(%rsp)
movq 8(%rcx),%rax
movq %rax,8(%rsp)
movq (%rdx),%rax
movq %rax,16(%rsp)
movq 8(%rdx),%rax
movq %rax,24(%rsp)
movq %rsp,%rax
leaq 16(%rsp),%rdx
movl (%rax),%ecx
cmpl (%rdx),%ecx
jne .Lj16
movl 8(%rax),%ecx
cmpl 8(%rdx),%ecx
jne .Lj16
movl 4(%rax),%ecx
cmpl 4(%rdx),%ecx
jne .Lj16
movl 12(%rax),%eax
cmpl 12(%rdx),%eax
jne .Lj16
movb $1,%al
jmp .Lj20
.Lj16:
xorb %al,%al
.Lj20:
leaq 40(%rsp),%rsp
ret
.seh_endproc
.Lc17:

With const however, it looks like this:

.section .text.n_unit1$_$tmyrect_$__$$_equal$tmyrect$tmyrect$$boolean,"x"
.balign 16,0x90
.globl UNIT1$_$TMYRECT_$__$$_equal$TMYRECT$TMYRECT$$BOOLEAN
UNIT1$_$TMYRECT_$__$$_equal$TMYRECT$TMYRECT$$BOOLEAN:
.Lc12:
movl (%rcx),%eax
cmpl (%rdx),%eax
jne .Lj18
movl 8(%rcx),%eax
cmpl 8(%rdx),%eax
jne .Lj18
movl 4(%rcx),%eax
cmpl 4(%rdx),%eax
jne .Lj18
movl 12(%rcx),%eax
cmpl 12(%rdx),%eax
jne .Lj18
movb $1,%al
ret
.Lj18:
xorb %al,%al
ret
.Lc13:

All of that said, I'd be quite happy to prepare and submit a patch that
corrects this, assuming it would be accepted and there is not some hidden
reason it is the way it is now, which is what I wanted to confirm here.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freepascal.org/pipermail/fpc-devel/attachments/20190126/d326a3ee/attachment.html>