View Issue Details

IDProjectCategoryView StatusLast Update
0036309FPCCompilerpublic2019-11-21 08:51
ReporterKarl-Michael SchindlerAssigned ToFlorian 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
PlatformdarwinOSMac OS XOS Version10.5, 10.6
Product Version3.3.1Product Build43441 
Target VersionFixed in Version3.3.1 
Summary0036309: commits 43439-43441 break building some targets on x86_64-darwin
DescriptionAfter commits 43439-34341 building all fails for the following targets on x86_64-darwin:
mips-linux, mipsel-linux, x86_64-darwin, and arm-embedded-armv6m. Build all on many other targets (>20) works.

Details:

Building mips-linux and mipsel-linux ends up in an endless loop after the following command:
make all OPT=-ap OS_TARGET=linux CPU_TARGET=mips
...
/BlaBla/FreePascal/compiler/ppcrossmips -Ur -Tlinux -Pmips -XPmips-linux- -Xr -Ur -Xs -O2 -n -Fi../inc -Fi../mips -Fi../unix -Fimips -FE. -FU/BlaBla/FreePascal/rtl/units/mips-linux -Cg -ap -dmips -dRELEASE -Us -Sg system.pp

Building arm-embedded-armv6m fails with:
make all OPT=-ap OS_TARGET=embedded CPU_TARGET=arm SUBARCH=armv6m
...
/sw/bin/gmkdir -p /BlaBla/FreePascal/rtl/units/arm-embedded
/BlaBla/FreePascal/compiler/ppcrossarm -Cparmv6m -Ur -Tembedded -Parm -XParm-embedded- -Xr -Ur -Xs -O2 -n -Fi../inc -Fi../arm -FE. -FU/BlaBla/FreePascal/rtl/units/arm-embedded -ap -darm -dRELEASE -Us -Sg system.pp @system.cfg
{standard input}: Assembler messages:
{standard input}:5095: Error: branch out of range
{standard input}:6135: Error: branch out of range
{standard input}:6232: Error: branch out of range
...
{standard input}:71580: Error: branch out of range
{standard input}:83535: Error: branch out of range
{standard input}:84460: Error: branch out of range
system.pp(336) Fatal: There were 1 errors compiling module, stopping
Fatal: Compilation aborted
make[3]: *** [Makefile:2818: system.ppu] Fehler 1
make[3]: Verzeichnis „/BlaBla/FreePascal/rtl/embedded“ wird verlassen
make[2]: *** [Makefile:2787: embedded_all] Fehler 2
make[2]: Verzeichnis „/BlaBla/FreePascal/rtl“ wird verlassen
make[1]: *** [Makefile:2641: rtl_all] Fehler 2
make[1]: Verzeichnis „/BlaBla/FreePascal“ wird verlassen
make: *** [Makefile:2889: build-stamp.arm-embedded] Fehler 2

Building x86_64-darwin fails at packages with:
make all OPT=-ap CPU_TARGET=x86_64 OS_TARGET=darwin
...
Start compiling package xforms for target x86_64-darwin.
       Compiling xforms/BuildUnit_xforms.pp
       Compiling ./xforms/src/xforms.pp
External command "/BlaBla/FreePascal/compiler/ppcx64 -Tdarwin -FUxforms/units/x86_64-darwin/ -Fu/BlaBla/FreePascal/rtl/units/x86_64-darwin/ -Fu/BlaBla/FreePascal/packages/x11/units/x86_64-darwin/ -Fuxforms/src -Fixforms/src -Ur -Xs -O2 -n -ap -v0 -Fl/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/lib -dx86_64 -dRELEASE -XX -CX -Sc -viq xforms/BuildUnit_xforms.pp" failed with exit code 256. Console output:
Target OS: Darwin for x86_64
Compiling xforms/BuildUnit_xforms.pp
Compiling ./xforms/src/xforms.pp
Assembling (pipe) xforms/units/x86_64-darwin/xforms.s
xforms.pp(2910) Fatal: There were 1 errors compiling module, stopping
Fatal: Compilation aborted
<unknown>:0: error: Unfinished frame!

