View Issue Details

IDProjectCategoryView StatusLast Update
0035346FPCCompilerpublic2019-06-28 15:32
ReporterJ. Gareth MoretonAssigned ToSven Barth 
PrioritylowSeveritytweakReproducibilityN/A
Status resolvedResolutionfixed 
PlatformCross-platformOSMicrosoft WindowsOS Version10 Professional
Product Version3.3.1Product Buildr41851 
Target Version3.3.1Fixed in Version3.3.1 
Summary0035346: [Refactor] Inlining methods for TCStream and TEntryFile
DescriptionThe following two patches seek to inline a number of very small static (non-virtual) methods for TEntryFile and TCStream, two internal compiler classes, in order to provide a slight performance gain by removing a function call in a chain.
Steps To ReproduceApply patches and confirm the compiler does not behave any differently.
Additional InformationThe existing inline directives for getboolean/putboolean in TEntryFile were moved from the interface to the implementation section, since their presence in the implementation section, adjacent to the actual code, makes for easier maintenance and verification.

make cycle FPCOPT="-CriotR" passes without incident, as does a full "make distclean all install"
Tagscompiler, inline, patch, refactoring
Fixed in Revision42303,42304
FPCOldBugId0
FPCTarget-
Attached Files
  • inline_cstreams.patch (3,796 bytes)
    Index: compiler/cstreams.pas
    ===================================================================
    --- compiler/cstreams.pas	(revision 41924)
    +++ compiler/cstreams.pas	(working copy)
    @@ -67,8 +67,8 @@
     
       TCStream = class(TObject)
       private
    -    function GetPosition: Longint;
    -    procedure SetPosition(Pos: Longint);
    +    function GetPosition: Longint; {$ifdef USEINLINE}inline;{$endif}
    +    procedure SetPosition(Pos: Longint); {$ifdef USEINLINE}inline;{$endif}
         function GetSize: Longint;
       protected
         procedure SetSize(NewSize: Longint); virtual;
    @@ -79,22 +79,22 @@
         procedure ReadBuffer(var Buffer; Count: Longint);
         procedure WriteBuffer(const Buffer; Count: Longint);
         function CopyFrom(Source: TCStream; Count: Longint): Longint;
    -    function ReadComponent(Instance: TCComponent): TCComponent;
    -    function ReadComponentRes(Instance: TCComponent): TCComponent;
    -    procedure WriteComponent(Instance: TCComponent);
    -    procedure WriteComponentRes(const ResName: string; Instance: TCComponent);
    -    procedure WriteDescendent(Instance, Ancestor: TCComponent);
    -    procedure WriteDescendentRes(const ResName: string; Instance, Ancestor: TCComponent);
    -    procedure WriteResourceHeader(const ResName: string; {!!!:out} var FixupInfo: Integer);
    -    procedure FixupResourceHeader(FixupInfo: Integer);
    -    procedure ReadResHeader;
    -    function ReadByte : Byte;
    -    function ReadWord : Word;
    -    function ReadDWord : Cardinal;
    +    function ReadComponent(Instance: TCComponent): TCComponent; {$ifdef USEINLINE}inline;{$endif}
    +    function ReadComponentRes(Instance: TCComponent): TCComponent; {$ifdef USEINLINE}inline;{$endif}
    +    procedure WriteComponent(Instance: TCComponent); {$ifdef USEINLINE}inline;{$endif}
    +    procedure WriteComponentRes(const ResName: string; Instance: TCComponent); {$ifdef USEINLINE}inline;{$endif}
    +    procedure WriteDescendent(Instance, Ancestor: TCComponent); {$ifdef USEINLINE}inline;{$endif}
    +    procedure WriteDescendentRes(const ResName: string; Instance, Ancestor: TCComponent); {$ifdef USEINLINE}inline;{$endif}
    +    procedure WriteResourceHeader(const ResName: string; {!!!:out} var FixupInfo: Integer); {$ifdef USEINLINE}inline;{$endif}
    +    procedure FixupResourceHeader(FixupInfo: Integer); {$ifdef USEINLINE}inline;{$endif}
    +    procedure ReadResHeader; {$ifdef USEINLINE}inline;{$endif}
    +    function ReadByte : Byte; {$ifdef USEINLINE}inline;{$endif}
    +    function ReadWord : Word; {$ifdef USEINLINE}inline;{$endif}
    +    function ReadDWord : Cardinal; {$ifdef USEINLINE}inline;{$endif}
         function ReadAnsiString : AnsiString;
    -    procedure WriteByte(b : Byte);
    -    procedure WriteWord(w : Word);
    -    procedure WriteDWord(d : Cardinal);
    +    procedure WriteByte(b : Byte); {$ifdef USEINLINE}inline;{$endif}
    +    procedure WriteWord(w : Word); {$ifdef USEINLINE}inline;{$endif}
    +    procedure WriteDWord(d : Cardinal); {$ifdef USEINLINE}inline;{$endif}
         Procedure WriteAnsiString (S : AnsiString);
         property Position: Longint read GetPosition write SetPosition;
         property Size: Longint read GetSize write SetSize;
    @@ -153,11 +153,11 @@
         FMemory: Pointer;
         FSize, FPosition: Longint;
       protected
    -    procedure SetPointer(Ptr: Pointer; ASize: Longint);
    +    procedure SetPointer(Ptr: Pointer; ASize: Longint); {$ifdef USEINLINE}inline;{$endif}
       public
         function Read(var Buffer; Count: Longint): Longint; override;
         function Seek(Offset: Longint; Origin: Word): Longint; override;
    -    procedure SaveToStream(Stream: TCStream);
    +    procedure SaveToStream(Stream: TCStream); {$ifdef USEINLINE}inline;{$endif}
         procedure SaveToFile(const FileName: string);
         property Memory: Pointer read FMemory;
       end;
    
    inline_cstreams.patch (3,796 bytes)
  • inline_entfile.patch (4,414 bytes)
    Index: compiler/entfile.pas
    ===================================================================
    --- compiler/entfile.pas	(revision 41924)
    +++ compiler/entfile.pas	(working copy)
    @@ -249,7 +249,7 @@
         constructor create(const fn:string);
         destructor  destroy;override;
         function getversion:integer;
    -    procedure flush;
    +    procedure flush; {$ifdef USEINLINE}inline;{$endif}
         procedure closefile;virtual;
         procedure newentry;
         property position:longint read getposition write setposition;
    @@ -264,9 +264,9 @@
         procedure readdata(out b;len:integer);
         procedure skipdata(len:integer);
         function  readentry:byte;
    -    function  EndOfEntry:boolean;
    -    function  entrysize:longint;
    -    function  entryleft:longint;
    +    function  EndOfEntry:boolean; {$ifdef USEINLINE}inline;{$endif}
    +    function  entrysize:longint; {$ifdef USEINLINE}inline;{$endif}
    +    function  entryleft:longint; {$ifdef USEINLINE}inline;{$endif}
         procedure getdatabuf(out b;len:integer;out res:integer);
         procedure getdata(out b;len:integer);
         function  getbyte:byte;
    @@ -275,14 +275,14 @@
         function  getlongint:longint;
         function getint64:int64;
         function  getqword:qword;
    -    function getaint:{$ifdef generic_cpu}int64{$else}aint{$endif};
    -    function getasizeint:{$ifdef generic_cpu}int64{$else}asizeint{$endif};
    -    function getpuint:{$ifdef generic_cpu}qword{$else}puint{$endif};
    -    function getptruint:{$ifdef generic_cpu}qword{$else}TConstPtrUInt{$endif};
    -    function getaword:{$ifdef generic_cpu}qword{$else}aword{$endif};
    +    function getaint:{$ifdef generic_cpu}int64{$else}aint{$ifdef USEINLINE}; inline{$endif}{$endif};
    +    function getasizeint:{$ifdef generic_cpu}int64{$else}asizeint{$ifdef USEINLINE}; inline{$endif}{$endif};
    +    function getpuint:{$ifdef generic_cpu}qword{$else}puint{$ifdef USEINLINE}; inline{$endif}{$endif};
    +    function getptruint:{$ifdef generic_cpu}qword{$else}TConstPtrUInt{$ifdef USEINLINE}; inline{$endif}{$endif};
    +    function getaword:{$ifdef generic_cpu}qword{$else}aword{$ifdef USEINLINE}; inline{$endif}{$endif};
         function  getreal:entryreal;
         function  getrealsize(sizeofreal : longint):entryreal;
    -    function  getboolean:boolean;inline;
    +    function  getboolean:boolean; {$ifdef USEINLINE}inline;{$endif}
         function  getstring:string;
         function  getpshortstring:pshortstring;
         function  getansistring:ansistring;
    @@ -297,23 +297,23 @@
         procedure writedata(const b;len:integer);
         procedure writeentry(ibnr:byte);
         procedure putdata(const b;len:integer);virtual;
    -    procedure putbyte(b:byte);
    -    procedure putword(w:word);
    -    procedure putdword(w:dword);
    -    procedure putlongint(l:longint);
    -    procedure putint64(i:int64);
    -    procedure putqword(q:qword);
    -    procedure putaint(i:aint);
    -    procedure putasizeint(i:asizeint);
    -    procedure putpuint(i:puint);
    -    procedure putptruint(v:TConstPtrUInt);
    -    procedure putaword(i:aword);
    +    procedure putbyte(b:byte); {$ifdef USEINLINE}inline;{$endif}
    +    procedure putword(w:word); {$ifdef USEINLINE}inline;{$endif}
    +    procedure putdword(w:dword); {$ifdef USEINLINE}inline;{$endif}
    +    procedure putlongint(l:longint); {$ifdef USEINLINE}inline;{$endif}
    +    procedure putint64(i:int64); {$ifdef USEINLINE}inline;{$endif}
    +    procedure putqword(q:qword); {$ifdef USEINLINE}inline;{$endif}
    +    procedure putaint(i:aint); {$ifdef USEINLINE}inline;{$endif}
    +    procedure putasizeint(i:asizeint); {$ifdef USEINLINE}inline;{$endif}
    +    procedure putpuint(i:puint); {$ifdef USEINLINE}inline;{$endif}
    +    procedure putptruint(v:TConstPtrUInt); {$ifdef USEINLINE}inline;{$endif}
    +    procedure putaword(i:aword); {$ifdef USEINLINE}inline;{$endif}
         procedure putreal(d:entryreal);
    -    procedure putboolean(b:boolean);inline;
    -    procedure putstring(const s:string);
    +    procedure putboolean(b:boolean); {$ifdef USEINLINE}inline;{$endif}
    +    procedure putstring(const s:string); {$ifdef USEINLINE}inline;{$endif}
         procedure putansistring(const s:ansistring);
    -    procedure putnormalset(const b);
    -    procedure putsmallset(const b);
    +    procedure putnormalset(const b); {$ifdef USEINLINE}inline;{$endif}
    +    procedure putsmallset(const b); {$ifdef USEINLINE}inline;{$endif}
         procedure tempclose;        // MG: not used, obsolete?
         function  tempopen:boolean; // MG: not used, obsolete?
       end;
    
    inline_entfile.patch (4,414 bytes)

