View Issue Details

IDProjectCategoryView StatusLast Update
0025586FPCCompilerpublic2021-04-13 17:16
ReporterMartin Friebe Assigned To 
PrioritynormalSeverityminorReproducibilityalways
Status newResolutionopen 
Platformw32OSwin 
Product Version2.7.1 
Summary0025586: peephole opt (i386) add (previoulsy commented) opt, load same memory twice
DescriptionThere is an old opt in the peephole for i386, that is commented.

The patch applies the necessary fixes, and activates it.

It translates
  movl [mem1],reg1
  movl [mem1],reg2
to
  movl [mem1],reg1
  movl reg1,reg2

Such code is produced by the both of below example in -O 1,2,3 and 4

There may be more examples, but some only in lower opt levels. Though that is because higher levels use registers, so to get mem loaded (and maybe trigger it) more complex code is needed.

The code after the opt is better, but not optimal, as now 2 regs have the same value, and so only one should be needed. To fix this would be way more complex, and until then this is an improvement.


--------------
The 2 patches are identical, except for indent.

The first only adds the changes, but does not change the indent of following "else" blocks. This allows to easily see the changes.

The 2nd includes that indent.
Additional Informationprogram project1;
{$mode objfpc}

type
  TFoo = class
    a,b : Integer;
    constructor Create;
  end;

  TFoo2=class(tfoo)
    constructor Create;
  end;

constructor TFoo2.Create;
begin
  inherited;
  a:= 1;
end;

constructor TFoo.Create;
begin
  b:=2;
end;

begin
end.

-----------------------------------
program project1;
{$mode objfpc}
type
  TFoo = class
    o1,o2,o3,o4,o5: TFoo;
    procedure Abc; virtual;
    procedure Bar; virtual;
  end;

procedure TFoo.Abc;
begin
  Bar;
  o1.bar;
  o2.bar;
  o3.bar;
  o4.bar;
  o5.bar;
end;

procedure TFoo.Bar;
begin
end;

begin
end.
TagsNo tags attached.
Fixed in Revision
FPCOldBugId0
FPCTarget
Attached Files

Activities

Martin Friebe

2014-01-23 22:32

manager  

popt386_move_same_mem_to_two_reg.patch (2,288 bytes)   
Index: compiler/i386/popt386.pas
===================================================================
--- compiler/i386/popt386.pas	(revision 26519)
+++ compiler/i386/popt386.pas	(working copy)
@@ -1369,21 +1369,19 @@
                                 end
                             end
                           else
