View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0038555 | FPC | Compiler | public | 2021-02-28 03:54 | 2021-03-04 09:31 |
Reporter | J. Gareth Moreton | Assigned To | Florian | ||
Priority | low | Severity | tweak | Reproducibility | N/A |
Status | resolved | Resolution | fixed | ||
Platform | i386 and x86_64 | OS | Microsoft Windows | ||
Product Version | 3.3.1 | ||||
Fixed in Version | 3.3.1 | ||||
Summary | 0038555: [Patch] SubMov2LeaSub optimisation improvement | ||||
Description | This 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 Reproduce | Apply patch and confirm correct compilation. | ||||
Additional Information | Whether 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. | ||||
Tags | compiler, i386, optimization, patch, x86_64 | ||||
Fixed in Revision | 48871 | ||||
FPCOldBugId | |||||
FPCTarget | - | ||||
Attached Files |
|
|
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. |
|
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. |
|
Hope this patch is better. I've removed calls to LEA optimisations for now and will come back to them later. |
|
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; |
|
Thanks, applied. |
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 |