Activities

J. Gareth Moreton

2019-04-09 04:12

developer   ~0115336

Last edited: 2019-04-09 04:13

View 2 revisions

A couple of minor fixes with parameter names and sanity checking were also applied, namely that the result for "getaword" etc now returns 0 instead of 4 if SizeOf(aword) is not 1, 2, 4 or 8 (strictly speaking, such a situation should never happen and hence the 'else' block should trigger an Internal Error).

Sven Barth

2019-04-14 22:25

manager   ~0115506

Why do you mix different changes in a single patch? The change of the comment or the changes regarding aint or ptruint are not related to the inlining. Mixing changes makes it hard to merge single changes if they're part of the same commit.

J. Gareth Moreton

2019-04-15 00:55

developer   ~0115511

Last edited: 2019-04-15 00:58

View 3 revisions

Because I'm unable to help myself when I see the smallest of errors and I don't know how I can provide a patch for every individual anomaly I see. It gets hard to separate out as well when creating said patches sometimes.

I can't exactly justify a patch that fixes a typo in a comment, for example. It's hard enough to draw attention to patches as it is. But if you want to ban me from making submissions because I'm too much of a troublemaking boat-rocker who won't conform or be obedient, I understand.

It's not like an office where I can e-mail or physically ask the programmer at the next desk about something that looks incorrect and get a second opinion on it, then patch it into a development branch that's distinct from the main or release branches.

