This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Partial inlining


> > this patch implements function splitting that allows partial inlining.
> > Functions are split if there is BB whose frequency is smaller than
> > frequency of entry BB such that the control flow must past through it just
> > once and afterwards we don't use values computed beforehand.
> 
> Are you sure we want that at -O2?  This looks more like an -O3 kind of things 
> to me, it can do dumb things like splitting a function and inlining back the 
> broken out part into the remaining one, with a lot of useless tree copying 
> in-between.

Well, at -O2 it splits only functions declared inline so it should not be terribly
back (i.e. it does not show as slowdown at our testers).
One option would be to further limit splitting only when call to split part
is cold or it makes previously uninlinable function inlinable so the degenerate
case of splitting and re-combining does not happen.
> 
> The attached testcase is even more interesting: the compiler used to inline a 
> pragma Inline function, now it first splits it, then inlines back the broken 
> out part, and finally doesn't inline the reconstructed function because the 
> size estimate isn't the same as the original one!
> 
> To reproduce: put the testcase into $(objdir)/gcc and run
>   ./gnat1 -quiet -O2 -gnatpgn -gnatwns -Iada -I$(srcdir)/gcc/ada exp_ch4.adb
> 
> The ICE is an unrelated known issue that I don't want to fix (for now) because 
> it happens to catch problems upstream (it already caught the noreturn bug we 
> discussed a few weeks ago).  The interesting function is Atree.Parent.

Jakub sent me a testcase where similarly funny thing happen, there it is because
the split part has frequency of 0 everywhere (it is guarted by builtin_expect).
This confuses inliner herusitic by concluding that time spent in function is 0
and savings is a call cost (a roundof error to prevent division by zero).
I will work on fixing that one first hopefully fixing this one too.

I suspect this scenario can happen too in more legal cases when the split part
ends up looking better candidate for inlining than the header.  This can also
be computed beforehand.

Honza
> 
> -- 
> Eric Botcazou

> with Atree;    use Atree;
> with Einfo;    use Einfo;
> with Exp_Util; use Exp_Util;
> with Nlists;   use Nlists;
> with Nmake;    use Nmake;
> with Rtsfind;  use Rtsfind;
> with Sinfo;    use Sinfo;
> with Tbuild;   use Tbuild;
> 
> package body Exp_Ch4 is
> 
>    procedure Expand_N_Allocator (N : Node_Id) is
>       PtrT  : constant Entity_Id  := Etype (N);
>       Loc   : constant Source_Ptr := Sloc (N);
>       T : constant Entity_Id := Entity (Expression (N));
>       Args, Decls : List_Id;
>       Arg1, Decl, Temp_Decl : Node_Id;
>       Temp  : Entity_Id;
> 
>    begin
> 
>       Temp := Make_Temporary (Loc, 'P');
> 
>       Arg1 :=
>         Make_Explicit_Dereference (Loc,
>           Prefix => New_Reference_To (Temp, Loc));
> 
>       Args := New_List (Arg1);
> 
>       if Has_Task (T) then
>          if Nkind (Parent (N)) = N_Assignment_Statement then
>             declare
>                Nam : constant Node_Id := Name (Parent (N));
> 
>             begin
>                if Is_Entity_Name (Nam) then
>                   Decls :=
>                     Build_Task_Image_Decls
>                       (Loc,
>                        New_Occurrence_Of
>                          (Entity (Nam), Sloc (Nam)), T);
> 
>                else
>                   Decls := Build_Task_Image_Decls (Loc, T, T);
>                end if;
>             end;
> 
>          elsif Nkind (Parent (N)) = N_Object_Declaration then
>             Decls :=
>               Build_Task_Image_Decls
>                 (Loc, Defining_Identifier (Parent (N)), T);
> 
>          else
>             Decls := Build_Task_Image_Decls (Loc, T, T);
>          end if;
>       end if;
> 
>       Temp_Decl :=
>         Make_Object_Declaration (Loc,
>           Defining_Identifier => Temp,
>           Constant_Present    => True,
>           Object_Definition   => New_Reference_To (PtrT, Loc),
>           Expression          => N);
> 
>       Insert_Action (N, Temp_Decl, Suppress => All_Checks);
> 
>    exception
>       when RE_Not_Available => return;
>    end Expand_N_Allocator;
> 
> end Exp_Ch4;

> with Types; use Types;
> 
> package Exp_Ch4 is
> 
>    procedure Expand_N_Allocator (N : Node_Id);
> 
> end Exp_Ch4;


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]