-(*                          {movl [mem1],reg1
-                            movl [mem1],reg2
-                            to:
-                              movl [mem1],reg1
-                              movl reg1,reg2 }
+                          {movl [mem1],reg1     to     movl [mem1],reg1
+                           movl [mem1],reg2            movl reg1,reg2 }
                             if (taicpu(p).oper[0]^.typ = top_ref) and
-                              (taicpu(p).oper[1]^.typ = top_reg) and
-                              (taicpu(hp1).oper[0]^.typ = top_ref) and
-                              (taicpu(hp1).oper[1]^.typ = top_reg) and
-                              (taicpu(p).opsize = taicpu(hp1).opsize) and
-                              RefsEqual(TReference(taicpu(p).oper[0]^^),taicpu(hp1).oper[0]^^.ref^) and
-                              (taicpu(p).oper[1]^.reg<>taicpu(hp1).oper[0]^^.ref^.base) and
-                              (taicpu(p).oper[1]^.reg<>taicpu(hp1).oper[0]^^.ref^.index) then
-                              taicpu(hp1).loadReg(0,taicpu(p).oper[1]^.reg)
-                            else*)
+                               (taicpu(p).oper[1]^.typ = top_reg) and
+                               (taicpu(hp1).oper[0]^.typ = top_ref) and
+                               (taicpu(hp1).oper[1]^.typ = top_reg) and
+                               (taicpu(p).opsize = taicpu(hp1).opsize) and
+                               RefsEqual(TReference(taicpu(p).oper[0]^.ref^),taicpu(hp1).oper[0]^.ref^) and
+                               not(RegInOp(getsupreg(taicpu(p).oper[1]^.reg),taicpu(hp1).oper[0]^)) then
+                              begin
+                                taicpu(hp1).loadReg(0,taicpu(p).oper[1]^.reg)
+                              end
+                              else
                             {   movl const1,[mem1]
                                 movl [mem1],reg1
                             to:

Martin Friebe

2014-01-23 22:32

manager  

popt386_move_same_mem_to_two_reg__with_all_indent.patch (4,871 bytes)   
Index: compiler/i386/popt386.pas
===================================================================
--- compiler/i386/popt386.pas	(revision 26519)
+++ compiler/i386/popt386.pas	(working copy)
@@ -1369,39 +1369,37 @@
                                 end
                             end
                           else
-(*                          {movl [mem1],reg1
-                            movl [mem1],reg2
-                            to:
-                              movl [mem1],reg1
-                              movl reg1,reg2 }
+                          {movl [mem1],reg1     to     movl [mem1],reg1
+                           movl [mem1],reg2            movl reg1,reg2 }
                             if (taicpu(p).oper[0]^.typ = top_ref) and
-                              (taicpu(p).oper[1]^.typ = top_reg) and
-                              (taicpu(hp1).oper[0]^.typ = top_ref) and
-                              (taicpu(hp1).oper[1]^.typ = top_reg) and
-                              (taicpu(p).opsize = taicpu(hp1).opsize) and
-                              RefsEqual(TReference(taicpu(p).oper[0]^^),taicpu(hp1).oper[0]^^.ref^) and
-                              (taicpu(p).oper[1]^.reg<>taicpu(hp1).oper[0]^^.ref^.base) and
-                              (taicpu(p).oper[1]^.reg<>taicpu(hp1).oper[0]^^.ref^.index) then
-                              taicpu(hp1).loadReg(0,taicpu(p).oper[1]^.reg)
-                            else*)
-                            {   movl const1,[mem1]
-                                movl [mem1],reg1
-                            to:
-                                movl const1,reg1
-                                movl reg1,[mem1] }
-                              if (taicpu(p).oper[0]^.typ = top_const) and
-                                 (taicpu(p).oper[1]^.typ = top_ref) and
-                                 (taicpu(hp1).oper[0]^.typ = top_ref) and
-                                 (taicpu(hp1).oper[1]^.typ = top_reg) and
-                                 (taicpu(p).opsize = taicpu(hp1).opsize) and
-                                 RefsEqual(taicpu(hp1).oper[0]^.ref^,taicpu(p).oper[1]^.ref^) and
-                                 not(reginref(getsupreg(taicpu(hp1).oper[1]^.reg),taicpu(hp1).oper[0]^.ref^)) then
-                                begin
-                                  allocregbetween(asml,taicpu(hp1).oper[1]^.reg,p,hp1,usedregs);
-                                  taicpu(hp1).loadReg(0,taicpu(hp1).oper[1]^.reg);
-                                  taicpu(hp1).loadRef(1,taicpu(p).oper[1]^.ref^);
-                                  taicpu(p).loadReg(1,taicpu(hp1).oper[0]^.reg);
-                                end
+                               (taicpu(p).oper[1]^.typ = top_reg) and
+                               (taicpu(hp1).oper[0]^.typ = top_ref) and
+                               (taicpu(hp1).oper[1]^.typ = top_reg) and
+                               (taicpu(p).opsize = taicpu(hp1).opsize) and
+                               RefsEqual(TReference(taicpu(p).oper[0]^.ref^),taicpu(hp1).oper[0]^.ref^) and
+                               not(RegInOp(getsupreg(taicpu(p).oper[1]^.reg),taicpu(hp1).oper[0]^)) then
+                              begin
+                                taicpu(hp1).loadReg(0,taicpu(p).oper[1]^.reg)
+                              end
+                              else
+                                {   movl const1,[mem1]
+                                    movl [mem1],reg1
+                                to:
+                                    movl const1,reg1
+                                    movl reg1,[mem1] }
+                                if (taicpu(p).oper[0]^.typ = top_const) and
+                                   (taicpu(p).oper[1]^.typ = top_ref) and
+                                   (taicpu(hp1).oper[0]^.typ = top_ref) and
+                                   (taicpu(hp1).oper[1]^.typ = top_reg) and
+                                   (taicpu(p).opsize = taicpu(hp1).opsize) and
+                                   RefsEqual(taicpu(hp1).oper[0]^.ref^,taicpu(p).oper[1]^.ref^) and
+                                   not(reginref(getsupreg(taicpu(hp1).oper[1]^.reg),taicpu(hp1).oper[0]^.ref^)) then
+                                  begin
+                                    allocregbetween(asml,taicpu(hp1).oper[1]^.reg,p,hp1,usedregs);
+                                    taicpu(hp1).loadReg(0,taicpu(hp1).oper[1]^.reg);
+                                    taicpu(hp1).loadRef(1,taicpu(p).oper[1]^.ref^);
+                                    taicpu(p).loadReg(1,taicpu(hp1).oper[0]^.reg);
+                                  end
                         end;
                       if GetNextInstruction(p, hp1) and
                          MatchInstruction(hp1,A_BTS,A_BTR,[Taicpu(p).opsize]) and

Sergei Gorelkin

2014-01-24 13:33

developer   ~0072632

Just a note: I have in plans fixing this case (double load of the methodpointer) at node level.

Florian

2014-01-25 08:59

administrator   ~0072654

@Sergei, the basic idea to fix the first example is (in optcse.pas):

@@ -270,11 +270,11 @@
       begin
         result:=fen_false;
         nodes:=nil;
- if n.nodetype in cseinvariant then
+ if n.nodetype in cseinvariant+[calln] then
           begin
             csedomain:=true;
             foreachnodestatic(pm_postprocess,n,@searchsubdomain,@csedomain);
- if not(csedomain) then
+ if not(csedomain) and not(n.nodetype=calln) then
               begin
                 { try to transform the tree to get better cse domains, consider:

However:
- it needs better documenting why calln is not part of cseinvariant
- it triggers another bug in the compiler

Fixing the second example on node level is a little bit harder, but it can be improved as well.

ravi dion

2021-04-13 17:16

reporter   ~0130349

Is this fixed after 7 years?

Issue History

Date Modified Username Field Change
2014-01-23 22:32 Martin Friebe New Issue
2014-01-23 22:32 Martin Friebe File Added: popt386_move_same_mem_to_two_reg.patch
2014-01-23 22:32 Martin Friebe File Added: popt386_move_same_mem_to_two_reg__with_all_indent.patch
2014-01-24 13:33 Sergei Gorelkin Note Added: 0072632
2014-01-25 08:59 Florian Note Added: 0072654
2021-04-13 17:16 ravi dion Note Added: 0130349