Ver código fonte

+ support for targets that require by-reference value parameters to be
be copied on the caller instead of callee side
o mark Darwin/Aarch64 as such a target (any AArch64 target will be like
that normally, as its ABI specifies this behaviour)
o don't mark by-reference value parameters on such targets as
vo_has_local_copy, since a) they don't have one (the copy is on the
caller side), and b) this ensures that all code handling such
parameters automatically knows that they are still by reference
after the init code has run
o when making the copies on the caller side, don't increase the
reference count for managed types except for variants, just like
is done when making the copies on the callee side. This is because
the reference count increasing code on the callee side only runs
for non-assembler functions, and we cannot know 100% certain on the
caller side whether the called function is assembler or not (e.g. in
case of externally declared functions)
o maybe over time we can reuse the Pascal code in
tcallparanode.copy_value_by_ref_para to replace the equivalent code
in hlcgobj and ncgutil also on the caller side for other targets

git-svn-id: trunk@29870 -

Jonas Maebe 10 anos atrás
pai
commit
c6ba0bb6fb
5 arquivos alterados com 251 adições e 7 exclusões
  1. 14 4
      compiler/hlcgobj.pas
  2. 224 0
      compiler/ncal.pas
  3. 7 2
      compiler/ncgutil.pas
  4. 2 1
      compiler/pparautl.pas
  5. 4 0
      compiler/systems.pas

+ 14 - 4
compiler/hlcgobj.pas

@@ -4523,7 +4523,11 @@ implementation
          if (tparavarsym(p).varspez=vs_value) then
           begin
             include(current_procinfo.flags,pi_needs_implicit_finally);
-            location_get_data_ref(list,tparavarsym(p).vardef,tparavarsym(p).localloc,href,is_open_array(tparavarsym(p).vardef),sizeof(pint));
+            location_get_data_ref(list,tparavarsym(p).vardef,tparavarsym(p).localloc,href,
+              is_open_array(tparavarsym(p).vardef) or
+              ((target_info.system in systems_caller_copy_addr_value_para) and
+               paramanager.push_addr_param(vs_value,tparavarsym(p).vardef,current_procinfo.procdef.proccalloption)),
+              sizeof(pint));
             if is_open_array(tparavarsym(p).vardef) then
               begin
                 if paramanager.push_high_param(tparavarsym(p).varspez,tparavarsym(p).vardef,current_procinfo.procdef.proccalloption) then
@@ -4543,7 +4547,8 @@ implementation
           end;
        end;
       { open arrays can contain elements requiring init/final code, so the else has been removed here }
-      if (tparavarsym(p).varspez=vs_value) and
+      if not(target_info.system in systems_caller_copy_addr_value_para) and
+         (tparavarsym(p).varspez=vs_value) and
          (is_open_array(tparavarsym(p).vardef) or
           is_array_of_const(tparavarsym(p).vardef)) then
         begin
@@ -4581,7 +4586,11 @@ implementation
                  if not((tparavarsym(p).vardef.typ=variantdef) and
                    paramanager.push_addr_param(tparavarsym(p).varspez,tparavarsym(p).vardef,current_procinfo.procdef.proccalloption)) then
                    begin
-                     location_get_data_ref(list,tparavarsym(p).vardef,tparavarsym(p).initialloc,href,is_open_array(tparavarsym(p).vardef),sizeof(pint));
+                     location_get_data_ref(list,tparavarsym(p).vardef,tparavarsym(p).initialloc,href,
+                       is_open_array(tparavarsym(p).vardef) or
+                       ((target_info.system in systems_caller_copy_addr_value_para) and
+                        paramanager.push_addr_param(vs_value,tparavarsym(p).vardef,current_procinfo.procdef.proccalloption)),
+                       sizeof(pint));
                      if is_open_array(tparavarsym(p).vardef) then
                        begin
                          if paramanager.push_high_param(tparavarsym(p).varspez,tparavarsym(p).vardef,current_procinfo.procdef.proccalloption) then
@@ -4656,7 +4665,8 @@ implementation
       { generate copies of call by value parameters, must be done before
         the initialization and body is parsed because the refcounts are
         incremented using the local copies }
