View Issue Details

IDProjectCategoryView StatusLast Update
0038691FPCCompilerpublic2021-04-19 21:55
ReporterJ. Gareth Moreton Assigned ToFlorian  
PrioritynormalSeveritymajorReproducibilityN/A
Status resolvedResolutionfixed 
Platformaarch64-linuxOSLInux (Raspberry Pi OS) 
Product Version3.2.0 
Fixed in Version3.3.1 
Summary0038691: [Patch] AArch64 OptPass1Shift register tracking fault fix
DescriptionThis patch fixes a fault with the OptPass1Shift optimisation that sometimes causes a register to be deallocated when it's still in use, thus permitting incorrect optimisations later on. This specifically caused a crash when the compiler was built with the "-glttt -CriotR" parameters (but which disappeared when -OoNOPEEPHOLE was also specified).
Steps To ReproduceApply patch and confirm correct compilation and removal of crash at ppc2 stage when building with "-glttt -CriotR" options.
Additional InformationAn example in the System unit (under -O2):

WIthout the Peephole Optimizer:

    lsr x3,x3,1
    mov x5,x3
.Ll778:
    sub x1,x1,x5
.Ll779:
    lsl x3,x5,1 <-- x5 is deallocated after this instruction
    add x6,x0,x3

Incorrect:

.Ll778:
    sub x1,x1,x3,lsr 1 <-- Peephole Optimizer notices that x5 is deallocated after this instruction even though it's actually in use, so mistakenly makes the lsr/sub optimisation
.Ll779:
    add x6,x0,x5,lsl 1 <-- x5 is now undefined

Fixed:

    lsr x5,x3,1
.Ll778:
    sub x1,x1,x5
.Ll779:
    add x6,x0,x5,lsl 1 <-- x5 is allocated up to this instruction so lsr/sub isn't optimised above.
Tagsaarch64, optimization, patch
Fixed in Revision49235
FPCOldBugId
FPCTarget-
Attached Files

Activities

J. Gareth Moreton

2021-04-01 15:45

developer  

optpass1shift-fix.patch (1,483 bytes)   
Index: compiler/aarch64/aoptcpu.pas
===================================================================
--- compiler/aarch64/aoptcpu.pas	(revision 49096)
+++ compiler/aarch64/aoptcpu.pas	(working copy)
@@ -379,15 +379,23 @@
                          taicpu(hp1).oper[0]^.reg, taicpu(p).oper[1]^.reg,
                          shifterop);
 
+                { Make sure the register used in the shifting is tracked all
+                  the way through, otherwise it may become deallocated while
+                  it's still live and cause incorrect optimisations later }
+                if (taicpu(hp1).oper[0]^.reg <> taicpu(p).oper[1]^.reg) then
+                  begin
+                    TransferUsedRegs(TmpUsedRegs);
+                    UpdateUsedRegs(TmpUsedRegs, tai(p.Next));
+                    ALlocRegBetween(taicpu(p).oper[1]^.reg, p, hp1, TmpUsedRegs);
+                  end;
+
                 taicpu(hp2).fileinfo:=taicpu(hp1).fileinfo;
                 asml.insertbefore(hp2, hp1);
-                GetNextInstruction(p, hp2);
-                asml.remove(p);
-                asml.remove(hp1);
-                p.free;
-                hp1.free;
-                p:=hp2;
-                DebugMsg('Peephole FoldShiftProcess done', p);
+
+                RemoveInstruction(hp1);
+                RemoveCurrentp(p);
+
+                DebugMsg('Peephole FoldShiftProcess done', hp2);
                 Result:=true;
                 break;
               end;
optpass1shift-fix.patch (1,483 bytes)   

J. Gareth Moreton

2021-04-01 15:48

developer   ~0130020

To add a note - this problem appears to be present in 3.2.0.

Chris Rorden

2021-04-01 16:00

reporter   ~0130021

Nice. I note that we are at 3.2.2-rc1. Is there a chance this patch could make it into the release?

J. Gareth Moreton

2021-04-01 16:07

developer   ~0130022

Well, others need to confirm if the problem actually is present in 3.2.0 or if it's due to another problem. I sense this problem is present in 3.2.0 though if only because the original bug wasn't my doing for once!

Florian

2021-04-19 21:55

administrator   ~0130457

Thanks, applied.

Issue History

Date Modified Username Field Change
2021-04-01 15:45 J. Gareth Moreton New Issue
2021-04-01 15:45 J. Gareth Moreton File Added: optpass1shift-fix.patch
2021-04-01 15:45 J. Gareth Moreton Tag Attached: patch
2021-04-01 15:45 J. Gareth Moreton Tag Attached: aarch64
2021-04-01 15:45 J. Gareth Moreton Tag Attached: optimization
2021-04-01 15:45 J. Gareth Moreton Severity minor => major
2021-04-01 15:45 J. Gareth Moreton Additional Information Updated View Revisions
2021-04-01 15:45 J. Gareth Moreton FPCTarget => -
2021-04-01 15:48 J. Gareth Moreton Product Version 3.3.1 => 3.2.0
2021-04-01 15:48 J. Gareth Moreton Note Added: 0130020
2021-04-01 16:00 Chris Rorden Note Added: 0130021
2021-04-01 16:07 J. Gareth Moreton Note Added: 0130022
2021-04-19 21:55 Florian Assigned To => Florian
2021-04-19 21:55 Florian Status new => resolved
2021-04-19 21:55 Florian Resolution open => fixed
2021-04-19 21:55 Florian Fixed in Version => 3.3.1
2021-04-19 21:55 Florian Fixed in Revision => 49235
2021-04-19 21:55 Florian Note Added: 0130457