View Issue Details

IDProjectCategoryView StatusLast Update
0036797FPCCompilerpublic2020-04-27 21:28
ReporterJ. Gareth Moreton Assigned ToFlorian  
PrioritylowSeveritytweakReproducibilityN/A
Status resolvedResolutionfixed 
PlatformCross-platformOSMicrosoft Windows 
Product Version3.3.1 
Fixed in Version3.3.1 
Summary0036797: [Patch / Refactor] RemoveCurrentP optimisations
DescriptionThis patch adds a new version of RemoveCurrentP that, along with the current functionality, will also set p to the specified parameter instead of calling GetNextInstruction. This serves to reduce the number of calls to GetNextInstruction when the next instruction is already known (often the case with a number of peephole optimisations).
Steps To ReproduceApply patch, confirm all compilers build correctly, and also confirm that the binaries produced with the compilers do NOT change (pure refactor).
Additional InformationThe existing version of RemoveCurrentP that takes only a single parameter has not changed. The new version takes a second paramter (declared as "const hp1: tai") but doesn't return a result. Calls to RemoveCurrentP where the next instruction is known have been replaced, as well as a more complex block in the AVR peephole optimiser where RemoveCurrentP was called 3 times sequentially - this has been changed to removing two of the instructions manually, then calling RemoveCurrentP once in order to set p to the instruction that appears after this triplet, and so UpdateUsedRegs is only called once.

There are a couple of points in the code where I've added the comment "// <-- Is this actually safe? hp1 is not necessarily the next instruction. [Kit]" because I replaced the duplet "RemoveCurrentP(p); p := hp1;" with "RemoveCurrentP(p, hp1);"; however, following the program flow yields the fact that hp1 is not necessarily the next instruction (it was obained through "GetNextInstructionUsingReg"). Program functionality is identical, and probably is safe, but optimisations between p and hp1 may get missed in the cases where the two instructions are not adjacent... this is something to check at another time.
Tagscompiler, optimizations, patch, refactor
Fixed in Revision45142
FPCOldBugId
FPCTarget-
Attached Files

Activities

J. Gareth Moreton

2020-03-16 02:41

developer  

removecurrentp_refactor.patch (12,265 bytes)   
Index: compiler/aoptobj.pas
===================================================================
--- compiler/aoptobj.pas	(revision 44303)
+++ compiler/aoptobj.pas	(working copy)
@@ -341,6 +341,9 @@
         { removes p from asml, updates registers and replaces it by a valid value, if this is the case true is returned }
         function RemoveCurrentP(var p : tai): boolean;
 