But if I had to explain the issues with the Result field in getaint... the return value is the number of bytes read, and in a situation where the size field is something unexpected, it causes a discrepancy between the reported number of bytes read and the number of bytes actually read from the stream, something that can easily cause a hard-to-trace error later on. In truth, this situation should raise an internal error because it should never be something other than a power of two, but something like that would warrant a separate patch.

Sven Barth

2019-04-16 11:08

manager   ~0115540

Then you need to restrain yourself. I have a multiple of changes in my working copy as well and I *always* take care that I commit only those that I need now and not all the stuff I have (and in 99% of the cases that works out).

The thing is that we might want to merge the changes to 3.2.0, but mixing different, unrelated things might lead to merge conflicts or might merge something that should not be merged. And while merges are always checked by the one doing the merge it's rather annoying to pick the commits apart manually.

If you should use "git svn" instead of pure svn you can help this by doing "git add -p" and adding only those changes you need and afterwards do a "git diff --staged" to get a diff files of only those changes.

rd0x

2019-04-16 13:03

reporter   ~0115545

J. Gareth Moreton, I'd recommend you to use a simple GUI like https://git-cola.github.io/

btw:
FPC should drop ancient SVN and move to github, makes sending patches so much simpler (pull request) and issues can also be tracked there - instead of using an insecure MantisB from 2012!!
https://www.cvedetails.com/vulnerability-list/vendor_id-11072/Mantisbt.html
https://www.exploit-db.com/search?q=Mantis+Bug+Tracker

