View Issue Details

IDProjectCategoryView StatusLast Update
0037343FPCCompilerpublic2020-07-17 22:58
ReporterJ. Gareth Moreton Assigned ToJonas Maebe  
PrioritylowSeveritytweakReproducibilityN/A
Status resolvedResolutionfixed 
PlatformCross-platformOSMicrosoft Windows 
Product Version3.3.1 
Target Version4.0.0Fixed in Version3.3.1 
Summary0037343: [Patch] Register promotion for single-field record types
DescriptionA developer came to me directly asking about an optimisation, and I raised it on the fpc-devel mailing list, namely the slightly sub-optimal situation where local variables of record types are always stored on the stack, never promoted to a register. While there is still some research to be done in this area of optimisation, the patch supplied will promote variables where the record type contains just a single field.

RegVar.lpr is a test program that shows the optimisation in action.
Steps To Reproduce- Compile test program RegVar.lpr, without the patch applied, and verify that the 2nd time metric (with the "Slow" comment) runs notably slower than the other two.
- Apply patch and then compile the test program again, and this time verify that the 2nd time metric is comparable to the other two.
- Confirm that the compiler has not suffered any regressions elsewhere.
Additional InformationSpecial thanks to Jonas Maebe for directing me to some useful internal methods that made this optimisation remarkably simple.

The most important changes in the patch exist only in the file "compiler/symsym.pas", where only 2 lines of code are added. The other changes relate to common subexpression elimination.
Tagscompiler, optimizations, patch
Fixed in Revision
FPCOldBugId
FPCTarget-
Attached Files

Activities

J. Gareth Moreton

2020-07-13 12:15

developer  

RegVar.lpr (666 bytes)   
program RegVar;

{$mode objfpc}

uses
  SysUtils;

type
  TTest = record
    P: int64;
  end;

  procedure Test;
  var
    V: TTest;
    P: int64;
    T: UInt64;
    i, C: LongWord;
  begin
    C := 1000 * 1000 * 1000;

    T := GetTickCount64;
    P := 1;
    for i := 1 to C do
      Inc(P);
    WriteLn(GetTickCount64 - T);

    T := GetTickCount64;
    V.P := 1;
    for i := 1 to C do
      Inc(V.P);
    WriteLn(GetTickCount64 - T); // Slow

    T := GetTickCount64;
    V.P := 1;
    P := V.P;
    for i := 1 to C do
      Inc(P);
    P := V.P;
    WriteLn(GetTickCount64 - T);
  end;

begin
  Test;
  ReadLn;
end.
RegVar.lpr (666 bytes)   
simple_record_regvar.patch (3,120 bytes)   
Index: compiler/optcse.pas
===================================================================
--- compiler/optcse.pas	(revision 45763)
+++ compiler/optcse.pas	(working copy)
@@ -51,7 +51,7 @@
       nutils,compinnr,
       nbas,nld,ninl,ncal,nadd,nmem,
       pass_1,
-      symconst,symdef,symsym,
+      symconst,symdef,symsym,symtable,symtype,
       defutil,
       optbase;
 
@@ -138,6 +138,7 @@
 
       var
         i : longint;
+        tempdef : tdef;
       begin
         result:=fen_false;
         { don't add the tree below an untyped const parameter: there is
@@ -160,14 +161,29 @@
         if
           { node possible to add? }
           assigned(n.resultdef) and
-          (
+          ((
             { regable expressions }
             (actualtargetnode(@n)^.flags*[nf_write,nf_modify,nf_address_taken]=[]) and
-            ((((tstoreddef(n.resultdef).is_intregable or tstoreddef(n.resultdef).is_fpuregable or tstoreddef(n.resultdef).is_const_intregable) and
+            (
+              tstoreddef(n.resultdef).is_intregable or
+              tstoreddef(n.resultdef).is_fpuregable or
+              tstoreddef(n.resultdef).is_const_intregable
+            ) and
             { is_int/fpuregable allows arrays and records to be in registers, cse cannot handle this }
-            (not(n.resultdef.typ in [arraydef,recorddef]))) or
-             is_dynamic_array(n.resultdef) or
-             ((n.resultdef.typ in [arraydef,recorddef]) and not(is_special_array(tstoreddef(n.resultdef))) and not(tstoreddef(n.resultdef).is_intregable) and not(tstoreddef(n.resultdef).is_fpuregable))
+            (
+              not(n.resultdef.typ in [arraydef,recorddef]) or
+              (
+                (
+                  (n.resultdef.typ = recorddef) and
+                  tabstractrecordsymtable(tabstractrecorddef(n.resultdef).symtable).has_single_field(tempdef)
+                ) or
+                is_dynamic_array(n.resultdef) or
+                (
+                  not(is_special_array(tstoreddef(n.resultdef))) and
+                  not(tstoreddef(n.resultdef).is_intregable) and
+                  not(tstoreddef(n.resultdef).is_fpuregable)
+                )
+              )
             ) and
             { same for voiddef }
             not(is_void(n.resultdef)) and
Index: compiler/symsym.pas
===================================================================
--- compiler/symsym.pas	(revision 45763)
+++ compiler/symsym.pas	(working copy)
@@ -1728,6 +1728,8 @@
 
 
     function tabstractvarsym.is_regvar(refpara: boolean):boolean;
+      var
+        tempdef : tdef;
       begin
         { Register variables are not allowed in the following cases:
            - regvars are disabled
@@ -1746,6 +1748,7 @@
 {$if not defined(powerpc) and not defined(powerpc64)}
                 and ((vardef.typ <> recorddef) or
                      (varregable = vr_addr) or
+                     tabstractrecordsymtable(tabstractrecorddef(vardef).symtable).has_single_field(tempdef) or
                      not(varstate in [vs_written,vs_readwritten]));
 {$endif}
       end;
simple_record_regvar.patch (3,120 bytes)   

Jonas Maebe

2020-07-17 22:58

manager   ~0124132

Thanks, applied. I had to do an extra fix to disable record regvars with floating point values in them though, because the rest of the code generator doesn't support them (they used to be disabled originally, but then got enabled at some point, possibly by accident).

Issue History

Date Modified Username Field Change
2020-07-13 12:15 J. Gareth Moreton New Issue
2020-07-13 12:15 J. Gareth Moreton File Added: RegVar.lpr
2020-07-13 12:15 J. Gareth Moreton File Added: simple_record_regvar.patch
2020-07-13 12:15 J. Gareth Moreton Tag Attached: patch
2020-07-13 12:15 J. Gareth Moreton Tag Attached: compiler
2020-07-13 12:15 J. Gareth Moreton Tag Attached: optimizations
2020-07-13 12:16 J. Gareth Moreton Priority normal => low
2020-07-13 12:16 J. Gareth Moreton Severity minor => tweak
2020-07-13 12:16 J. Gareth Moreton FPCTarget => -
2020-07-13 12:18 J. Gareth Moreton Additional Information Updated View Revisions
2020-07-17 22:58 Jonas Maebe Assigned To => Jonas Maebe
2020-07-17 22:58 Jonas Maebe Status new => resolved
2020-07-17 22:58 Jonas Maebe Resolution open => fixed
2020-07-17 22:58 Jonas Maebe Fixed in Version => 3.3.1
2020-07-17 22:58 Jonas Maebe Note Added: 0124132