+        { removes p from asml, updates registers and replaces p with hp1 (if the next instruction was known beforehand) }
+        procedure RemoveCurrentP(var p: tai; const hp1: tai); inline;
+
        { traces sucessive jumps to their final destination and sets it, e.g.
          je l1                je l3
          <code>               <code>
@@ -1498,6 +1501,15 @@
       end;
 
 
+    procedure TAOptObj.RemoveCurrentP(var p: tai; const hp1: tai); inline;
+      begin
+        UpdateUsedRegs(tai(p.Next));
+        AsmL.Remove(p);
+        p.Free;
+        p := hp1;
+      end;
+
+
     function FindLiveLabel(hp: tai; var l: tasmlabel): Boolean;
       var
         next: tai;
Index: compiler/arm/aoptcpu.pas
===================================================================
--- compiler/arm/aoptcpu.pas	(revision 44303)
+++ compiler/arm/aoptcpu.pas	(working copy)
@@ -1720,9 +1720,7 @@
                             DebugMsg('Peephole AndStrb2Strb done', p);
                             taicpu(hp1).loadReg(0,taicpu(p).oper[1]^.reg);
                             AllocRegBetween(taicpu(p).oper[1]^.reg,p,hp1,UsedRegs);
-                            GetNextInstruction(p, hp1);
                             RemoveCurrentP(p);
-                            p:=hp1;
                             result:=true;
                           end
                         {
@@ -1994,8 +1992,7 @@
                             AllocRegBetween(taicpu(hp1).oper[3]^.reg,p,hp1,UsedRegs);
 
                             taicpu(hp1).ops:=4;
-                            RemoveCurrentP(p);
-                            p:=hp1;
+                            RemoveCurrentP(p, hp1); // <-- Is this actually safe? hp1 is not necessarily the next instruction. [Kit]
                           end;
 
                         result:=true;
Index: compiler/avr/aoptcpu.pas
===================================================================
--- compiler/avr/aoptcpu.pas	(revision 44303)
+++ compiler/avr/aoptcpu.pas	(working copy)
@@ -679,7 +679,8 @@
                     begin
                       DebugMsg('Peephole AddAdc2Add performed', p);
 
-                      result:=RemoveCurrentP(p);
+                      RemoveCurrentP(p, hp1);
+                      Result := True;
                     end;
                   end;
                 A_SUB:
@@ -692,7 +693,8 @@
 
                       taicpu(hp1).opcode:=A_SUB;
 
-                      result:=RemoveCurrentP(p);
+                      RemoveCurrentP(p, hp1);
+                      Result := True;
                     end;
                   end;
                 A_CLR:
@@ -795,9 +797,23 @@
 
                            taicpu(hp3).loadreg(1, taicpu(p).oper[0]^.reg);
 
-                           RemoveCurrentP(p);
-                           RemoveCurrentP(p);
-                           result:=RemoveCurrentP(p);
+                           { We're removing 3 concurrent instructions.  Remove hp1
+                             and hp2 manually instead of calling RemoveCurrentP
+                             as this means we won't be calling UpdateUsedRegs 3 times }
+                           asml.Remove(hp1);
+                           hp1.Free;
+
+                           asml.Remove(hp2);
+                           hp2.Free;
+
+                           { By removing p last, we've guaranteed that p.Next is
+                             valid (storing it prior to removing the instructions
+                             may result in a dangling pointer if hp1 immediately
+                             follows p), and because hp1, hp2 and hp3 came from
+                             sequential calls to GetNextInstruction, it is
+                             guaranteed that UpdateUsedRegs will stop at hp3. [Kit] }
+                           RemoveCurrentP(p, hp3);
+                           Result := True;
                          end
                        else
                          begin
@@ -883,7 +899,8 @@
                           not(MatchInstruction(hp1,[A_CALL,A_RCALL])) then
                           begin
                             DebugMsg('Peephole Mov2Nop performed', p);
-                            result:=RemoveCurrentP(p);
+                            RemoveCurrentP(p, hp1);
+                            Result := True;
                             exit;
                           end;
                       end;
@@ -1092,7 +1109,8 @@
                         begin
                           DebugMsg('Peephole MovMov2Mov performed', p);
 
-                          result:=RemoveCurrentP(p);
+                          RemoveCurrentP(p,hp1);
+                          Result := True;
 
                           GetNextInstruction(hp1,hp1);
                           if not assigned(hp1) then
Index: compiler/x86/aoptx86.pas
===================================================================
--- compiler/x86/aoptx86.pas	(revision 44303)
+++ compiler/x86/aoptx86.pas	(working copy)
@@ -1085,10 +1085,8 @@
             if (taicpu(p).ops = 2) then
              { remove "imul $1, reg" }
               begin
-                hp1 := tai(p.Next);
                 DebugMsg(SPeepholeOptimization + 'Imul2Nop done',p);
-                RemoveCurrentP(p);
-                result:=true;
+                Result := RemoveCurrentP(p);
               end
             else
              { change "imul $1, reg1, reg2" to "mov reg1, reg2" }
@@ -1138,7 +1136,7 @@
                   AsmL.InsertAfter(hp1,p);
                   DebugMsg(SPeepholeOptimization + 'Imul2LeaShl done',p);
                   taicpu(hp1).fileinfo:=taicpu(p).fileinfo;
-                  RemoveCurrentP(p);
+                  RemoveCurrentP(p, hp1);
                   if ShiftValue>0 then
                     AsmL.InsertAfter(taicpu.op_const_reg(A_SHL, opsize, ShiftValue, taicpu(hp1).oper[1]^.reg),hp1);
               end;
@@ -1395,9 +1393,7 @@
               <nop> }
             if MatchOperand(taicpu(p).oper[0]^,taicpu(p).oper[1]^) then
               begin
-                GetNextInstruction(p,hp1);
                 RemoveCurrentP(p);
-                p:=hp1;
                 result:=true;
                 exit;
               end
@@ -1539,10 +1535,9 @@
                     if not(RegUsedAfterInstruction(taicpu(p).oper[1]^.reg,hp2,TmpUsedRegs)) then
                       begin
                         taicpu(hp1).loadoper(2,taicpu(p).oper[0]^);
-                        RemoveCurrentP(p);
+                        RemoveCurrentP(p, hp1); // <-- Is this actually safe? hp1 is not necessarily the next instruction. [Kit]
                         asml.Remove(hp2);
                         hp2.Free;
-                        p:=hp1;
                       end;
                   end
                 else if (hp1.typ = ait_instruction) and
@@ -1578,11 +1573,11 @@
                         { we cannot eliminate the first move if
                           the operations uses the same register for source and dest }
                         if not(OpsEqual(taicpu(hp1).oper[1]^,taicpu(hp1).oper[0]^)) then
-                          RemoveCurrentP(p);
+                          RemoveCurrentP(p, nil);
+                        p:=hp1;
                         taicpu(hp1).loadoper(1, taicpu(hp2).oper[1]^);
                         asml.remove(hp2);
                         hp2.Free;
-                        p:=hp1;
                         result:=true;
                       end;
                   end;
@@ -1843,8 +1838,7 @@
           begin
             DebugMsg(SPeepholeOptimization + 'Mov2Nop 1 done',p);
             { take care of the register (de)allocs following p }
-            RemoveCurrentP(p);
-            p:=hp1;
+            RemoveCurrentP(p, hp1);
             Result:=true;
             exit;
           end;
@@ -2072,7 +2066,7 @@
                           begin
                             DebugMsg(SPeepholeOptimization + 'MovMovXX2MovXX 1 done',p);
                             taicpu(hp1).loadref(0,taicpu(p).oper[0]^.ref^);
-                            RemoveCurrentP(p);
+                            RemoveCurrentP(p, hp1);
                             Result:=True;
                             Exit;
                           end;
@@ -2088,8 +2082,7 @@
               else
                 begin
                   DebugMsg(SPeepholeOptimization + 'Mov2Nop 5 done',p);
-                  RemoveCurrentP(p);
-                  p:=hp1;
+                  RemoveCurrentP(p, hp1);
                   Result := True;
                   Exit;
                 end;
@@ -2567,8 +2560,7 @@
 
                           { We can remove the original MOV too }
                           DebugMsg(SPeepholeOptimization + 'MovMov2NopNop 6b done',p);
-                          RemoveCurrentP(p);
-                          p:=hp1;
+                          RemoveCurrentP(p, hp1);
                           Result:=true;
                           Exit;
                         end;
@@ -2589,8 +2581,7 @@
                       else
                         begin
                           DebugMsg(SPeepholeOptimization + 'MovMov2Mov 6 done',p);
-                          RemoveCurrentP(p);
-                          p:=hp1;
+                          RemoveCurrentP(p, hp1);
                           Result:=true;
                           Exit;
                         end;
@@ -2625,8 +2616,7 @@
                         else
                           begin
                             DebugMsg(SPeepholeOptimization + 'MovMov2Mov 7 done',p);
-                            RemoveCurrentP(p);
-                            p:=hp1;
+                            RemoveCurrentP(p, hp1);
                             Result:=true;
                             Exit;
                           end;
@@ -2786,7 +2776,7 @@
                     }
                     asml.remove(hp2);
                     hp2.Free;
-                    RemoveCurrentP(p);
+                    RemoveCurrentP(p, hp1);
                     Result:=True;
                     Exit;
                   end;
@@ -3222,7 +3212,7 @@
                             if not(taicpu(p).oper[0]^.ref^.scalefactor in [0,1]) then
                               taicpu(hp1).oper[ref]^.ref^.scalefactor:=taicpu(p).oper[0]^.ref^.scalefactor;
                             inc(taicpu(hp1).oper[ref]^.ref^.offset,taicpu(p).oper[0]^.ref^.offset);
-                            RemoveCurrentP(p);
+                            RemoveCurrentP(p, hp1);
                             result:=true;
                             exit;
                           end
@@ -4808,8 +4798,7 @@
                           Taicpu(hp1).loadoper(1,Taicpu(hp1).oper[0]^);
                         Taicpu(hp1).loadconst(0,0);
                         Taicpu(hp1).opcode:=carryadd_opcode;
-                        RemoveCurrentP(p);
-                        p:=hp1;
+                        RemoveCurrentP(p, hp1);
                         result:=true;
                         exit;
                       end;
@@ -5490,8 +5479,7 @@
                   ((MaskLength+taicpu(hp1).oper[0]^.val)>=topsize2memsize[taicpu(hp1).opsize]) then
                   begin
                     DebugMsg(SPeepholeOptimization + 'AndShlToShl done',p);
-                    RemoveCurrentP(p);
-                    p:=hp1;
+                    RemoveCurrentP(p, hp1);
                     Result:=true;
                     exit;
                   end;
@@ -5713,7 +5701,7 @@
             taicpu(hp1).opcode := A_JMP;
             taicpu(hp1).is_jmp := true;
             DebugMsg(SPeepholeOptimization + 'LeaCallLeaRet2Jmp done',p);
-            RemoveCurrentP(p);
+            RemoveCurrentP(p, hp1);
             AsmL.Remove(hp2);
             hp2.free;
             AsmL.Remove(hp3);
removecurrentp_refactor.patch (12,265 bytes)   

J. Gareth Moreton

2020-04-24 11:47

developer   ~0122376

Is there a big with this or is it simply a low priority?

Florian

2020-04-26 15:25

administrator   ~0122449

Sorry, I forgot to comment: currently, the patch makes the compiler miss a few optimization opportunities: just compare the assembly of the system unit before and after the patch: some mov* $0,... are not changed into xor* ...

J. Gareth Moreton

2020-04-26 19:50

developer   ~0122455

Ah, fair point! I'll have a look at those - thanks.

J. Gareth Moreton

2020-04-26 22:46

developer   ~0122462

Apologies Florian - on which platform and settings do the missed optimisations occur? So far, for me, on x86_64-win64, system.s is identical for both -O2 and -O4.

J. Gareth Moreton

2020-04-27 12:02

developer   ~0122468

I haven't been able to reproduce it on Windows, but given your description of the fault, I think I found the cause of the bug - it was to do with the presence of RemoveCurrentp in PostPeepholeOptLea - hp1 skips over a few instructions sometimes, and I'm guessing it skips over the instructions that set parameters stored in registers, therefore if any are set to zero, the post peephole optimizer misses them due to p being set to hp1, which was originally the CALL instruction (now JMP).
removecurrentp_refactor_2.patch (12,838 bytes)   
Index: compiler/aoptobj.pas
===================================================================
--- compiler/aoptobj.pas	(revision 45063)
+++ compiler/aoptobj.pas	(working copy)
@@ -341,6 +341,9 @@
         { removes p from asml, updates registers and replaces it by a valid value, if this is the case true is returned }
         function RemoveCurrentP(var p : tai): boolean;
 
+        { removes p from asml, updates registers and replaces p with hp1 (if the next instruction was known beforehand) }
+        procedure RemoveCurrentP(var p: tai; const hp1: tai); inline;
+
        { traces sucessive jumps to their final destination and sets it, e.g.
          je l1                je l3
          <code>               <code>
@@ -1498,6 +1501,15 @@
       end;
 
 
+    procedure TAOptObj.RemoveCurrentP(var p: tai; const hp1: tai); inline;
+      begin
+        UpdateUsedRegs(tai(p.Next));
+        AsmL.Remove(p);
+        p.Free;
+        p := hp1;
+      end;
+
+
     function FindLiveLabel(hp: tai; var l: tasmlabel): Boolean;
       var
         next: tai;
Index: compiler/arm/aoptcpu.pas
===================================================================
--- compiler/arm/aoptcpu.pas	(revision 45063)
+++ compiler/arm/aoptcpu.pas	(working copy)
@@ -1518,9 +1518,7 @@
                             DebugMsg('Peephole AndStrb2Strb done', p);
                             taicpu(hp1).loadReg(0,taicpu(p).oper[1]^.reg);
                             AllocRegBetween(taicpu(p).oper[1]^.reg,p,hp1,UsedRegs);
-                            GetNextInstruction(p, hp1);
                             RemoveCurrentP(p);
-                            p:=hp1;
                             result:=true;
                           end
                         {
@@ -1792,8 +1790,7 @@
                             AllocRegBetween(taicpu(hp1).oper[3]^.reg,p,hp1,UsedRegs);
 
                             taicpu(hp1).ops:=4;
-                            RemoveCurrentP(p);
-                            p:=hp1;
+                            RemoveCurrentP(p, hp1); // <-- Is this actually safe? hp1 is not necessarily the next instruction. [Kit]
                           end;
 
                         result:=true;
Index: compiler/avr/aoptcpu.pas
===================================================================
--- compiler/avr/aoptcpu.pas	(revision 45063)
+++ compiler/avr/aoptcpu.pas	(working copy)
@@ -679,7 +679,8 @@
                     begin
                       DebugMsg('Peephole AddAdc2Add performed', p);
 
-                      result:=RemoveCurrentP(p);
+                      RemoveCurrentP(p, hp1);
+                      Result := True;
                     end;
                   end;
                 A_SUB:
@@ -692,7 +693,8 @@
 
                       taicpu(hp1).opcode:=A_SUB;
 
-                      result:=RemoveCurrentP(p);
+                      RemoveCurrentP(p, hp1);
+                      Result := True;
                     end;
                   end;
                 A_CLR:
@@ -795,9 +797,23 @@
 
                            taicpu(hp3).loadreg(1, taicpu(p).oper[0]^.reg);
 
-                           RemoveCurrentP(p);
-                           RemoveCurrentP(p);
-                           result:=RemoveCurrentP(p);
+                           { We're removing 3 concurrent instructions.  Remove hp1
+                             and hp2 manually instead of calling RemoveCurrentP
+                             as this means we won't be calling UpdateUsedRegs 3 times }
+                           asml.Remove(hp1);
+                           hp1.Free;
+
+                           asml.Remove(hp2);
+                           hp2.Free;
+
+                           { By removing p last, we've guaranteed that p.Next is
+                             valid (storing it prior to removing the instructions
+                             may result in a dangling pointer if hp1 immediately
+                             follows p), and because hp1, hp2 and hp3 came from
+                             sequential calls to GetNextInstruction, it is
+                             guaranteed that UpdateUsedRegs will stop at hp3. [Kit] }
+                           RemoveCurrentP(p, hp3);
+                           Result := True;
                          end
                        else
                          begin
@@ -883,7 +899,8 @@
                           not(MatchInstruction(hp1,[A_CALL,A_RCALL])) then
                           begin
                             DebugMsg('Peephole Mov2Nop performed', p);
-                            result:=RemoveCurrentP(p);
+                            RemoveCurrentP(p, hp1);
+                            Result := True;
                             exit;
                           end;
                       end;
@@ -1092,7 +1109,8 @@
                         begin
                           DebugMsg('Peephole MovMov2Mov performed', p);
 
-                          result:=RemoveCurrentP(p);
+                          RemoveCurrentP(p,hp1);
+                          Result := True;
 
                           GetNextInstruction(hp1,hp1);
                           if not assigned(hp1) then
Index: compiler/x86/aoptx86.pas
===================================================================
--- compiler/x86/aoptx86.pas	(revision 45063)
+++ compiler/x86/aoptx86.pas	(working copy)
@@ -1085,10 +1085,8 @@
             if (taicpu(p).ops = 2) then
              { remove "imul $1, reg" }
               begin
-                hp1 := tai(p.Next);
                 DebugMsg(SPeepholeOptimization + 'Imul2Nop done',p);
-                RemoveCurrentP(p);
-                result:=true;
+                Result := RemoveCurrentP(p);
               end
             else
              { change "imul $1, reg1, reg2" to "mov reg1, reg2" }
@@ -1138,7 +1136,7 @@
                   AsmL.InsertAfter(hp1,p);
                   DebugMsg(SPeepholeOptimization + 'Imul2LeaShl done',p);
                   taicpu(hp1).fileinfo:=taicpu(p).fileinfo;
-                  RemoveCurrentP(p);
+                  RemoveCurrentP(p, hp1);
                   if ShiftValue>0 then
                     AsmL.InsertAfter(taicpu.op_const_reg(A_SHL, opsize, ShiftValue, taicpu(hp1).oper[1]^.reg),hp1);
               end;
@@ -1395,9 +1393,7 @@
               <nop> }
             if MatchOperand(taicpu(p).oper[0]^,taicpu(p).oper[1]^) then
               begin
-                GetNextInstruction(p,hp1);
                 RemoveCurrentP(p);
-                p:=hp1;
                 result:=true;
                 exit;
               end
@@ -1539,10 +1535,9 @@
                     if not(RegUsedAfterInstruction(taicpu(p).oper[1]^.reg,hp2,TmpUsedRegs)) then
                       begin
                         taicpu(hp1).loadoper(2,taicpu(p).oper[0]^);
-                        RemoveCurrentP(p);
+                        RemoveCurrentP(p, hp1); // <-- Is this actually safe? hp1 is not necessarily the next instruction. [Kit]
                         asml.Remove(hp2);
                         hp2.Free;
-                        p:=hp1;
                       end;
                   end
                 else if (hp1.typ = ait_instruction) and
@@ -1578,11 +1573,11 @@
                         { we cannot eliminate the first move if
                           the operations uses the same register for source and dest }
                         if not(OpsEqual(taicpu(hp1).oper[1]^,taicpu(hp1).oper[0]^)) then
-                          RemoveCurrentP(p);
+                          RemoveCurrentP(p, nil);
+                        p:=hp1;
                         taicpu(hp1).loadoper(1, taicpu(hp2).oper[1]^);
                         asml.remove(hp2);
                         hp2.Free;
-                        p:=hp1;
                         result:=true;
                       end;
                   end;
@@ -1843,8 +1838,7 @@
           begin
             DebugMsg(SPeepholeOptimization + 'Mov2Nop 1 done',p);
             { take care of the register (de)allocs following p }
-            RemoveCurrentP(p);
-            p:=hp1;
+            RemoveCurrentP(p, hp1);
             Result:=true;
             exit;
           end;
@@ -2072,7 +2066,7 @@
                           begin
                             DebugMsg(SPeepholeOptimization + 'MovMovXX2MovXX 1 done',p);
                             taicpu(hp1).loadref(0,taicpu(p).oper[0]^.ref^);
-                            RemoveCurrentP(p);
+                            RemoveCurrentP(p, hp1);
                             Result:=True;
                             Exit;
                           end;
@@ -2088,8 +2082,7 @@
               else
                 begin
                   DebugMsg(SPeepholeOptimization + 'Mov2Nop 5 done',p);
-                  RemoveCurrentP(p);
-                  p:=hp1;
+                  RemoveCurrentP(p, hp1);
                   Result := True;
                   Exit;
                 end;
@@ -2567,8 +2560,7 @@
 
                           { We can remove the original MOV too }
                           DebugMsg(SPeepholeOptimization + 'MovMov2NopNop 6b done',p);
-                          RemoveCurrentP(p);
-                          p:=hp1;
+                          RemoveCurrentP(p, hp1);
                           Result:=true;
                           Exit;
                         end;
@@ -2589,8 +2581,7 @@
                       else
                         begin
                           DebugMsg(SPeepholeOptimization + 'MovMov2Mov 6 done',p);
-                          RemoveCurrentP(p);
-                          p:=hp1;
+                          RemoveCurrentP(p, hp1);
                           Result:=true;
                           Exit;
                         end;
@@ -2625,8 +2616,7 @@
                         else
                           begin
                             DebugMsg(SPeepholeOptimization + 'MovMov2Mov 7 done',p);
-                            RemoveCurrentP(p);
-                            p:=hp1;
+                            RemoveCurrentP(p, hp1);
                             Result:=true;
                             Exit;
                           end;
@@ -2786,7 +2776,7 @@
                     }
                     asml.remove(hp2);
                     hp2.Free;
-                    RemoveCurrentP(p);
+                    RemoveCurrentP(p, hp1);
                     Result:=True;
                     Exit;
                   end;
@@ -3222,7 +3212,7 @@
                             if not(taicpu(p).oper[0]^.ref^.scalefactor in [0,1]) then
                               taicpu(hp1).oper[ref]^.ref^.scalefactor:=taicpu(p).oper[0]^.ref^.scalefactor;
                             inc(taicpu(hp1).oper[ref]^.ref^.offset,taicpu(p).oper[0]^.ref^.offset);
-                            RemoveCurrentP(p);
+                            RemoveCurrentP(p, hp1);
                             result:=true;
                             exit;
                           end
@@ -4808,8 +4798,7 @@
                           Taicpu(hp1).loadoper(1,Taicpu(hp1).oper[0]^);
                         Taicpu(hp1).loadconst(0,0);
                         Taicpu(hp1).opcode:=carryadd_opcode;
-                        RemoveCurrentP(p);
-                        p:=hp1;
+                        RemoveCurrentP(p, hp1);
                         result:=true;
                         exit;
                       end;
@@ -5524,8 +5513,7 @@
                   ((MaskLength+taicpu(hp1).oper[0]^.val)>=topsize2memsize[taicpu(hp1).opsize]) then
                   begin
                     DebugMsg(SPeepholeOptimization + 'AndShlToShl done',p);
-                    RemoveCurrentP(p);
-                    p:=hp1;
+                    RemoveCurrentP(p, hp1);
                     Result:=true;
                     exit;
                   end;
@@ -5698,7 +5686,7 @@
         end;
 
       var
-        hp1, hp2, hp3: tai;
+        hp1, hp2, hp3, hp4: tai;
       begin
         Result:=false;
         { replace
@@ -5724,6 +5712,8 @@
           (taicpu(p).oper[0]^.ref^.segment=NR_NO) and
           (taicpu(p).oper[1]^.reg=NR_STACK_POINTER_REG) and
           GetNextInstruction(p, hp1) and
+          { Take a copy of hp1 }
+          SetAndTest(hp1, hp4) and
           { trick to skip label }
           ((hp1.typ=ait_instruction) or GetNextInstruction(hp1, hp1)) and
           SkipSimpleInstructions(hp1) and
@@ -5747,7 +5737,7 @@
             taicpu(hp1).opcode := A_JMP;
             taicpu(hp1).is_jmp := true;
             DebugMsg(SPeepholeOptimization + 'LeaCallLeaRet2Jmp done',p);
-            RemoveCurrentP(p);
+            RemoveCurrentP(p, hp4);
             AsmL.Remove(hp2);
             hp2.free;
             AsmL.Remove(hp3);
removecurrentp_refactor_2.patch (12,838 bytes)   

Florian

2020-04-27 21:28

administrator   ~0122473

Thanks, now it worked, applied.

Issue History

Date Modified Username Field Change
2020-03-16 02:41 J. Gareth Moreton New Issue
2020-03-16 02:41 J. Gareth Moreton File Added: removecurrentp_refactor.patch
2020-03-16 02:54 J. Gareth Moreton Tag Attached: patch
2020-03-16 02:54 J. Gareth Moreton Tag Attached: compiler
2020-03-16 02:54 J. Gareth Moreton Tag Attached: optimizations
2020-03-16 02:54 J. Gareth Moreton Tag Attached: refactor
2020-03-16 02:55 J. Gareth Moreton Priority normal => low
2020-03-16 02:55 J. Gareth Moreton Severity minor => tweak
2020-03-16 02:55 J. Gareth Moreton FPCTarget => -
2020-04-24 11:47 J. Gareth Moreton Note Added: 0122376
2020-04-26 15:25 Florian Note Added: 0122449
2020-04-26 19:50 J. Gareth Moreton Note Added: 0122455
2020-04-26 22:46 J. Gareth Moreton Note Added: 0122462
2020-04-27 12:02 J. Gareth Moreton Note Added: 0122468
2020-04-27 12:02 J. Gareth Moreton File Added: removecurrentp_refactor_2.patch
2020-04-27 21:28 Florian Assigned To => Florian
2020-04-27 21:28 Florian Status new => resolved
2020-04-27 21:28 Florian Resolution open => fixed
2020-04-27 21:28 Florian Fixed in Version => 3.3.1
2020-04-27 21:28 Florian Fixed in Revision => 45142
2020-04-27 21:28 Florian Note Added: 0122473