J. Gareth Moreton

2019-04-16 13:31

developer   ~0115547

But you don't want to merge this into 3.2.0. The bug fixes are worse than a simple inlining, and who gives a damn about spelling mistakes in comments?

Sven Barth

2019-04-16 16:25

manager   ~0115552

It's about the principle: If you don't adhere to the guidelines with something simple like this who's to say you're doing it for something much more complex?

J. Gareth Moreton

2019-04-16 16:31

developer   ~0115553

I'm too unreliable, too emotional, too subhuman. The simple answer is... I won't, because if I see a bug, I must fix it, and I cannot stand to see a bug knowingly be kept in a live product. But you dodged the comment... who in their right mind will even look at a patch that just fixes typos or bugs that are unlikely to get triggered.

But you don't need my patches, no-one does. I'm done. You don't need me, admit it. FPC doesn't need anyone. I'm too dumb and stupid to understand and properly use git and SVN.

And another thing... having to separate out all these changes into multiple patches just causes merge conflicts when you try to put them together. Even patches that were fine initially end up causing merge conflicts because they're left for so long that other changes were made over the top of them.

I know no-one in their right mind would ever hire me as a programmer again, and I don't care. I'm not needed.

P.S. I also can't use git any more for new branches because my hard drive is full and I don't even have the money to buy food, let alone a new hard drive, but yeah, tough s***, life's unfair. You don't need me.

J. Gareth Moreton

2019-04-18 00:39

developer   ~0115613

I have no excuse for my outburst, even if there are other unrelated factors at play. Still, here are the patches with all the 'noise' removed.

J. Gareth Moreton

2019-04-18 02:10

developer   ~0115619

Last edited: 2019-04-18 04:43

View 2 revisions

I've uploaded some alternative versions of the patches that make use of {$ifdef USEINLINE}, which seems to be a common theme in the compiler for inlined code.

After a bit of research, it seems that USEINLINE is NOT defined if EXTDEBUG is defined, so I figure it would help to abide by those standards and guidelines...

J. Gareth Moreton

2019-04-18 21:41

developer   ~0115663

Updated patches to be compatible with the latest trunk version. Only the patches with the {$ifdef USEINLINE} directives are now present, since this is a design principle of the compiler that all inlines are disabled for debugging purposes if EXTDEBUG is present ("inline" really messes up breakpoints).

This should pair nicely with 0035409.

Jonas Maebe

2019-04-22 17:27

manager   ~0115730

