Re: Ping: PR61629 (was Re: Delay RTL initialization until it is really needed)

[Richard Biener <richard dot guenther at gmail dot com>]

On Thu, Jul 24, 2014 at 8:57 AM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> Ping.  Originaly message was here:
>
>   https://gcc.gnu.org/ml/gcc-patches/2014-07/msg01113.html

Ok.

Thanks,
Richard.

> Richard Sandiford <rdsandiford@googlemail.com> writes:
>> Richard Sandiford <rdsandiford@googlemail.com> writes:
>>> Jan Hubicka <hubicka@ucw.cz> writes:
>>>> Hi,
>>>
>>>> IRA initialization shows high in profiles even when building lto
>>>> objects.  This patch simply delays RTL backend initialization until we
>>>> really decide to output a function.  In some cases this avoids the
>>>> initialization completely (like in the case of LTO but also user
>>>> target attributes) and there is some hope for better cache locality.
>>>>
>>>> Basic idea is to have two flags saying whether lang and target
>>>> dependent bits needs initialization and check it when starting
>>>> function codegen.
>>>>
>>>> Bootstrapped/regtested x86_64-linux, testing also at AIX. Ok if it passes?
>>>>
>>>> Honza
>>>>
>>>>     * toplev.c (backend_init_target): Move init_emit_regs and init_regs to...
>>>>     (backend_init) ... here; skip ira_init_once and backend_init_target.
>>>>     (target_reinit) ... and here; clear this_target_rtl->lang_dependent_initialized.
>>>>     (lang_dependent_init_target): Clear this_target_rtl->lang_dependent_initialized;
>>>>     break out rtl initialization to ...
>>>>     (initialize_rtl): ... here; call also backend_init_target and ira_init_once.
>>>>     * toplev.h (initialize_rtl): New function.
>>>>     * function.c: Include toplev.h
>>>>     (init_function_start): Call initialize_rtl.
>>>>     * rtl.h (target_rtl): Add target_specific_initialized,
>>>>     lang_dependent_initialized.
>>>> Index: toplev.c
>>>> ===================================================================
>>>> --- toplev.c        (revision 211837)
>>>> +++ toplev.c        (working copy)
>>>> @@ -1583,14 +1583,6 @@ backend_init_target (void)
>>>>    /* Initialize alignment variables.  */
>>>>    init_alignments ();
>>>>
>>>> -  /* This reinitializes hard_frame_pointer, and calls init_reg_modes_target()
>>>> -     to initialize reg_raw_mode[].  */
>>>> -  init_emit_regs ();
>>>> -
>>>> -  /* This invokes target hooks to set fixed_reg[] etc, which is
>>>> -     mode-dependent.  */
>>>> -  init_regs ();
>>>> -
>>>>    /* This depends on stack_pointer_rtx.  */
>>>>    init_fake_stack_mems ();
>>>>
>>>> @@ -1632,9 +1624,13 @@ backend_init (void)
>>>>    init_varasm_once ();
>>>>    save_register_info ();
>>>>
>>>> -  /* Initialize the target-specific back end pieces.  */
>>>> -  ira_init_once ();
>>>> -  backend_init_target ();
>>>> +  /* Middle end needs this initialization for default mem attributes
>>>> +     used by early calls to make_decl_rtl.  */
>>>> +  init_emit_regs ();
>>>> +
>>>> +  /* Middle end needs this initialization for mode tables used to assign
>>>> +     modes to vector variables.  */
>>>> +  init_regs ();
>>>
>>> This causes a segfault on gcc.target/mips/umips-store16-1.c.  The register
>>> asm:
>>>
>>> register unsigned int global asm ("$16");
>>>
>>> causes us to globalise $16 and call reinit_regs.  reinit_regs in turn
>>> calls ira_init, but IRA hasn't been initialised at this point and
>>> prerequisites like init_fake_stack_mems haven't yet been called.
>>>
>>> Does the patch below look OK?
>>>
>>>> @@ -1686,6 +1682,31 @@ lang_dependent_init_target (void)
>>>>       front end is initialized.  It also depends on the HAVE_xxx macros
>>>>       generated from the target machine description.  */
>>>>    init_optabs ();
>>>> +  this_target_rtl->lang_dependent_initialized = false;
>>>> +}
>>>> +
>>>> +/* Perform initializations that are lang-dependent or target-dependent.
>>>> +   but matters only for late optimizations and RTL generation.  */
>>>> +
>>>> +void
>>>> +initialize_rtl (void)
>>>> +{
>>>> +  static int initialized_once;
>>>> +
>>>> +  /* Initialization done just once per compilation, but delayed
>>>> +     till code generation.  */
>>>> +  if (!initialized_once)
>>>> +    ira_init_once ();
>>>> +  initialized_once = true;
>>>> +
>>>> +  /* Target specific RTL backend initialization.  */
>>>> +  if (!this_target_rtl->target_specific_initialized)
>>>> +    backend_init_target ();
>>>> +  this_target_rtl->target_specific_initialized = true;
>>>> +
>>>> +  if (this_target_rtl->lang_dependent_initialized)
>>>> +    return;
>>>> +  this_target_rtl->lang_dependent_initialized = true;
>>>>
>>>>    /* The following initialization functions need to generate rtl, so
>>>>       provide a dummy function context for them.  */
>>>
>>> Why do you need both these flags?  We only call this function once
>>> the language has been initialised, so we should always be initialising
>>> both sets of information (backend_init_target and the stuff after
>>> the comment above, from the old lang_dependent_init_target).
>>>
>>> How about the second patch below, still under testing?  The new assert
>>> is OK for target_reinit because it has:
>>>
>>>   this_target_rtl->target_specific_initialized = false;
>>>
>>>   /* This initializes hard_frame_pointer, and calls init_reg_modes_target()
>>>      to initialize reg_raw_mode[].  */
>>>   init_emit_regs ();
>>>
>>>   /* This invokes target hooks to set fixed_reg[] etc, which is
>>>      mode-dependent.  */
>>>   init_regs ();
>>>
>>>   /* Reinitialize lang-dependent parts.  */
>>>   lang_dependent_init_target ();
>>>
>>> i.e. it sets the flag to say that the RTL stuff hasn't been initialised
>>> and then goes on to initialise everything that needs to be deferred.
>>
>> Now tested on mips64-linux-gnu.  OK for both patches?
>>
>>> Thanks,
>>> Richard
>>>
>>>
>>> gcc/
>>>      PR rtl-optimization/61629
>>>      * reginfo.c (reinit_regs): Only call ira_init and recog_init if
>>>      they have already been initialized.
>>>
>>> Index: gcc/reginfo.c
>>> ===================================================================
>>> --- gcc/reginfo.c    2014-07-16 07:59:00.039669668 +0100
>>> +++ gcc/reginfo.c    2014-07-16 07:59:00.397672987 +0100
>>> @@ -533,8 +533,11 @@ reinit_regs (void)
>>>    init_regs ();
>>>    /* caller_save needs to be re-initialized.  */
>>>    caller_save_initialized_p = false;
>>> -  ira_init ();
>>> -  recog_init ();
>>> +  if (this_target_rtl->target_specific_initialized)
>>> +    {
>>> +      ira_init ();
>>> +      recog_init ();
>>> +    }
>>>  }
>>>
>>>  /* Initialize some fake stack-frame MEM references for use in
>>>
>>>
>>> gcc/
>>>      * rtl.h (target_rtl): Remove lang_dependent_initialized.
>>>      * toplev.c (initialize_rtl): Don't use it.  Move previously
>>>      "language-dependent" calls to...
>>>      (backend_init): ...here.
>>>      (lang_dependent_init_target): Don't set lang_dependent_initialized.
>>>      Assert that RTL initialization hasn't happend yet.
>>>
>>> Index: gcc/rtl.h
>>> ===================================================================
>>> --- gcc/rtl.h        2014-07-11 11:55:14.265158363 +0100
>>> +++ gcc/rtl.h        2014-07-16 08:16:23.245370141 +0100
>>> @@ -2517,7 +2517,6 @@ struct GTY(()) target_rtl {
>>>
>>>    /* Track if RTL has been initialized.  */
>>>    bool target_specific_initialized;
>>> -  bool lang_dependent_initialized;
>>>  };
>>>
>>>  extern GTY(()) struct target_rtl default_target_rtl;
>>> Index: gcc/toplev.c
>>> ===================================================================
>>> --- gcc/toplev.c     2014-07-11 11:54:41.604838961 +0100
>>> +++ gcc/toplev.c     2014-07-16 08:22:36.226034738 +0100
>>> @@ -1604,6 +1604,10 @@ backend_init_target (void)
>>>       on a mode change.  */
>>>    init_expmed ();
>>>    init_lower_subreg ();
>>> +  init_set_costs ();
>>> +
>>> +  init_expr_target ();
>>> +  ira_init ();
>>>
>>>    /* We may need to recompute regno_save_code[] and regno_restore_code[]
>>>       after a mode change as well.  */
>>> @@ -1682,7 +1686,8 @@ lang_dependent_init_target (void)
>>>       front end is initialized.  It also depends on the HAVE_xxx macros
>>>       generated from the target machine description.  */
>>>    init_optabs ();
>>> -  this_target_rtl->lang_dependent_initialized = false;
>>> +
>>> +  gcc_assert (!this_target_rtl->target_specific_initialized);
>>>  }
>>>
>>>  /* Perform initializations that are lang-dependent or target-dependent.
>>> @@ -1701,26 +1706,10 @@ initialize_rtl (void)
>>>
>>>    /* Target specific RTL backend initialization.  */
>>>    if (!this_target_rtl->target_specific_initialized)
>>> -    backend_init_target ();
>>> -  this_target_rtl->target_specific_initialized = true;
>>> -
>>> -  if (this_target_rtl->lang_dependent_initialized)
>>> -    return;
>>> -  this_target_rtl->lang_dependent_initialized = true;
>>> -
>>> -  /* The following initialization functions need to generate rtl, so
>>> -     provide a dummy function context for them.  */
>>> -  init_dummy_function_start ();
>>> -
>>> -  /* Do the target-specific parts of expr initialization.  */
>>> -  init_expr_target ();
>>> -
>>> -  /* Although the actions of these functions are language-independent,
>>> -     they use optabs, so we cannot call them from backend_init.  */
>>> -  init_set_costs ();
>>> -  ira_init ();
>>> -
>>> -  expand_dummy_function_end ();
>>> +    {
>>> +      backend_init_target ();
>>> +      this_target_rtl->target_specific_initialized = true;
>>> +    }
>>>  }
>>>
>>>  /* Language-dependent initialization.  Returns nonzero on success.  */