View Issue Details

IDProjectCategoryView StatusLast Update
0038555FPCCompilerpublic2021-03-04 09:31
ReporterJ. Gareth Moreton Assigned ToFlorian  
PrioritylowSeveritytweakReproducibilityN/A
Status resolvedResolutionfixed 
Platformi386 and x86_64OSMicrosoft Windows 
Product Version3.3.1 
Fixed in Version3.3.1 
Summary0038555: [Patch] SubMov2LeaSub optimisation improvement
DescriptionThis patch improves upon the SubMov2LeaSub optimisation by removing the SUB instruction if the register is not used afterwards, thus reducing the instruction count as well as breaking the dependency chain.
Steps To ReproduceApply patch and confirm correct compilation.
Additional InformationWhether or not the SUB instruction is successfully removed, the current instruction ends up being an LEA instruction, which can sometimes be optimised further if it is adjacent to another LEA instruction, but these kinds of optimisations occur in Pass 1. As a compromise, OptPass2SUB now checks the previous instruction after the optimisation and, if it's a LEA, will call OptPass1LEA on it.

Tests on i386-win32 and x86_64-win64, both without special options and with TEST_OPT="-O4 -gl", pass without incident.
Tagscompiler, i386, optimization, patch, x86_64
Fixed in Revision48871
FPCOldBugId
FPCTarget-
Attached Files

Relationships

parent of 0038579 resolvedFlorian [Patch] AddMov2LeaAdd and -Os change for SubMov2LeaSub 

Activities

Florian

2021-02-28 09:09

administrator   ~0129223

I do not like the idea to call Pass1 in Pass2. This is against all design principle. Just factor out an OptSUBMov routine which can be called in pass 1 and pass 2. There is one thing to fix though: sub might in theory alter the flags, so when removing the sub, you have to check that the flags are not used.

J. Gareth Moreton

2021-02-28 09:43

developer   ~0129224

Last edited: 2021-02-28 09:47

View 2 revisions

I figured that would be the case with calling Pass 1 in Pass 2, hence my e-mail suggestion. A few Pass 2 routines do this already, and I admit I'm a little uneasy about them.

Regarding flags being used, usually there's a CMP between a SUB and a conditional jump etc. at this stage - unnecessary CMPs are only stripped out at the post-peephole stage. Also, the original instructions were a SUB followed by a MOV that used the modified register - usually the compiler doesn't make constructs that then use the flags right after an explicit move like that. Nevertheless, I'll make modifications and check for the flags anyway in order to be safe.

J. Gareth Moreton

2021-02-28 10:39

developer   ~0129225

Hope this patch is better. I've removed calls to LEA optimisations for now and will come back to them later.

J. Gareth Moreton

2021-02-28 10:41

developer   ~0129226

submov-optimisation.patch (1,918 bytes)   
Index: compiler/x86/aoptx86.pas
===================================================================
--- compiler/x86/aoptx86.pas	(revision 48813)
+++ compiler/x86/aoptx86.pas	(working copy)
@@ -7076,7 +7076,7 @@
             movl/q %reg1,%reg2
           To:
             leal/q $-x(%reg1),%reg2
-            subl/q $x,%reg1
+            subl/q $x,%reg1 (can be removed if %reg1 or the flags are not used afterwards)
 
           Breaks the dependency chain and potentially permits the removal of
           a CMP instruction if one follows.
@@ -7100,12 +7100,26 @@
             taicpu(hp1).opcode := A_LEA;
             taicpu(hp1).loadref(0, NewRef);
 
-            { Move what is now the LEA instruction to before the SUB instruction }
-            Asml.Remove(hp1);
-            Asml.InsertBefore(hp1, p);
-            AllocRegBetween(taicpu(hp1).oper[1]^.reg, hp1, p, UsedRegs);
+            TransferUsedRegs(TmpUsedRegs);
+            UpdateUsedRegs(TmpUsedRegs, tai(p.Next));
+            if RegUsedAfterInstruction(NewRef.base, hp1, TmpUsedRegs) or
+              RegUsedAfterInstruction(NR_DEFAULTFLAGS, hp1, TmpUsedRegs) then
+              begin
+                { Move what is now the LEA instruction to before the SUB instruction }
+                Asml.Remove(hp1);
+                Asml.InsertBefore(hp1, p);
+                AllocRegBetween(taicpu(hp1).oper[1]^.reg, hp1, p, UsedRegs);
 
-            DebugMsg(SPeepholeOptimization + 'SubMov2LeaSub', p);
+                DebugMsg(SPeepholeOptimization + 'SubMov2LeaSub', p);
+                p := hp1;
+              end
+            else
+              begin
+                { Since %reg1 or the flags aren't used afterwards, we can delete p completely }
+                RemoveCurrentP(p, hp1);
+                DebugMsg(SPeepholeOptimization + 'SubMov2Lea', p);
+              end;
+
             Result := True;
           end;
       end;
submov-optimisation.patch (1,918 bytes)   

Florian

2021-03-02 22:47

administrator   ~0129335

Thanks, applied.

Issue History

Date Modified Username Field Change
2021-02-28 03:54 J. Gareth Moreton New Issue
2021-02-28 03:54 J. Gareth Moreton File Added: submov-optimisation.patch
2021-02-28 03:54 J. Gareth Moreton Priority normal => low
2021-02-28 03:54 J. Gareth Moreton Severity minor => tweak
2021-02-28 03:54 J. Gareth Moreton FPCTarget => -
2021-02-28 03:54 J. Gareth Moreton Tag Attached: patch
2021-02-28 03:54 J. Gareth Moreton Tag Attached: compiler
2021-02-28 03:54 J. Gareth Moreton Tag Attached: optimization
2021-02-28 03:54 J. Gareth Moreton Tag Attached: i386
2021-02-28 03:54 J. Gareth Moreton Tag Attached: x86_64
2021-02-28 03:56 J. Gareth Moreton Additional Information Updated View Revisions
2021-02-28 09:09 Florian Note Added: 0129223
2021-02-28 09:43 J. Gareth Moreton Note Added: 0129224
2021-02-28 09:47 J. Gareth Moreton Note Edited: 0129224 View Revisions
2021-02-28 10:37 J. Gareth Moreton File Deleted: submov-optimisation.patch
2021-02-28 10:39 J. Gareth Moreton Note Added: 0129225
2021-02-28 10:39 J. Gareth Moreton File Added: submov-optimisation.patch
2021-02-28 10:41 J. Gareth Moreton File Deleted: submov-optimisation.patch
2021-02-28 10:41 J. Gareth Moreton Note Added: 0129226
2021-02-28 10:41 J. Gareth Moreton File Added: submov-optimisation.patch
2021-03-02 22:47 Florian Assigned To => Florian
2021-03-02 22:47 Florian Status new => resolved
2021-03-02 22:47 Florian Resolution open => fixed
2021-03-02 22:47 Florian Fixed in Version => 3.3.1
2021-03-02 22:47 Florian Fixed in Revision => 48871
2021-03-02 22:47 Florian Note Added: 0129335
2021-03-04 09:30 J. Gareth Moreton Relationship added related to 0038579
2021-03-04 09:31 J. Gareth Moreton Relationship deleted related to 0038579
2021-03-04 09:31 J. Gareth Moreton Relationship added parent of 0038579