An inline directive in the implementation but not in the interface should be rejected by the compiler, because it changes the interface crc. I know it does not do that currently, but that is a compiler bug.

J. Gareth Moreton

2019-04-22 17:51

developer   ~0115732

Oh that's interesting. I put "inline" in the implementation section because I felt it improved maintainability, as you could see the directive alongside the code that you were inlining and quickly determine if it's appropriate or not.

Nevertheless, if you say it's a bug, I'll change the patches.

J. Gareth Moreton

2019-04-22 18:32

developer   ~0115733

Updated patches so "inline" now only appears in the interface section.

It might be classed as 'noise', but the "inline;" modifiers on getboolean/putboolean have been changed to "{$ifdef USEINLINE}inline;{$endif}" to match how the directive is used elsewhere.

inline_cstreams.patch (3,796 bytes)
Index: compiler/cstreams.pas
===================================================================
--- compiler/cstreams.pas	(revision 41924)
+++ compiler/cstreams.pas	(working copy)
@@ -67,8 +67,8 @@
 
   TCStream = class(TObject)
   private
-    function GetPosition: Longint;
-    procedure SetPosition(Pos: Longint);
+    function GetPosition: Longint; {$ifdef USEINLINE}inline;{$endif}
+    procedure SetPosition(Pos: Longint); {$ifdef USEINLINE}inline;{$endif}
     function GetSize: Longint;
   protected
     procedure SetSize(NewSize: Longint); virtual;
@@ -79,22 +79,22 @@
     procedure ReadBuffer(var Buffer; Count: Longint);
     procedure WriteBuffer(const Buffer; Count: Longint);
     function CopyFrom(Source: TCStream; Count: Longint): Longint;
-    function ReadComponent(Instance: TCComponent): TCComponent;
-    function ReadComponentRes(Instance: TCComponent): TCComponent;
-    procedure WriteComponent(Instance: TCComponent);
-    procedure WriteComponentRes(const ResName: string; Instance: TCComponent);
-    procedure WriteDescendent(Instance, Ancestor: TCComponent);
-    procedure WriteDescendentRes(const ResName: string; Instance, Ancestor: TCComponent);
-    procedure WriteResourceHeader(const ResName: string; {!!!:out} var FixupInfo: Integer);
-    procedure FixupResourceHeader(FixupInfo: Integer);
-    procedure ReadResHeader;
-    function ReadByte : Byte;
-    function ReadWord : Word;
-    function ReadDWord : Cardinal;
+    function ReadComponent(Instance: TCComponent): TCComponent; {$ifdef USEINLINE}inline;{$endif}
+    function ReadComponentRes(Instance: TCComponent): TCComponent; {$ifdef USEINLINE}inline;{$endif}
+    procedure WriteComponent(Instance: TCComponent); {$ifdef USEINLINE}inline;{$endif}
+    procedure WriteComponentRes(const ResName: string; Instance: TCComponent); {$ifdef USEINLINE}inline;{$endif}
+    procedure WriteDescendent(Instance, Ancestor: TCComponent); {$ifdef USEINLINE}inline;{$endif}
+    procedure WriteDescendentRes(const ResName: string; Instance, Ancestor: TCComponent); {$ifdef USEINLINE}inline;{$endif}
+    procedure WriteResourceHeader(const ResName: string; {!!!:out} var FixupInfo: Integer); {$ifdef USEINLINE}inline;{$endif}
+    procedure FixupResourceHeader(FixupInfo: Integer); {$ifdef USEINLINE}inline;{$endif}
+    procedure ReadResHeader; {$ifdef USEINLINE}inline;{$endif}
+    function ReadByte : Byte; {$ifdef USEINLINE}inline;{$endif}
+    function ReadWord : Word; {$ifdef USEINLINE}inline;{$endif}
+    function ReadDWord : Cardinal; {$ifdef USEINLINE}inline;{$endif}
     function ReadAnsiString : AnsiString;
