* use more precise vs_* information to replace less parameters of inlined

    procedures with const and value parameters with temps, allowing a bit
    more value propagation
  + tinline6.pp for testing wrong propagation of value parameters in
    dangerous situations

git-svn-id: trunk@1914 -

.gitattributes

@@ -5362,6 +5362,7 @@ tests/test/tinline2.pp svneol=native#text/plain
 tests/test/tinline3.pp svneol=native#text/plain
 tests/test/tinline4.pp svneol=native#text/plain
 tests/test/tinline5.pp -text
+tests/test/tinline6.pp svneol=native#text/plain
 tests/test/tint641.pp svneol=native#text/plain
 tests/test/tint642.pp svneol=native#text/plain
 tests/test/tint643.pp svneol=native#text/plain

compiler/ncal.pas

@@ -2096,12 +2096,30 @@ type
       end;
 
 
+    function nonlocalvars(var n: tnode; arg: pointer): foreachnoderesult;
+      begin
+        result := fen_false;
+        { this is just to play it safe, there are more safe situations }
+        if (n.nodetype = derefn) or
+           ((n.nodetype = loadn) and
+            { globals and fields of (possibly global) objects could always be changed in the callee }
+            ((tloadnode(n).symtable.symtabletype in [globalsymtable,objectsymtable]) or
+            { statics can only be modified by functions in the same unit }
+             ((tloadnode(n).symtable.symtabletype = staticsymtable) and
+              (tloadnode(n).symtable = tsymtable(arg))))) or
+           ((n.nodetype = subscriptn) and
+            (tsubscriptnode(n).vs.owner.symtabletype = objectsymtable)) then
+          result := fen_norecurse_true;
+      end;
+
+
     procedure tcallnode.createinlineparas(var createstatement, deletestatement: tstatementnode);
       var
         para: tcallparanode;
         tempnode: ttempcreatenode;
         tempnodes: ttempnodes;
         n: tnode;
+        paracomplexity: longint;
       begin
         { parameters }
         para := tcallparanode(left);
@@ -2119,38 +2137,80 @@ type
                 n := para.left.getcopy;
                 para.left.free;
                 para.left := n;
+                firstpass(para.left);
 
                 { create temps for value parameters, function result and also for    }
                 { const parameters which are passed by value instead of by reference }
                 { we need to take care that we use the type of the defined parameter and not of the
                   passed parameter, because these can be different in case of a formaldef (PFV) }
