diff mbox

RFA: patch to fix PR64157

Message ID 5482356B.4070400@redhat.com
State New
Headers show

Commit Message

Vladimir Makarov Dec. 5, 2014, 10:44 p.m. UTC
The following patch fixes

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64157


   After calling target_reinit from save_target_globals for switchable 
targets (as ppc), a lot of ira data (register sets, classes etc) become 
undefined.  After that ira-costs.c crashes when the undefined data are used.

   The patch was successfully bootstrapped and tested on x86-64.

   Ok to commit to the trunk?

2014-12-05  Vladimir Makarov  <vmakarov@redhat.com>

         PR rtl-optimization/64157
         * toplev.c (target_reinit): Call ira_init.

Comments

Richard Sandiford Dec. 8, 2014, 3:41 p.m. UTC | #1
Vladimir Makarov <vmakarov@redhat.com> writes:
>    The following patch fixes
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64157
>
>
>    After calling target_reinit from save_target_globals for switchable 
> targets (as ppc), a lot of ira data (register sets, classes etc) become 
> undefined.  After that ira-costs.c crashes when the undefined data are used.
>
>    The patch was successfully bootstrapped and tested on x86-64.
>
>    Ok to commit to the trunk?
>
> 2014-12-05  Vladimir Makarov  <vmakarov@redhat.com>
>
>          PR rtl-optimization/64157
>          * toplev.c (target_reinit): Call ira_init.
>
> Index: toplev.c
> ===================================================================
> --- toplev.c	(revision 218378)
> +++ toplev.c	(working copy)
> @@ -1888,6 +1888,8 @@ target_reinit (void)
>    /* This invokes target hooks to set fixed_reg[] etc, which is
>       mode-dependent.  */
>    init_regs ();
> +  /* Set IRA data depended on target parameters.  */
> +  ira_init ();

Could you give more details about how this happens?  It's reverting part of:

2014-06-25  Jan Hubicka  <hubicka@ucw.cz>

	* 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.

which was supposed to delay the ira_init so that it only gets called
once we start to compile a function.  It sounds from your patch like
that either isn't early enough or isn't happening at all for some reason.

Thanks,
Richard
Vladimir Makarov Dec. 8, 2014, 5:41 p.m. UTC | #2
On 2014-12-08 10:41 AM, Richard Sandiford wrote:
> Vladimir Makarov <vmakarov@redhat.com> writes:
>>     The following patch fixes
>>
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64157
>>
>>
>>     After calling target_reinit from save_target_globals for switchable
>> targets (as ppc), a lot of ira data (register sets, classes etc) become
>> undefined.  After that ira-costs.c crashes when the undefined data are used.
>>
>>     The patch was successfully bootstrapped and tested on x86-64.
>>
>>     Ok to commit to the trunk?
>>
>> 2014-12-05  Vladimir Makarov  <vmakarov@redhat.com>
>>
>>           PR rtl-optimization/64157
>>           * toplev.c (target_reinit): Call ira_init.
>>
>> Index: toplev.c
>> ===================================================================
>> --- toplev.c	(revision 218378)
>> +++ toplev.c	(working copy)
>> @@ -1888,6 +1888,8 @@ target_reinit (void)
>>     /* This invokes target hooks to set fixed_reg[] etc, which is
>>        mode-dependent.  */
>>     init_regs ();
>> +  /* Set IRA data depended on target parameters.  */
>> +  ira_init ();
>
> Could you give more details about how this happens?  It's reverting part of:
>

I don't know this code well, Richard.  I've just see that IRA data is 
undefined after save_target_globals, e.g. max_struct_costs_size has 
value zero which never should happen.  As save_target_globals calls
target_reinit and target_reinit has a comment

/* Reinitialize everything when target parameters, such as register 
usage, 

    have changed.  */

I think it is natural reinitialize data for IRA from here.

I'd appreciate if somebody look at this problem as it is not RA one per 
se.   It is very easy to reproduce even on x86 build for ppc target.
diff mbox

Patch

Index: toplev.c
===================================================================
--- toplev.c	(revision 218378)
+++ toplev.c	(working copy)
@@ -1888,6 +1888,8 @@  target_reinit (void)
   /* This invokes target hooks to set fixed_reg[] etc, which is
      mode-dependent.  */
   init_regs ();
+  /* Set IRA data depended on target parameters.  */
+  ira_init ();
 
   /* Reinitialize lang-dependent parts.  */
   lang_dependent_init_target ();