-    procedure WriteByte(b : Byte);
-    procedure WriteWord(w : Word);
-    procedure WriteDWord(d : Cardinal);
+    procedure WriteByte(b : Byte); {$ifdef USEINLINE}inline;{$endif}
+    procedure WriteWord(w : Word); {$ifdef USEINLINE}inline;{$endif}
+    procedure WriteDWord(d : Cardinal); {$ifdef USEINLINE}inline;{$endif}
     Procedure WriteAnsiString (S : AnsiString);
     property Position: Longint read GetPosition write SetPosition;
     property Size: Longint read GetSize write SetSize;
@@ -153,11 +153,11 @@
     FMemory: Pointer;
     FSize, FPosition: Longint;
   protected
-    procedure SetPointer(Ptr: Pointer; ASize: Longint);
+    procedure SetPointer(Ptr: Pointer; ASize: Longint); {$ifdef USEINLINE}inline;{$endif}
   public
     function Read(var Buffer; Count: Longint): Longint; override;
     function Seek(Offset: Longint; Origin: Word): Longint; override;
-    procedure SaveToStream(Stream: TCStream);
+    procedure SaveToStream(Stream: TCStream); {$ifdef USEINLINE}inline;{$endif}
     procedure SaveToFile(const FileName: string);
     property Memory: Pointer read FMemory;
   end;
inline_cstreams.patch (3,796 bytes)
inline_entfile.patch (4,414 bytes)
Index: compiler/entfile.pas
===================================================================
--- compiler/entfile.pas	(revision 41924)
+++ compiler/entfile.pas	(working copy)
@@ -249,7 +249,7 @@
     constructor create(const fn:string);
     destructor  destroy;override;
     function getversion:integer;
-    procedure flush;
+    procedure flush; {$ifdef USEINLINE}inline;{$endif}
     procedure closefile;virtual;
     procedure newentry;
     property position:longint read getposition write setposition;
@@ -264,9 +264,9 @@
     procedure readdata(out b;len:integer);
     procedure skipdata(len:integer);
     function  readentry:byte;
-    function  EndOfEntry:boolean;
-    function  entrysize:longint;
-    function  entryleft:longint;
+    function  EndOfEntry:boolean; {$ifdef USEINLINE}inline;{$endif}
+    function  entrysize:longint; {$ifdef USEINLINE}inline;{$endif}
+    function  entryleft:longint; {$ifdef USEINLINE}inline;{$endif}
     procedure getdatabuf(out b;len:integer;out res:integer);
     procedure getdata(out b;len:integer);
     function  getbyte:byte;
@@ -275,14 +275,14 @@
     function  getlongint:longint;
     function getint64:int64;
     function  getqword:qword;
-    function getaint:{$ifdef generic_cpu}int64{$else}aint{$endif};
-    function getasizeint:{$ifdef generic_cpu}int64{$else}asizeint{$endif};
-    function getpuint:{$ifdef generic_cpu}qword{$else}puint{$endif};
-    function getptruint:{$ifdef generic_cpu}qword{$else}TConstPtrUInt{$endif};
-    function getaword:{$ifdef generic_cpu}qword{$else}aword{$endif};
+    function getaint:{$ifdef generic_cpu}int64{$else}aint{$ifdef USEINLINE}; inline{$endif}{$endif};
+    function getasizeint:{$ifdef generic_cpu}int64{$else}asizeint{$ifdef USEINLINE}; inline{$endif}{$endif};
+    function getpuint:{$ifdef generic_cpu}qword{$else}puint{$ifdef USEINLINE}; inline{$endif}{$endif};
+    function getptruint:{$ifdef generic_cpu}qword{$else}TConstPtrUInt{$ifdef USEINLINE}; inline{$endif}{$endif};
+    function getaword:{$ifdef generic_cpu}qword{$else}aword{$ifdef USEINLINE}; inline{$endif}{$endif};
     function  getreal:entryreal;
     function  getrealsize(sizeofreal : longint):entryreal;
-    function  getboolean:boolean;inline;
+    function  getboolean:boolean; {$ifdef USEINLINE}inline;{$endif}
     function  getstring:string;
     function  getpshortstring:pshortstring;
     function  getansistring:ansistring;
@@ -297,23 +297,23 @@
     procedure writedata(const b;len:integer);
     procedure writeentry(ibnr:byte);
     procedure putdata(const b;len:integer);virtual;
