View Issue Details

IDProjectCategoryView StatusLast Update
0036675FPCCompilerpublic2020-02-09 18:40
ReporterJ. Gareth Moreton Assigned ToFlorian  
PrioritylowSeveritytweakReproducibilityN/A
Status resolvedResolutionfixed 
Platformi386 and x86_64OSMicrosoft Windows 
Product Version3.3.1 
Summary0036675: [Patch] CMOV extensions
DescriptionThis patch extends the JmpMov2CMov optimisation to also include MOVs that read from the stack (and don't have an index register specified), since these should never contain invalid addresses (except if there's an erroneous assembler block earlier that corrupts the stack or overwrites EBP/RBP and doesn't restore it).
Steps To ReproduceApply patch and confirm correct and improved optimisation.
Tagscompiler, i386, optimizations, patch, x86, x86_64
Fixed in Revision44141
FPCOldBugId
FPCTarget-
Attached Files

Activities

J. Gareth Moreton

2020-02-07 15:31

developer  

cmov-extensions.patch (3,087 bytes)   
Index: compiler/x86/aoptx86.pas
===================================================================
--- compiler/x86/aoptx86.pas	(revision 44124)
+++ compiler/x86/aoptx86.pas	(working copy)
@@ -61,6 +61,15 @@
           except where the register is being written }
         function ReplaceRegisterInInstruction(const p: taicpu; const AOldReg, ANewReg: TRegister): Boolean;
 
+        { Returns true if the reference only refers to ESP or EBP (or their 64-bit equivalents),
+          or writes to a global symbol }
+        class function IsRefSafe(const ref: PReference): Boolean; static; inline;
+
+
+        { Returns true if the given MOV instruction can be safely converted to CMOV }
+        class function CanBeCMOV(p : tai) : boolean; static;
+
+
         function DeepMOVOpt(const p_mov: taicpu; const hp: taicpu): Boolean;
 
         procedure DebugMsg(const s : string; p : tai);inline;
@@ -1627,6 +1636,23 @@
       end;
 
 
+    class function TX86AsmOptimizer.IsRefSafe(const ref: PReference): Boolean; inline;
+      begin
+        Result :=
+          (ref^.index = NR_NO) and
+          (
+{$ifdef x86_64}
+            (
+              (ref^.base = NR_RIP) and
+              (ref^.refaddr in [addr_pic, addr_pic_no_got])
+            ) or
+{$endif x86_64}
+            (ref^.base = NR_STACK_POINTER_REG) or
+            (ref^.base = current_procinfo.framepointer)
+          );
+      end;
+
+
     function TX86AsmOptimizer.DeepMOVOpt(const p_mov: taicpu; const hp: taicpu): Boolean;
       var
         CurrentReg, ReplaceReg: TRegister;
@@ -4638,7 +4664,7 @@
       end;
 
 
-    function CanBeCMOV(p : tai) : boolean;
+    class function TX86AsmOptimizer.CanBeCMOV(p : tai) : boolean;
       begin
          CanBeCMOV:=assigned(p) and
            MatchInstruction(p,A_MOV,[S_W,S_L,S_Q]) and
@@ -4648,16 +4674,15 @@
             or ((taicpu(p).oper[0]^.typ = top_ref) and
              (taicpu(p).oper[0]^.ref^.refaddr = addr_no))
            }
-           (MatchOpType(taicpu(p),top_reg,top_reg) or
-            { allow references, but only pure symbols or got rel. addressing with RIP as based,
-              it is not expected that this can cause a seg. violation }
-            (MatchOpType(taicpu(p),top_ref,top_reg) and
-             (((taicpu(p).oper[0]^.ref^.base=NR_NO) and (taicpu(p).oper[0]^.ref^.refaddr=addr_no)){$ifdef x86_64} or
-              ((taicpu(p).oper[0]^.ref^.base=NR_RIP) and (taicpu(p).oper[0]^.ref^.refaddr=addr_pic)){$endif x86_64}
-             ) and
-             (taicpu(p).oper[0]^.ref^.index=NR_NO) and
-             (taicpu(p).oper[0]^.ref^.offset=0)
-            )
+           (taicpu(p).oper[1]^.typ = top_reg) and
+           (
+             (taicpu(p).oper[0]^.typ = top_reg) or
+             { allow references, but only pure symbols or got rel. addressing with RIP as based,
+               it is not expected that this can cause a seg. violation }
+             (
+               (taicpu(p).oper[0]^.typ = top_ref) and
+               IsRefSafe(taicpu(p).oper[0]^.ref)
+             )
            );
       end;
 
cmov-extensions.patch (3,087 bytes)   

Florian

2020-02-09 18:40

administrator   ~0120974

Thanks, applied.

Issue History

Date Modified Username Field Change
2020-02-07 15:31 J. Gareth Moreton New Issue
2020-02-07 15:31 J. Gareth Moreton File Added: cmov-extensions.patch
2020-02-07 15:32 J. Gareth Moreton Tag Attached: patch
2020-02-07 15:32 J. Gareth Moreton Tag Attached: compiler
2020-02-07 15:32 J. Gareth Moreton Tag Attached: optimizations
2020-02-07 15:32 J. Gareth Moreton Tag Attached: x86_64
2020-02-07 15:32 J. Gareth Moreton Tag Attached: x86
2020-02-07 15:32 J. Gareth Moreton Tag Attached: i386
2020-02-07 15:32 J. Gareth Moreton Priority normal => low
2020-02-07 15:32 J. Gareth Moreton Severity minor => tweak
2020-02-07 15:32 J. Gareth Moreton FPCTarget => -
2020-02-09 18:40 Florian Assigned To => Florian
2020-02-09 18:40 Florian Status new => resolved
2020-02-09 18:40 Florian Resolution open => fixed
2020-02-09 18:40 Florian Fixed in Revision => 44141
2020-02-09 18:40 Florian Note Added: 0120974