+                paracomplexity := node_complexity(para.left);
+                { check if we have to create a temp, assign the parameter's }
+                { contents to that temp and then substitute the paramter    }
+                { with the temp everywhere in the function                  }
                 if
-                  (
-                   { the problem is that we can't take the address of a function result :( }
-                   (vo_is_funcret in tparavarsym(para.parasym).varoptions) or
-                   (para.parasym.varspez = vs_value) or
-                   ((para.parasym.varspez = vs_const) and
-                    (para.parasym.vartype.def.deftype<>formaldef) and
-                   { the compiler expects that it can take the address of parameters passed by reference in
-                     the case of const so we can't replace the node simply by a constant node
-                     When playing with this code, ensure that
-                     function f(const a,b  : longint) : longint;inline;
-                       begin
-                         result:=a*b;
-                       end;
-
-                     [...]
-                     ...:=f(10,20));
-                     [...]
+                  ((tparavarsym(para.parasym).varregable = vr_none) and
+                   not(para.left.expectloc in [LOC_REFERENCE,LOC_CREFERENCE]))  or
+                  { we can't assign to formaldef temps }
+                  ((para.parasym.vartype.def.deftype<>formaldef) and
+                   (
+                    { if paracomplexity > 1, we normally take the address of   }
+                    { the parameter expression, store it in a temp and         }
+                    { substitute the dereferenced temp in the inlined function }
+                    { We can't do this if we can't take the address of the     }
+                    { parameter expression, so in that case assign to a temp   }
+                    ((paracomplexity > 1) and
+                     (not valid_for_addr(para.left,false) or
+                      (para.left.nodetype = calln) or
+                      is_constnode(para.left))) or
+                    { the problem is that we can't take the address of a function result :( }
+                    (vo_is_funcret in tparavarsym(para.parasym).varoptions) or
+                    { we do not need to create a temp for value parameters }
+                    { which are not modified in the inlined function       }
+                    { const parameters can get vs_readwritten if their     }
+                    { address is taken                                     }
+                    ((((para.parasym.varspez = vs_value) and
+                       (para.parasym.varstate in [vs_initialised,vs_declared,vs_read])) or
+                      { in case of const, this is only necessary if the     }
+                      { variable would be passed by value normally, or if   }
+                      { there is such a variable somewhere in an expression }
+                       ((para.parasym.varspez = vs_const) and
+                        (not paramanager.push_addr_param(vs_const,para.parasym.vartype.def,procdefinition.proccalloption) or
+                         (paracomplexity > 1)))) and
+                     { however, if we pass a global variable, an object field or}
+                     { an expression containing a pointer dereference as        }
+                     { parameter, this value could be modified in other ways as }
+                     { well and in such cases create a temp to be on the safe   }
+                     { side                                                     }
+                     foreachnodestatic(para.left,@nonlocalvars,pointer(symtableproc))) or
+                    { value parameters of which we know they are modified by }
+                    { definition have to be copied to a temp                 }
+                    ((para.parasym.varspez = vs_value) and
+                     not(para.parasym.varstate in [vs_initialised,vs_declared,vs_read])) or
+                    { the compiler expects that it can take the address of parameters passed by reference in
+                      the case of const so we can't replace the node simply by a constant node
+                      When playing with this code, ensure that
+                      function f(const a,b  : longint) : longint;inline;
+                        begin
+                          result:=a*b;
+                        end;
 
-                     is still folded. (FK)
-                     }
-                    (
-                      { this must be a not ... of course }
-                      not(paramanager.push_addr_param(vs_const,para.parasym.vartype.def,procdefinition.proccalloption)) or
-                      (node_complexity(para.left) >= NODE_COMPLEXITY_INF)
-                    ))
-                   ) then
+                      [...]
+                      ...:=f(10,20));
+                      [...]
+
+                      is still folded. (FK)
+                      }
+                    ((para.parasym.varspez = vs_const) and
+                     { const para's can get vs_readwritten if their address }
+                     { is taken                                             }
+                     ((para.parasym.varstate = vs_readwritten) or
+                      { call-by-reference const's may need to be passed by }
+                      { reference to function called in the inlined code   }
+                      (paramanager.push_addr_param(vs_const,para.parasym.vartype.def,procdefinition.proccalloption) and
+                       (not valid_for_addr(para.left,false) or
+                        is_constnode(para.left)))))
+                   )
+                  ) then
                   begin
                     { in theory, this is always regable, but ncgcall can't }
                     { handle it yet in all situations (JM)                 }
@@ -2176,7 +2236,11 @@ type
                         addstatement(deletestatement,ctempdeletenode.create_normal_temp(tempnode));
                       end
                   end
-                else if (node_complexity(para.left) > 1) then
+                { otherwise if the parameter is "complex", take the address   }
+                { of the parameter expression, store it in a temp and replace }
+                { occurrences of the parameter with dereferencings of this    }
+                { temp                                                        }
+                else if (paracomplexity > 1) then
                   begin
                     tempnode := ctempcreatenode.create(voidpointertype,voidpointertype.def.size,tt_persistent,tparavarsym(para.parasym).varregable<>vr_none);
                     addstatement(createstatement,tempnode);

tests/test/tinline6.pp

@@ -0,0 +1,43 @@
+{$inline on}
+{$mode objfpc}
+
+type
+  tc = class
+    lf: longint;
+    procedure t(l: longint); inline;
+  end;
+
+var
+  a: longint;
+
+procedure tc.t(l: longint); inline;
+begin
+  lf := 10;
+  if (l <> 5) then
+    begin
+      writeln('error class');
+      halt(1);
+    end;
+end;
+
+
+procedure t(l: longint); inline;
+begin
+  a := 10;
+  if (l <> 5) then
+    begin
+      writeln('error proc');
+      halt(1);
+    end;
+end;
+
+var
+  c: tc;
+
+begin
+  c := tc.create;
+  c.lf := 5;
+  c.t(c.lf);
+  a := 5;
+  t(a);
+end.