-    procedure putbyte(b:byte);
-    procedure putword(w:word);
-    procedure putdword(w:dword);
-    procedure putlongint(l:longint);
-    procedure putint64(i:int64);
-    procedure putqword(q:qword);
-    procedure putaint(i:aint);
-    procedure putasizeint(i:asizeint);
-    procedure putpuint(i:puint);
-    procedure putptruint(v:TConstPtrUInt);
-    procedure putaword(i:aword);
+    procedure putbyte(b:byte); {$ifdef USEINLINE}inline;{$endif}
+    procedure putword(w:word); {$ifdef USEINLINE}inline;{$endif}
+    procedure putdword(w:dword); {$ifdef USEINLINE}inline;{$endif}
+    procedure putlongint(l:longint); {$ifdef USEINLINE}inline;{$endif}
+    procedure putint64(i:int64); {$ifdef USEINLINE}inline;{$endif}
+    procedure putqword(q:qword); {$ifdef USEINLINE}inline;{$endif}
+    procedure putaint(i:aint); {$ifdef USEINLINE}inline;{$endif}
+    procedure putasizeint(i:asizeint); {$ifdef USEINLINE}inline;{$endif}
+    procedure putpuint(i:puint); {$ifdef USEINLINE}inline;{$endif}
+    procedure putptruint(v:TConstPtrUInt); {$ifdef USEINLINE}inline;{$endif}
+    procedure putaword(i:aword); {$ifdef USEINLINE}inline;{$endif}
     procedure putreal(d:entryreal);
-    procedure putboolean(b:boolean);inline;
-    procedure putstring(const s:string);
+    procedure putboolean(b:boolean); {$ifdef USEINLINE}inline;{$endif}
+    procedure putstring(const s:string); {$ifdef USEINLINE}inline;{$endif}
     procedure putansistring(const s:ansistring);
-    procedure putnormalset(const b);
-    procedure putsmallset(const b);
+    procedure putnormalset(const b); {$ifdef USEINLINE}inline;{$endif}
+    procedure putsmallset(const b); {$ifdef USEINLINE}inline;{$endif}
     procedure tempclose;        // MG: not used, obsolete?
     function  tempopen:boolean; // MG: not used, obsolete?
   end;
inline_entfile.patch (4,414 bytes)

rd0x

2019-06-28 09:01

reporter   ~0116983

Maybe change lines like
function getaint:{$ifdef generic_cpu}int64{$else}aint{$ifdef USEINLINE}; inline{$endif}{$endif};
to
function getaint:{$ifdef generic_cpu}int64{$else}aint{$endif}; {$ifdef USEINLINE}inline;{$endif}
for better clarity (compiler/entfile.pas)

J. Gareth Moreton

2019-06-28 10:01

developer   ~0116986

The reason why it is set up like that is because the generic CPU versions do not have implementations that are appropriate for inlining. Normally the inlining is only done for functions that simply call other functions.

Sven Barth

2019-06-28 15:32

manager   ~0116990

Thank you for your patches, I've applied them.

Interestingly sometimes inlining of tentryfile.putstring is not done (e.g. in tprocdef.ppuwrite, line 5919). I've not yet investigated why the compiler does not do it in that case.

Please test and close if okay.

Issue History