-      current_procinfo.procdef.parast.SymList.ForEachCall(@g_copyvalueparas,list);
+      if not(target_info.system in systems_caller_copy_addr_value_para) then
+        current_procinfo.procdef.parast.SymList.ForEachCall(@g_copyvalueparas,list);
 
       if not(po_assembler in current_procinfo.procdef.procoptions) then
         begin

+ 224 - 0
compiler/ncal.pas

@@ -201,6 +201,10 @@ interface
           fparainit,
           fparacopyback: tnode;
           procedure handlemanagedbyrefpara(orgparadef: tdef);virtual;abstract;
+          { on some targets, value parameters that are passed by reference must
+            be copied to a temp location by the caller (and then a reference to
+            this temp location must be passed) }
+          procedure copy_value_by_ref_para;
        public
           callparaflags : tcallparaflags;
           parasym       : tparavarsym;
@@ -592,6 +596,208 @@ implementation
                              TCALLPARANODE
  ****************************************************************************}
 
+    procedure tcallparanode.copy_value_by_ref_para;
+      var
+        initstat,
+        copybackstat,
+        finistat: tstatementnode;
+        finiblock: tblocknode;
+        paratemp: ttempcreatenode;
+        arraysize,
+        arraybegin: tnode;
+        lefttemp: ttempcreatenode;
+        vardatatype,
+        temparraydef: tdef;
+      begin
+        { this routine is for targets where by-reference value parameters need
+          to be copied by the caller. It's basically the node-level equivalent
+          of thlcgobj.g_copyvalueparas }
+
+        { in case of an array constructor, we don't need a copy since the array
+          constructor itself is already constructed on the fly (and hence if
+          it's modified by the caller, that's no problem) }
+        if not is_array_constructor(left.resultdef) then
+          begin
+            fparainit:=internalstatements(initstat);
+            fparacopyback:=internalstatements(copybackstat);
+            finiblock:=internalstatements(finistat);
+            paratemp:=nil;
+
+            { making a copy of an open array, an array of const or a dynamic
+              array requires dynamic memory allocation since we don't know the
+              size at compile time }
+            if is_open_array(left.resultdef) or
+               is_array_of_const(left.resultdef) or
+               (is_dynamic_array(left.resultdef) and
+                is_open_array(parasym.vardef)) then
+              begin
+                 paratemp:=ctempcreatenode.create(voidpointertype,voidpointertype.size,tt_persistent,true);
+                 if is_dynamic_array(left.resultdef) then
+                   begin
+                      { note that in insert_typeconv, this dynamic array was
+                        already converted into an open array (-> dereferenced)
+                        and then its resultdef was restored to the original
+                        dynamic array one -> get the address before treating it
+                        as a dynamic array here }
+                     { first restore the actual resultdef of left }
+                     temparraydef:=left.resultdef;
+                     left.resultdef:=parasym.vardef;
+                     { get its address }
+                     lefttemp:=ctempcreatenode.create(voidpointertype,voidpointertype.size,tt_persistent,true);
+                     addstatement(initstat,lefttemp);
+                     addstatement(finistat,ctempdeletenode.create(lefttemp));
+                     addstatement(initstat,
+                       cassignmentnode.create(
+                         ctemprefnode.create(lefttemp),
+                         caddrnode.create_internal(left)
+                       )
+                     );
+                     { restore the resultdef }
+                     left.resultdef:=temparraydef;
+                     { now treat that address (correctly) as the original
+                       dynamic array to get its start and length }
+                     arraybegin:=cvecnode.create(
+                       ctypeconvnode.create_explicit(ctemprefnode.create(lefttemp),
+                         left.resultdef),
+                       genintconstnode(0)
+                     );
+                     arraysize:=caddnode.create(muln,
+                       geninlinenode(in_length_x,false,
+                         ctypeconvnode.create_explicit(ctemprefnode.create(lefttemp),
+                           left.resultdef)
+                       ),
+                       genintconstnode(tarraydef(left.resultdef).elementdef.size)
+                     );
+                   end
+                 else
+                   begin
+                     { no problem here that left is used multiple times, as
+                       sizeof() will simply evaluate to the high parameter }
+                     arraybegin:=left.getcopy;
+                     arraysize:=geninlinenode(in_sizeof_x,false,left);
+                   end;
+                 addstatement(initstat,paratemp);
+                 { paratemp:=getmem(sizeof(para)) }
+                 addstatement(initstat,
+                   cassignmentnode.create(
+                     ctemprefnode.create(paratemp),
+                     ccallnode.createintern('fpc_getmem',
+                       ccallparanode.create(
+                         arraysize.getcopy,nil
+                       )
+                     )
+                   )
+                 );
+                 { move(para,temp,sizeof(arr)) (no "left.getcopy" below because
+                   we replace left afterwards) }
+                 addstatement(initstat,
+                   ccallnode.createintern('MOVE',
+                     ccallparanode.create(
+                       arraysize,
+                       ccallparanode.create(
+                         cderefnode.create(ctemprefnode.create(paratemp)),
+                         ccallparanode.create(
+                           arraybegin,nil
+                         )
+                       )
+                     )
+                   )
+                 );
+                 { no reference count increases, that's still done on the callee
+                   side because for compatibility with targets that perform this
+                   copy on the callee side, that should only be done for non-
+                   assember functions (and we can't know that 100% certain here,
+                   e.g. in case of external declarations) (*) }
+
+                 { free the memory again after the call: freemem(paratemp) }
+                 addstatement(finistat,
+                   ccallnode.createintern('fpc_freemem',
+                     ccallparanode.create(
+                       ctemprefnode.create(paratemp),nil
+                     )
+                   )
+                 );
+                 { replace the original parameter with a dereference of the
+                   temp typecasted to the same type as the original parameter
+                   (don't free left, it has been reused above) }
+                 left:=ctypeconvnode.create_internal(
+                   cderefnode.create(ctemprefnode.create(paratemp)),
+                   left.resultdef);
+              end
+            else if is_shortstring(parasym.vardef) then
+              begin
+                { the shortstring parameter may have a different size than the
+                  parameter type -> assign and truncate/extend }
+                paratemp:=ctempcreatenode.create(parasym.vardef,parasym.vardef.size,tt_persistent,false);
+                addstatement(initstat,paratemp);
+                { assign shortstring }
+                addstatement(initstat,
+                  cassignmentnode.create(
+                    ctemprefnode.create(paratemp),left
+                  )
+                );
+                { replace parameter with temp (don't free left, it has been
+                  reused above) }
+                left:=ctemprefnode.create(paratemp);
+              end
+            else if parasym.vardef.typ=variantdef then
+              begin
+                vardatatype:=search_system_type('TVARDATA').typedef;
+                paratemp:=ctempcreatenode.create(vardatatype,vardatatype.size,tt_persistent,false);
+                addstatement(initstat,paratemp);
+                addstatement(initstat,
+                  ccallnode.createintern('fpc_variant_copy_overwrite',
+                    ccallparanode.create(
+                      ctypeconvnode.create_explicit(ctemprefnode.create(paratemp),
+                          vardatatype
+                        ),
+                      ccallparanode.create(ctypeconvnode.create_explicit(left,
+                        vardatatype),
+                        nil
+                      )
+                    )
+                  )
+                );
+                { replace parameter with temp (don't free left, it has been
+                  reused above) }
+                left:=ctypeconvnode.create_explicit(ctemprefnode.create(paratemp),parasym.vardef);
+              end
+            else if is_managed_type(left.resultdef) then
+              begin
+                { don't increase/decrease the reference count here, will be done by
+                  the callee (see (*) above) -> typecast to array of byte
+                  for the assignment to the temp }
+                temparraydef:=getarraydef(u8inttype,left.resultdef.size);
+                paratemp:=ctempcreatenode.create(temparraydef,temparraydef.size,tt_persistent,false);
+                addstatement(initstat,paratemp);
+                addstatement(initstat,
+                  cassignmentnode.create(
+                    ctemprefnode.create(paratemp),
+                    ctypeconvnode.create_internal(left,temparraydef)
+                  )
+                );
+                left:=ctypeconvnode.create_explicit(ctemprefnode.create(paratemp),left.resultdef);
+              end
+            else
+              begin
+                paratemp:=ctempcreatenode.create(left.resultdef,left.resultdef.size,tt_persistent,false);
+                addstatement(initstat,paratemp);
+                addstatement(initstat,
+                  cassignmentnode.create(ctemprefnode.create(paratemp),left)
+                );
+                { replace parameter with temp (don't free left, it has been
+                  reused above) }
+                left:=ctemprefnode.create(paratemp);
+              end;
+            addstatement(finistat,ctempdeletenode.create(paratemp));
+            addstatement(copybackstat,finiblock);
+            firstpass(fparainit);
+            firstpass(left);
+            firstpass(fparacopyback);
+          end;
+      end;
+
+
     constructor tcallparanode.create(expr,next : tnode);
 
       begin
@@ -721,6 +927,7 @@ implementation
           tcallparanode(right).firstcallparan;
         if not assigned(left.resultdef) then
           get_paratype;
+
         if assigned(parasym) and
            (target_info.system in systems_managed_vm) and
            (parasym.varspez in [vs_var,vs_out,vs_constref]) and
@@ -729,6 +936,23 @@ implementation
            (left.nodetype<>nothingn) then
           handlemanagedbyrefpara(left.resultdef);
 
+        { for targets that have to copy "value parameters by reference" on the
+          caller side
+
+          aktcallnode may not be assigned in case firstcallparan is called for
+          fake parameters to inline nodes (in that case, we don't have a real
+          call and hence no "caller side" either)
+          }
+        if assigned(aktcallnode) and
+           (target_info.system in systems_caller_copy_addr_value_para) and
+           ((assigned(parasym) and
+             (parasym.varspez=vs_value)) or
+            (cpf_varargs_para in callparaflags)) and
+           (left.nodetype<>nothingn) and
+           paramanager.push_addr_param(vs_value,left.resultdef,
+                      aktcallnode.procdefinition.proccalloption) then
+          copy_value_by_ref_para;
+
         { does it need to load RTTI? }
         if assigned(parasym) and (parasym.varspez=vs_out) and
            (cs_create_pic in current_settings.moduleswitches) and

+ 7 - 2
compiler/ncgutil.pas

@@ -698,7 +698,11 @@ implementation
                  if not((tparavarsym(p).vardef.typ=variantdef) and
                     paramanager.push_addr_param(tparavarsym(p).varspez,tparavarsym(p).vardef,current_procinfo.procdef.proccalloption)) then
                    begin
-                     hlcg.location_get_data_ref(list,tparavarsym(p).vardef,tparavarsym(p).initialloc,href,is_open_array(tparavarsym(p).vardef),sizeof(pint));
+                     hlcg.location_get_data_ref(list,tparavarsym(p).vardef,tparavarsym(p).initialloc,href,
+                       is_open_array(tparavarsym(p).vardef) or
+                       ((target_info.system in systems_caller_copy_addr_value_para) and
+                        paramanager.push_addr_param(vs_value,tparavarsym(p).vardef,current_procinfo.procdef.proccalloption)),
+                        sizeof(pint));
                      if is_open_array(tparavarsym(p).vardef) then
                        begin
                          { open arrays do not contain correct element count in their rtti,
@@ -1330,7 +1334,8 @@ implementation
         { generate copies of call by value parameters, must be done before
           the initialization and body is parsed because the refcounts are
           incremented using the local copies }
-        current_procinfo.procdef.parast.SymList.ForEachCall(@copyvalueparas,list);
+        if not(target_info.system in systems_caller_copy_addr_value_para) then
+          current_procinfo.procdef.parast.SymList.ForEachCall(@copyvalueparas,list);
 {$ifdef powerpc}
         { unget the register that contains the stack pointer before the procedure entry, }
         { which is used to access the parameters in their original callee-side location  }

+ 2 - 1
compiler/pparautl.pas

@@ -335,7 +335,8 @@ implementation
            if (varspez=vs_value) and
               paramanager.push_addr_param(varspez,vardef,pd.proccalloption) and
               not(is_open_array(vardef) or
-                  is_array_of_const(vardef)) then
+                  is_array_of_const(vardef)) and
+              not(target_info.system in systems_caller_copy_addr_value_para) then
              include(varoptions,vo_has_local_copy);
 
            { needs high parameter ? }

+ 4 - 0
compiler/systems.pas

@@ -338,6 +338,10 @@ interface
          system_jvm_android32
        ];
 
+       { all systems where a value parameter passed by reference must be copied
+         on the caller side rather than on the callee side }
+       systems_caller_copy_addr_value_para = [system_aarch64_darwin];
+
        { pointer checking (requires special code in FPC_CHECKPOINTER,
          and can never work for libc-based targets or any other program
          linking to an external library)