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.

Not the right person to give a qualified answer to your question, but bare record parameters in method signatures are kind of a problem throughout various parts of the standard FPC and Lazarus libraries, I've noticed.

It almost makes me think the either the compiler should perform the kind of optimization it does with const/var/constref implicitly no matter what, or it should just not allow you to specify methods without 's the point in having a way of doing something that is guaranteed to always give worse results than all of the other ways? It seems pointless.

At the very least, perhaps there should be a section in the documentation that talks about how passing records (and objects) actually works regarding optimization with the different keywords, just to make sure that people are aware?

Op 2019-01-26 om 20:51 schreef Ben Grasset:

I think it is rather the other way around. TRect was first I think, but then I noticed I also needed TPoint.

Anyway, a patch would be welcome.

Am So., 27. Jan. 2019, 00:01 hat John Doe
geschrieben:

No, the default by-value passing has its uses. With "const" and "constref" one can't modify the record and one needs to make a copy to do so and with "var" one always changes the passed in reference so one needs a copy if one does not want that.
Changing this behavior would not only be a backwards incompatibility, but would also violate the ABI which describes how records shall be passed. And if it says copy then it shall get copy ("var" and "constref" are essentially pointers and "const" puts the decision to the compiler and is not part of the system ABI anyway).

Regards,
Sven

On Sat, Jan 26, 2019 at 7:06 PM Marco van de Voort <

wrote: Anyway, a patch would be welcome.

Ok, I'll work one up that merges properly against the current trunk versions. Thanks for the quick response also, I was actually not sure who had written those files... didn't look at the license header. ;)

On Sat, Jan 26, 2019 at 7:44 PM Sven Barth via fpc-devel <

On second thought, I definitely agree with you that changing the basic behavior of the compiler in that regard sounds like an idea that is not remotely possible due to how it would very likely break large amounts of existing code. That was a dumb suggestion on my part.

However, I'm also quite simply unaware of any specific realistic scenario where I would *ever* specify a un-prefixed record in a method (in my own projects for example I actually haven't used anything but "var" and "constref" for years, not even just normal const as it doesn't always "do the trick" I've found), and I do think that on a more broad scope it is highly unlikely that most existing record methods written in an "unprefixed" fashion were done that way for any particular or intentional reason.

Like I suggested earlier, it seems like many people are simply unaware that FPC's behaviour in that area is completely predictable and fully *unrelated* to any optimization settings: it just will not, ever, under any circumstances, pass a record or object without var or const or constref by reference.

The only guess I have as to *why* they don't know (or maybe even actually think it's "not that important") is that they (they being, again, the people who write code like this) are people who originally learned Pascal with Turbo Pascal where that sort of optimization was far less relevant and may not have even done anything (as you couldn't even return records or objects from functions in TP, I don't think.)

Regardless, units like graphmath.pp
<https://github.com/graemeg/lazarus/blob/upstream/lcl/graphmath.pp> from the LCL (which I know is slightly out of scope here) are quite simply painful to read. Look at those operator overloads for TFloatPoint! No const, no inline. In fact only two methods in the entire unit are marked inline, despite how it's full of very short math functions that should very clearly be marked as such.

I think there's a LOT of "free" performance-increase potential that everyone would benefit from lurking in many places, that doesn't require anything new from the compiler but just for things like that to be remedied.

[...] Regardless, units like graphmath.pp <https://github.com/graemeg/lazarus/blob/upstream/lcl/graphmath.pp>

I added some const and inline.

Mattias

Am So., 27. Jan. 2019, 02:53 hat John Doe
geschrieben:

As I suggested in my answer one situation is interfacing with non-Pascal code.
Also there is another side effect that at least needs to be kept in mind: if you have multithreaded code having one of the by-ref parameter types across thread boundaries and one thread changing a value (e.g. because it was passed as "var" there and some other function works with "constref") will result in the values all threads see to be changed. This might be quite a constructed scenario, but we do explicitly warn in the documentation of a similar situation. See the second remark here: https://freepascal.org/docs-html/current/ref/refsu67.html#x179-20100014.4.4 (the first remark there is also somewhat important as "const" alone provides no guarantee of being passed by-ref, only that the compiler won't allow you to write assignments with the parameter or its fields on the left side).

Regards,
Sven

Op 2019-01-27 om 01:55 schreef Ben Grasset:

Thanks, I saw some of the new constructors of tpoint/tsize are also not const. I wonder though that maybe makes passing properties etc difficult?

Just thinking out loud (probably a bad idea) but:

Would it perhaps be possible to treat this as a type of optimization however? Which is to say, if the called routine makes no changes to the passed record, then it should not matter whether compiler implicitly const passes the parameter or copies it? (If the routine *does* change the passed record then it does the normal thing.) Of course I have no idea whether the dataflow analysis I imagine needed to support this is whatsoever plausible to be done without unacceptable compromises (And even then, I'd guess it's not something there'd be sufficient appetite for even if in theory it might be possible...?)

Am So., 27. Jan. 2019, 14:12 hat Walter Prins
geschrieben:

We currently have no function specific adjustments of the calling convention, so this would be rather involved to implement (I know that e.g. the Visual Studio compiler does that).
Also keep in mind that a unit does not know how it is used. With Lazarus a package is compiled once for multiple projects where one could be a self contained application and the other a library that has such a function in its export section (constructed example, I know, but those things need to be kept in mind).

Regards,
Sven

Walter Prins wrote

Would it perhaps be possible to treat this as a type of optimization however?  Which is to say, if the called routine makes no changes to the passed record, then it should not matter whether compiler implicitly const passes the parameter or copies it?

It does, because another routine called by this routine may modify the original record.

E.g.

var
   r: record a: longint end;;
procedure modify;
begin
   r.a:=2;
end;
procedure test(r: trecord);
begin
   modify;
   if r.a1 then
     writeln('error');
end;
begin
   r.a:=1;
   test(r);
end.

Jonas

It is possible.
Note that changing the signature needs a wrapper when creating a proc value:
;

Mattias

On Sun, Jan 27, 2019 at 8:00 AM Marco van de Voort <

wrote: I wonder though that maybe makes passing properties etc difficult?

As far I know "const" has no tangible effect on how properties can be passed. Here's a little example to demonstrate:

program Test;
{$mode ObjFPC}{$H+}{$J-}
{$modeswitch AdvancedRecords}
type
  TRecA = record
  public
    A, B: SizeInt;
    constructor Create(const IA, IB: SizeInt);
    constructor Create(const ARec: TRecA);
  end;
  TRecB = record
  private
    FRecA: TRecA;
  public
    constructor Create(const ARec: TRecA);
    property RecA: TRecA read FRecA;
  end;
  TObjectB = object
  private
    FRecA: TRecA;
  public
    constructor Create(const ARec: TRecA);
    property RecA: TRecA read FRecA;
  end;
  TClassB = class
  private
    FRecA: TRecA;
  public
    constructor Create(const ARec: TRecA);
    property RecA: TRecA read FRecA;
  end;
  constructor TRecA.Create(const IA, IB: SizeInt);
  begin
    A := IA;
    B := IB;
  end;
  constructor TRecA.Create(const ARec: TRecA);
  begin
    A := ARec.A;
    B := ARec.B;
  end;
  constructor TRecB.Create(const ARec: TRecA);
  begin
    FRecA := ARec;
  end;
  constructor TObjectB.Create(const ARec: TRecA);
  begin
    FRecA := ARec;
  end;
  constructor TClassB.Create(const ARec: TRecA);
  begin
    FRecA := ARec;
  end;
var
  R: TRecB;
  O: TObjectB;
  C: TClassB;
begin
  R := TRecB.Create(TRecA.Create(5, 10));
  WriteLn(R.RecA.A, ' ', R.RecA.B);
  O.Create(R.RecA);
  WriteLn(O.RecA.A, ' ', O.RecA.B);
  C := TClassB.Create(O.RecA);
  WriteLn(C.RecA.A, ' ', C.RecA.B);
  R := TRecB.Create(C.RecA); //back to where we started...
  WriteLn(R.RecA.A, ' ', R.RecA.B);
  C.Free();
end.

Compiles without issues and works as intended. Personally though something I often do is use static "class functions" instead of constructors for advanced records, as they have the benefit of being able to be marked inline as well as not having the "at least one parameter" restriction. From an end-user perspective they're syntactically identical to how you'd call an actual constructor, also, so there's no worries in that area either.

The program above changed in that fashion, where for example the TRecA record is written like this:

type
  TRecA = record
  public
    A, B: SizeInt;
    class function Create(const IA, IB: SizeInt): TRecA; static; inline;
    class function Create(const ARec: TRecA): TRecA; static; inline;
  end;
  class function TRecA.Create(const IA, IB: SizeInt): TRec;
  begin
    with Result do begin
      A := IA;
      B := IB;
    end;
  end;
  class function TRecA.Create(const ARec: TRecA): TRec;
  begin
    with Result do begin
      A := ARec.A;
      B := ARec.B;
    end;
  end;

generates assembly at -O3 for the main method that is actually two lines *shorter* than the assembly for the constructor version, while also of course avoiding three entire function calls completely via inlining (both TRecA constructors plus the one for TRecB.)

So I could change things in that regard too for TSize/TPoint/TRect while I'm at it, unless anyone has any objections.