Date Modified Username Field Change
2019-04-09 04:08 J. Gareth Moreton New Issue
2019-04-09 04:08 J. Gareth Moreton File Added: inline_cstreams.patch
2019-04-09 04:09 J. Gareth Moreton File Added: inline_entfile.patch
2019-04-09 04:09 J. Gareth Moreton Tag Attached: compiler
2019-04-09 04:09 J. Gareth Moreton Tag Attached: patch
2019-04-09 04:09 J. Gareth Moreton Tag Attached: refactoring
2019-04-09 04:09 J. Gareth Moreton Priority normal => low
2019-04-09 04:09 J. Gareth Moreton Severity minor => tweak
2019-04-09 04:10 J. Gareth Moreton Tag Attached: inline
2019-04-09 04:12 J. Gareth Moreton Note Added: 0115336
2019-04-09 04:13 J. Gareth Moreton Note Edited: 0115336 View Revisions
2019-04-14 22:25 Sven Barth Note Added: 0115506
2019-04-15 00:55 J. Gareth Moreton Note Added: 0115511
2019-04-15 00:58 J. Gareth Moreton Note Edited: 0115511 View Revisions
2019-04-15 00:58 J. Gareth Moreton Note Edited: 0115511 View Revisions
2019-04-16 11:08 Sven Barth Note Added: 0115540
2019-04-16 13:03 rd0x Note Added: 0115545
2019-04-16 13:31 J. Gareth Moreton Note Added: 0115547
2019-04-16 13:31 J. Gareth Moreton File Deleted: inline_cstreams.patch
2019-04-16 13:31 J. Gareth Moreton File Deleted: inline_entfile.patch
2019-04-16 13:32 J. Gareth Moreton Status new => closed
2019-04-16 13:32 J. Gareth Moreton Assigned To => J. Gareth Moreton
2019-04-16 13:32 J. Gareth Moreton Resolution open => suspended
2019-04-16 16:25 Sven Barth Note Added: 0115552
2019-04-16 16:31 J. Gareth Moreton Note Added: 0115553
2019-04-16 16:31 J. Gareth Moreton Status closed => feedback
2019-04-16 16:31 J. Gareth Moreton Resolution suspended => reopened
2019-04-16 16:32 J. Gareth Moreton Status feedback => closed
2019-04-16 16:32 J. Gareth Moreton Resolution reopened => suspended
2019-04-18 00:39 J. Gareth Moreton Assigned To J. Gareth Moreton =>
2019-04-18 00:39 J. Gareth Moreton Note Added: 0115613
2019-04-18 00:39 J. Gareth Moreton Status closed => feedback
2019-04-18 00:39 J. Gareth Moreton Resolution suspended => reopened
2019-04-18 00:39 J. Gareth Moreton Status feedback => new
2019-04-18 00:39 J. Gareth Moreton File Added: inline_cstreams.patch
2019-04-18 00:39 J. Gareth Moreton File Added: inline_entfile.patch
2019-04-18 02:08 J. Gareth Moreton File Added: inline_cstreams_with_def.patch
2019-04-18 02:08 J. Gareth Moreton File Added: inline_entfile_with_def.patch
2019-04-18 02:10 J. Gareth Moreton Note Added: 0115619
2019-04-18 04:43 J. Gareth Moreton Note Edited: 0115619 View Revisions
2019-04-18 21:38 J. Gareth Moreton File Deleted: inline_entfile_with_def.patch
2019-04-18 21:38 J. Gareth Moreton File Deleted: inline_cstreams_with_def.patch
2019-04-18 21:39 J. Gareth Moreton File Deleted: inline_entfile.patch
2019-04-18 21:39 J. Gareth Moreton File Deleted: inline_cstreams.patch
2019-04-18 21:39 J. Gareth Moreton File Added: inline_cstreams.patch
2019-04-18 21:39 J. Gareth Moreton File Added: inline_entfile.patch
2019-04-18 21:41 J. Gareth Moreton Note Added: 0115663
2019-04-22 17:27 Jonas Maebe Note Added: 0115730
2019-04-22 17:51 J. Gareth Moreton Note Added: 0115732
2019-04-22 18:30 J. Gareth Moreton File Deleted: inline_cstreams.patch
2019-04-22 18:30 J. Gareth Moreton File Deleted: inline_entfile.patch
2019-04-22 18:32 J. Gareth Moreton File Added: inline_cstreams.patch
2019-04-22 18:32 J. Gareth Moreton File Added: inline_entfile.patch
2019-04-22 18:32 J. Gareth Moreton Note Added: 0115733
2019-06-28 09:01 rd0x Note Added: 0116983
2019-06-28 10:01 J. Gareth Moreton Note Added: 0116986
2019-06-28 15:32 Sven Barth Assigned To => Sven Barth
2019-06-28 15:32 Sven Barth Status new => resolved
2019-06-28 15:32 Sven Barth Resolution reopened => fixed
2019-06-28 15:32 Sven Barth Fixed in Version => 3.3.1
2019-06-28 15:32 Sven Barth Fixed in Revision => 42303,42304
2019-06-28 15:32 Sven Barth FPCTarget => -
2019-06-28 15:32 Sven Barth Note Added: 0116990