The installer encountered the following error:
Compilation of "BuildUnit_xforms.pp" failed
make[2]: *** [Makefile:1742: smart] Fehler 1
make[2]: Verzeichnis „/Volumes/Reserve/Developer/FreePascal/packages“ wird verlassen
make[1]: *** [Makefile:2735: packages_smart] Fehler 2
make[1]: Verzeichnis „/Volumes/Reserve/Developer/FreePascal“ wird verlassen
make: *** [Makefile:2890: build-stamp.x86_64-darwin] Fehler 2
Additional InformationNewer commits up to 43457 do not change it.
TagsNo tags attached.
Fixed in Revisionr43521
FPCOldBugId
FPCTarget-
Attached Files
  • jump-optimizations-can-do-check.patch (4,078 bytes)
    Index: compiler/aoptobj.pas
    ===================================================================
    --- compiler/aoptobj.pas	(revision 43510)
    +++ compiler/aoptobj.pas	(working copy)
    @@ -397,6 +397,9 @@
             function OptimizeConditionalJump(CJLabel: TAsmLabel; var p: tai; hp1: tai; var stoploop: Boolean): Boolean;
     {$endif JVM}
     
    +        { Function to determine if the jump optimisations can be performed }
    +        function CanDoJumpOpts: Boolean; virtual;
    +
             { Jump/label optimisation entry method }
             function DoJumpOptimizations(var p: tai; var stoploop: Boolean): Boolean;
     
    @@ -2091,6 +2094,13 @@
         end;
     
     
    +    function TAOptObj.CanDoJumpOpts: Boolean;
    +      begin
    +        { Always allow by default }
    +        Result := True;
    +      end;
    +
    +
         function TAOptObj.DoJumpOptimizations(var p: tai; var stoploop: Boolean): Boolean;
           var
             hp1, hp2: tai;
    @@ -2342,8 +2352,10 @@
         procedure TAOptObj.PeepHoleOptPass1;
           var
             p,hp1,hp2,hp3 : tai;
    -        stoploop, FirstInstruction: boolean;
    +        stoploop, FirstInstruction, JumpOptsAvailable: boolean;
           begin
    +        JumpOptsAvailable := CanDoJumpOpts();
    +
             StartPoint := BlockStart;
     
             repeat
    @@ -2365,25 +2377,19 @@
                     InsertLLItem(tai(p.Previous),p,tai_comment.create(strpnew(GetAllocationString(UsedRegs))));
     {$endif DEBUG_OPTALLOC}
     
    -              { Handle Jmp Optimizations first }
    -{$if defined(ARM)}
    -              { Cannot perform these jump optimisations if the ARM architecture has 16-bit thumb codes }
    -              if not (
    -                (current_settings.instructionset = is_thumb) and not(CPUARM_HAS_THUMB2 in cpu_capabilities[current_settings.cputype])
    -              ) then
    -{$endif defined(ARM)}
    -                if DoJumpOptimizations(p, stoploop) then
    -                  begin
    -                    UpdateUsedRegs(p);
    -                    if FirstInstruction then
    -                      { Update StartPoint, since the old p was removed;
    -                        don't set FirstInstruction to False though, as
    -                        the new p might get removed too. }
    -                      StartPoint := p;
    +              { Handle jump optimizations first }
    +              if JumpOptsAvailable and DoJumpOptimizations(p, stoploop) then
    +                begin
    +                  UpdateUsedRegs(p);
    +                  if FirstInstruction then
    +                    { Update StartPoint, since the old p was removed;
    +                      don't set FirstInstruction to False though, as
    +                      the new p might get removed too. }
    +                    StartPoint := p;
     
    -                    if (p.typ = ait_instruction) and IsJumpToLabel(taicpu(p)) then
    -                      Continue;
    -                  end;
    +                  if (p.typ = ait_instruction) and IsJumpToLabel(taicpu(p)) then
    +                    Continue;
    +                end;
     
                   if PeepHoleOptPass1Cpu(p) then
                     begin
    Index: compiler/arm/aoptcpu.pas
    ===================================================================
    --- compiler/arm/aoptcpu.pas	(revision 43510)
    +++ compiler/arm/aoptcpu.pas	(working copy)
    @@ -34,6 +34,9 @@
     
     Type
       TCpuAsmOptimizer = class(TAsmOptimizer)
    +    { Can't be done in some cases due to the limited range of jumps }
    +    function CanDoJumpOpts: Boolean; override;
    +
         { uses the same constructor as TAopObj }
         function PeepHoleOptPass1Cpu(var p: tai): boolean; override;
         procedure PeepHoleOptPass2;override;
    @@ -379,6 +382,16 @@
         end;
     {$endif DEBUG_AOPTCPU}
     
    +
    +  function TCpuAsmOptimizer.CanDoJumpOpts: Boolean;
    +    begin
    +      { Cannot perform these jump optimisations if the ARM architecture has 16-bit thumb codes }
    +      Result := not (
    +        (current_settings.instructionset = is_thumb) and not (CPUARM_HAS_THUMB2 in cpu_capabilities[current_settings.cputype])
    +      );
    +    end;
    +
    +
       function TCpuAsmOptimizer.RemoveSuperfluousMove(const p: tai; movp: tai; const optimizer: string):boolean;
         var
           alloc,
    
  • jump-optimizations-mips-bug-fix.patch (5,146 bytes)
    Index: compiler/aoptobj.pas
    ===================================================================
    --- compiler/aoptobj.pas	(revision 43513)
    +++ compiler/aoptobj.pas	(working copy)
    @@ -375,6 +375,10 @@
             { Output debug message to console - null function if EXTDEBUG is not defined }
             class procedure DebugWrite(Message: string); static; inline;
     
    +        { Converts a conditional jump into an unconditional jump.  Only call this
    +          procedure on an instruction that you already know is a conditional jump }
    +        procedure MakeUnconditional(p: taicpu); virtual;
    +
             { Removes all instructions between an unconditional jump and the next label }
             procedure RemoveDeadCodeAfterJump(p: taicpu);
     
    @@ -1564,6 +1571,25 @@
     {$endif DEBUG_JUMP}
           end;
     
    +
    +    { Converts a conditional jump into an unconditional jump.  Only call this
    +      procedure on an instruction that you already know is a conditional jump }
    +    procedure TAOptObj.MakeUnconditional(p: taicpu);
    +      begin
    +        { TODO: If anyone can improve this particular optimisation to work on
    +          AVR and RISC-V, please do (it's currently not called at all). [Kit] }
    +{$if not defined(avr) and not defined(riscv32) and not defined(riscv64)}
    +{$if defined(powerpc) or defined(powerpc64)}
    +        p.condition.cond := C_None;
    +        p.condition.simple := True;
    +{$else powerpc}
    +        p.condition := C_None;
    +{$endif powerpc}
    +        p.opcode := aopt_uncondjmp;
    +{$endif not avr and not riscv}
    +      end;
    +
    +
         { Removes all instructions between an unconditional jump and the next label }
         procedure TAOptObj.RemoveDeadCodeAfterJump(p: taicpu);
           var
    @@ -2014,16 +2040,13 @@
                                 Result := True;
                                 Exit;
     
    -{$if not defined(avr) and not defined(riscv32) and not defined(riscv64) and not defined(mips)}
    +{$if not defined(avr) and not defined(riscv32) and not defined(riscv64)}
                               end
                             else
                               { NOTE: There is currently no watertight, cross-platform way to create
                                 an unconditional jump without access to the cg object.  If anyone can
                                 improve this particular optimisation to work on AVR and RISC-V,
    -                            please do. [Kit]
    -
    -                            On MIPS, it causes an endless loop, so I disabled it for now
    -                          }
    +                            please do. [Kit] }
                               begin
                                 { Since cond1 is a subset of inv(cond2), jmp<cond2> will always branch if
                                   jmp<cond1> does not, so change jmp<cond2> to an unconditional jump. }
    @@ -2030,13 +2053,7 @@
     
                                 DebugWrite('JUMP DEBUG: jmp<cond> before jmp<inv_cond> - made second jump unconditional');
     
    -{$if defined(powerpc) or defined(powerpc64)}
    -                            taicpu(hp2).condition.cond := C_None;
    -                            taicpu(hp2).condition.simple := True;
    -{$else powerpc}
    -                            taicpu(hp2).condition := C_None;
    -{$endif powerpc}
    -                            taicpu(hp2).opcode := aopt_uncondjmp;
    +                            MakeUnconditional(taicpu(hp1));
     
                                 { NOTE: Changing the jump to unconditional won't open up new opportunities
                                   for GetFinalDestination on earlier jumps because there's no live label
    Index: compiler/mips/aoptcpu.pas
    ===================================================================
    --- compiler/mips/aoptcpu.pas	(revision 43513)
    +++ compiler/mips/aoptcpu.pas	(working copy)
    @@ -25,7 +25,7 @@
     
     {$i fpcdefs.inc}
     
    -{$define DEBUG_AOPTCPU}
    +{ $define DEBUG_AOPTCPU}
     
       Interface
     
    @@ -36,6 +36,10 @@
           TAsmOpSet = set of TAsmOp;
     
           TCpuAsmOptimizer = class(TAsmOptimizer)
    +        { Converts a conditional jump into an unconditional jump.  Only call this
    +          procedure on an instruction that you already know is a conditional jump }
    +        procedure MakeUnconditional(p: taicpu); override;
    +
             function RegModifiedByInstruction(Reg: TRegister; p1: tai): boolean; override;
             function GetNextInstructionUsingReg(Current: tai;
               var Next: tai; reg: TRegister): Boolean;
    @@ -147,6 +151,28 @@
     {$endif DEBUG_AOPTCPU}
     
     
    +  { Converts a conditional jump into an unconditional jump.  Only call this
    +    procedure on an instruction that you already know is a conditional jump }
    +  procedure TCpuAsmOptimizer.MakeUnconditional(p: taicpu);
    +    var
    +      idx, topidx: Byte;
    +    begin
    +      inherited MakeUnconditional(p);
    +
    +      topidx := p.ops-1;
    +      if topidx = 0 then
    +        Exit;
    +
    +      { Move destination address into first register, then delete the rest }
    +      p.loadoper(0, p.oper[topidx]^);
    +      for idx := topidx downto 1 do
    +        p.freeop(idx);
    +      p.ops := 1;
    +      p.opercnt := 1;
    +
    +    end;
    +
    +
      function TCpuAsmOptimizer.InstructionLoadsFromReg(const reg: TRegister; const hp: tai): boolean;
         var
           p: taicpu;
    

Activities

Karl-Michael Schindler

2019-11-14 11:52

reporter   ~0119294

Update: svn commit 43462 resolves the error for the target x86_64-darwin, but not the other ones.

Karl-Michael Schindler

2019-11-14 11:56

reporter   ~0119295

Can someone check on other platforms, whether the endless loop for mips(el)-linux and the "branch out of range"-problem with arm-embedded-armv6m show up, too?

Karl-Michael Schindler

2019-11-14 12:04

reporter   ~0119297

Maybe this helps: with -va the log for mips-linux becomes:
...
[0.215] generic.inc(771,9) Handling switch "$IFNDEF"
[0.215] generic.inc(771,9) IFNDEF FPC_SYSTEM_HAS_FPC_HELP_CONSTRUCTOR found, accepted
[0.215] generic.inc(775,1) procedure/function $fpc_help_constructor(Pointer;var Pointer;LongWord):^untyped;
800 6.949/8.608 Kb Used
[0.215] generic.inc(800,8) Handling switch "$ENDIF"
[0.215] generic.inc(800,8) ENDIF FPC_SYSTEM_HAS_FPC_HELP_CONSTRUCTOR found
[0.215] generic.inc(803,9) Handling switch "$IFNDEF"
[0.215] generic.inc(803,9) IFNDEF FPC_SYSTEM_HAS_FPC_HELP_DESTRUCTOR found, accepted
[0.215] generic.inc(806,1) procedure/function $fpc_help_destructor(Pointer;Pointer;LongWord);
[0.215] generic.inc(819,8) Handling switch "$ENDIF"
[0.215] generic.inc(819,8) ENDIF FPC_SYSTEM_HAS_FPC_HELP_DESTRUCTOR found
[0.215] generic.inc(822,9) Handling switch "$IFNDEF"
[0.215] generic.inc(822,9) IFNDEF FPC_SYSTEM_HAS_FPC_HELP_FAIL found, accepted
[0.219] generic.inc(825,1) procedure/function $fpc_help_fail(Pointer;var Pointer;LongWord);
[0.219] generic.inc(829,7) Hint: Conversion between ordinals and pointers is not portable

Florian

2019-11-14 22:39

administrator   ~0119305

Can please check with r43466 or higher?

Karl-Michael Schindler

2019-11-14 22:55

reporter   ~0119306

I can confirm that commit 43466 fixes the "branch out of range"-problem with arm-embedded-armv6m.
Only the endless loops of mips and mipsel remain.

J. Gareth Moreton

2019-11-15 00:35

developer   ~0119307

Erp... I hope they're not related to the jump optimisations. Does -va give any clues?

Karl-Michael Schindler

2019-11-15 10:30

reporter   ~0119308

Sorry to disappoint you. The endless loops for mips/mipsel occur definitely after the commits 43439-43441 with ppccrossmips when it compiles system.pp. At least for me -va does not give any further clue, than what i noted above. Would it be helpful to paste the complete log of -va to pastebin?

J. Gareth Moreton

2019-11-15 16:17

developer   ~0119320

Absolutely. Please do. It might be a while lot of nothing, but it might reveal something too.

Florian

2019-11-15 21:52

administrator   ~0119329

I applied a temporary fix in r43486. Gareth, can you please check what's wrong on MIPS? It ends in an endless loops in TAOptObj.OptimizeConditionalJump It appears when compiling the system unit for mipsel-linux and can be reproduced when debugging with lazarus.

Florian

2019-11-15 22:02

administrator   ~0119330

Gareth, another thing: Compiling with -Os on i386-linux generates jna *_$RAUTILS$_Ld4(,%edx,4) ...

J. Gareth Moreton

2019-11-16 00:36

developer   ~0119334

Sure thing, I'll see what I can do. Sorry for all this trouble.

J. Gareth Moreton

2019-11-18 03:44

developer   ~0119376

Okay, I've managed to get an i386 build to work on my Ubuntu virtual machine, and I've reproduced the [J mem??] bug, but it's not due to my jump optimisations. I completely commented them out and the error still occured, meaning that something else is causing it. The file and line number point to the 'case' line in the following method in compiler/rautils.pas:

Procedure TOperand.SetSize(_size:longint;force:boolean);
begin
  if force or
     ((size = OS_NO) and (_size<=16)) then
   Begin
     case _size of
        1 : size:=OS_8;
        2 : size:=OS_16{ could be S_IS};
        4 : size:=OS_32{ could be S_IL or S_FS};
        8 : size:=OS_64{ could be S_D or S_FL};
       10 : size:=OS_F80;
       16 : size:=OS_128;
     end;
   end;
end;

(Also a side-note... that case block doesn't handle all possible values)

J. Gareth Moreton

2019-11-19 21:58

developer   ~0119405

I believe I have fixed the MIPS bug. It was the silliest error in that I was modifying the wrong instruction in the list.

Since converting a MIPS conditional jump into an unconditional jump requires a bit more than just changing the condition, I have introduced a new method called "MakeUnconditional" that can do platform-specific changes (currently, only MIPS has code that does this, namely removing the registers etc that it's checking).

"jump-optimizations-can-do-check.patch" is something I've shown Florian privately, and is a cleaner solution when it comes to disabling the jump optimisations on specific platforms (e.g. ARM with 16-bit thumbs).

The invalid instruction on i386-linux -Os mode has not been fixed, because it is not caused by the jump optimisations, but something else entirely (see previous post).

jump-optimizations-can-do-check.patch (4,078 bytes)
Index: compiler/aoptobj.pas
===================================================================
--- compiler/aoptobj.pas	(revision 43510)
+++ compiler/aoptobj.pas	(working copy)
@@ -397,6 +397,9 @@
         function OptimizeConditionalJump(CJLabel: TAsmLabel; var p: tai; hp1: tai; var stoploop: Boolean): Boolean;
 {$endif JVM}
 
+        { Function to determine if the jump optimisations can be performed }
+        function CanDoJumpOpts: Boolean; virtual;
+
         { Jump/label optimisation entry method }
         function DoJumpOptimizations(var p: tai; var stoploop: Boolean): Boolean;
 
@@ -2091,6 +2094,13 @@
     end;
 
 
+    function TAOptObj.CanDoJumpOpts: Boolean;
+      begin
+        { Always allow by default }
+        Result := True;
+      end;
+
+
     function TAOptObj.DoJumpOptimizations(var p: tai; var stoploop: Boolean): Boolean;
       var
         hp1, hp2: tai;
@@ -2342,8 +2352,10 @@
     procedure TAOptObj.PeepHoleOptPass1;
       var
         p,hp1,hp2,hp3 : tai;
-        stoploop, FirstInstruction: boolean;
+        stoploop, FirstInstruction, JumpOptsAvailable: boolean;
       begin
+        JumpOptsAvailable := CanDoJumpOpts();
+
         StartPoint := BlockStart;
 
         repeat
@@ -2365,25 +2377,19 @@
                 InsertLLItem(tai(p.Previous),p,tai_comment.create(strpnew(GetAllocationString(UsedRegs))));
 {$endif DEBUG_OPTALLOC}
 
-              { Handle Jmp Optimizations first }
-{$if defined(ARM)}
-              { Cannot perform these jump optimisations if the ARM architecture has 16-bit thumb codes }
-              if not (
-                (current_settings.instructionset = is_thumb) and not(CPUARM_HAS_THUMB2 in cpu_capabilities[current_settings.cputype])
-              ) then
-{$endif defined(ARM)}
-                if DoJumpOptimizations(p, stoploop) then
-                  begin
-                    UpdateUsedRegs(p);
-                    if FirstInstruction then
-                      { Update StartPoint, since the old p was removed;
-                        don't set FirstInstruction to False though, as
-                        the new p might get removed too. }
-                      StartPoint := p;
+              { Handle jump optimizations first }
+              if JumpOptsAvailable and DoJumpOptimizations(p, stoploop) then
+                begin
+                  UpdateUsedRegs(p);
+                  if FirstInstruction then
+                    { Update StartPoint, since the old p was removed;
+                      don't set FirstInstruction to False though, as
+                      the new p might get removed too. }
+                    StartPoint := p;
 
-                    if (p.typ = ait_instruction) and IsJumpToLabel(taicpu(p)) then
-                      Continue;
-                  end;
+                  if (p.typ = ait_instruction) and IsJumpToLabel(taicpu(p)) then
+                    Continue;
+                end;
 
               if PeepHoleOptPass1Cpu(p) then
                 begin
Index: compiler/arm/aoptcpu.pas
===================================================================
--- compiler/arm/aoptcpu.pas	(revision 43510)
+++ compiler/arm/aoptcpu.pas	(working copy)
@@ -34,6 +34,9 @@
 
 Type
   TCpuAsmOptimizer = class(TAsmOptimizer)
+    { Can't be done in some cases due to the limited range of jumps }
+    function CanDoJumpOpts: Boolean; override;
+
     { uses the same constructor as TAopObj }
     function PeepHoleOptPass1Cpu(var p: tai): boolean; override;
     procedure PeepHoleOptPass2;override;
@@ -379,6 +382,16 @@
     end;
 {$endif DEBUG_AOPTCPU}
 
+
+  function TCpuAsmOptimizer.CanDoJumpOpts: Boolean;
+    begin
+      { Cannot perform these jump optimisations if the ARM architecture has 16-bit thumb codes }
+      Result := not (
+        (current_settings.instructionset = is_thumb) and not (CPUARM_HAS_THUMB2 in cpu_capabilities[current_settings.cputype])
+      );
+    end;
+
+
   function TCpuAsmOptimizer.RemoveSuperfluousMove(const p: tai; movp: tai; const optimizer: string):boolean;
     var
       alloc,
jump-optimizations-mips-bug-fix.patch (5,146 bytes)
Index: compiler/aoptobj.pas
===================================================================
--- compiler/aoptobj.pas	(revision 43513)
+++ compiler/aoptobj.pas	(working copy)
@@ -375,6 +375,10 @@
         { Output debug message to console - null function if EXTDEBUG is not defined }
         class procedure DebugWrite(Message: string); static; inline;
 
+        { Converts a conditional jump into an unconditional jump.  Only call this
+          procedure on an instruction that you already know is a conditional jump }
+        procedure MakeUnconditional(p: taicpu); virtual;
+
         { Removes all instructions between an unconditional jump and the next label }
         procedure RemoveDeadCodeAfterJump(p: taicpu);
 
@@ -1564,6 +1571,25 @@
 {$endif DEBUG_JUMP}
       end;
 
+
+    { Converts a conditional jump into an unconditional jump.  Only call this
+      procedure on an instruction that you already know is a conditional jump }
+    procedure TAOptObj.MakeUnconditional(p: taicpu);
+      begin
+        { TODO: If anyone can improve this particular optimisation to work on
+          AVR and RISC-V, please do (it's currently not called at all). [Kit] }
+{$if not defined(avr) and not defined(riscv32) and not defined(riscv64)}
+{$if defined(powerpc) or defined(powerpc64)}
+        p.condition.cond := C_None;
+        p.condition.simple := True;
+{$else powerpc}
+        p.condition := C_None;
+{$endif powerpc}
+        p.opcode := aopt_uncondjmp;
+{$endif not avr and not riscv}
+      end;
+
+
     { Removes all instructions between an unconditional jump and the next label }
     procedure TAOptObj.RemoveDeadCodeAfterJump(p: taicpu);
       var
@@ -2014,16 +2040,13 @@
                             Result := True;
                             Exit;
 
-{$if not defined(avr) and not defined(riscv32) and not defined(riscv64) and not defined(mips)}
+{$if not defined(avr) and not defined(riscv32) and not defined(riscv64)}
                           end
                         else
                           { NOTE: There is currently no watertight, cross-platform way to create
                             an unconditional jump without access to the cg object.  If anyone can
                             improve this particular optimisation to work on AVR and RISC-V,
-                            please do. [Kit]
-
-                            On MIPS, it causes an endless loop, so I disabled it for now
-                          }
+                            please do. [Kit] }
                           begin
                             { Since cond1 is a subset of inv(cond2), jmp<cond2> will always branch if
                               jmp<cond1> does not, so change jmp<cond2> to an unconditional jump. }
@@ -2030,13 +2053,7 @@
 
                             DebugWrite('JUMP DEBUG: jmp<cond> before jmp<inv_cond> - made second jump unconditional');
 
-{$if defined(powerpc) or defined(powerpc64)}
-                            taicpu(hp2).condition.cond := C_None;
-                            taicpu(hp2).condition.simple := True;
-{$else powerpc}
-                            taicpu(hp2).condition := C_None;
-{$endif powerpc}
-                            taicpu(hp2).opcode := aopt_uncondjmp;
+                            MakeUnconditional(taicpu(hp1));
 
                             { NOTE: Changing the jump to unconditional won't open up new opportunities
                               for GetFinalDestination on earlier jumps because there's no live label
Index: compiler/mips/aoptcpu.pas
===================================================================
--- compiler/mips/aoptcpu.pas	(revision 43513)
+++ compiler/mips/aoptcpu.pas	(working copy)
@@ -25,7 +25,7 @@
 
 {$i fpcdefs.inc}
 
-{$define DEBUG_AOPTCPU}
+{ $define DEBUG_AOPTCPU}
 
   Interface
 
@@ -36,6 +36,10 @@
       TAsmOpSet = set of TAsmOp;
 
       TCpuAsmOptimizer = class(TAsmOptimizer)
+        { Converts a conditional jump into an unconditional jump.  Only call this
+          procedure on an instruction that you already know is a conditional jump }
+        procedure MakeUnconditional(p: taicpu); override;
+
         function RegModifiedByInstruction(Reg: TRegister; p1: tai): boolean; override;
         function GetNextInstructionUsingReg(Current: tai;
           var Next: tai; reg: TRegister): Boolean;
@@ -147,6 +151,28 @@
 {$endif DEBUG_AOPTCPU}
 
 
+  { Converts a conditional jump into an unconditional jump.  Only call this
+    procedure on an instruction that you already know is a conditional jump }
+  procedure TCpuAsmOptimizer.MakeUnconditional(p: taicpu);
+    var
+      idx, topidx: Byte;
+    begin
+      inherited MakeUnconditional(p);
+
+      topidx := p.ops-1;
+      if topidx = 0 then
+        Exit;
+
+      { Move destination address into first register, then delete the rest }
+      p.loadoper(0, p.oper[topidx]^);
+      for idx := topidx downto 1 do
+        p.freeop(idx);
+      p.ops := 1;
+      p.opercnt := 1;
+
+    end;
+
+
  function TCpuAsmOptimizer.InstructionLoadsFromReg(const reg: TRegister; const hp: tai): boolean;
     var
       p: taicpu;

Florian

2019-11-20 23:15

administrator   ~0119410

Thanks for the patches, with r43521 they are applied and also the -Os problem is fixed.

Karl-Michael Schindler

2019-11-21 08:51

reporter   ~0119412

Thank you all.

Issue History

Date Modified Username Field Change
2019-11-13 12:07 Karl-Michael Schindler New Issue
2019-11-14 11:52 Karl-Michael Schindler Note Added: 0119294
2019-11-14 11:56 Karl-Michael Schindler Note Added: 0119295
2019-11-14 12:04 Karl-Michael Schindler Note Added: 0119297
2019-11-14 22:39 Florian Note Added: 0119305
2019-11-14 22:55 Karl-Michael Schindler Note Added: 0119306
2019-11-15 00:35 J. Gareth Moreton Note Added: 0119307
2019-11-15 10:30 Karl-Michael Schindler Note Added: 0119308
2019-11-15 16:17 J. Gareth Moreton Note Added: 0119320
2019-11-15 21:52 Florian Note Added: 0119329
2019-11-15 22:02 Florian Note Added: 0119330
2019-11-16 00:36 J. Gareth Moreton Note Added: 0119334
2019-11-16 00:39 J. Gareth Moreton Assigned To => J. Gareth Moreton
2019-11-16 00:39 J. Gareth Moreton Status new => assigned
2019-11-18 03:44 J. Gareth Moreton Note Added: 0119376
2019-11-19 21:58 J. Gareth Moreton File Added: jump-optimizations-can-do-check.patch
2019-11-19 21:58 J. Gareth Moreton File Added: jump-optimizations-mips-bug-fix.patch
2019-11-19 21:58 J. Gareth Moreton Note Added: 0119405
2019-11-19 21:58 J. Gareth Moreton Assigned To J. Gareth Moreton => Florian
2019-11-19 21:58 J. Gareth Moreton Status assigned => feedback
2019-11-19 21:58 J. Gareth Moreton FPCTarget => -
2019-11-20 23:15 Florian Status feedback => resolved
2019-11-20 23:15 Florian Resolution open => fixed
2019-11-20 23:15 Florian Fixed in Version => 3.3.1
2019-11-20 23:15 Florian Fixed in Revision => r43521
2019-11-20 23:15 Florian Note Added: 0119410
2019-11-21 08:51 Karl-Michael Schindler Status resolved => closed
2019-11-21 08:51 Karl-Michael Schindler Note Added: 0119412