[avr] Fix avr build broken by r276985.

Message ID 19818313-26f0-c675-79b2-d7e3d284a2f3@gjlay.de
State New
Headers show
Series
  • [avr] Fix avr build broken by r276985.
Related show

Commit Message

Georg-Johann Lay Oct. 17, 2019, 11:18 a.m.
Hi,

r276985 broke avr because it removed PARAM_ALLOW_STORE_DATA_RACES from 
--params.  The patch fixes that by using flag_store_data_races = 1 instead.

Ok to apply?

Johann

	* config/avr/avr.c (avr_option_override): Fix broken build
	introduced by r276985.

Comments

Richard Biener Oct. 17, 2019, 11:20 a.m. | #1
On Thu, 17 Oct 2019, Georg-Johann Lay wrote:

> Hi,

> 

> r276985 broke avr because it removed PARAM_ALLOW_STORE_DATA_RACES from

> --params.  The patch fixes that by using flag_store_data_races = 1 instead.

> 

> Ok to apply?


OK and sorry for the breakage.

Richard.
Eric Botcazou Oct. 17, 2019, 11:22 a.m. | #2
> r276985 broke avr because it removed PARAM_ALLOW_STORE_DATA_RACES from

> --params.  The patch fixes that by using flag_store_data_races = 1 instead.


Note that you'll unconditionally override the user, unlike the original code.

-- 
Eric Botcazou
Jakub Jelinek Oct. 17, 2019, 11:27 a.m. | #3
On Thu, Oct 17, 2019 at 01:22:54PM +0200, Eric Botcazou wrote:
> > r276985 broke avr because it removed PARAM_ALLOW_STORE_DATA_RACES from

> > --params.  The patch fixes that by using flag_store_data_races = 1 instead.

> 

> Note that you'll unconditionally override the user, unlike the original code.


Yeah, better make that
  if (!global_options_set.x_flag_store_data_races)
    flag_store_data_races = 1;

	Jakub
Georg-Johann Lay Oct. 17, 2019, 11:42 a.m. | #4
Am 10/17/19 um 1:22 PM schrieb Eric Botcazou:
>> r276985 broke avr because it removed PARAM_ALLOW_STORE_DATA_RACES from

>> --params.  The patch fixes that by using flag_store_data_races = 1 instead.

> 

> Note that you'll unconditionally override the user, unlike the original code.


You're right.  What about this one?

Johann


	Fix breakage introduced by r276985.
	* config/avr/avr.c (avr_option_override): Remove set of
	PARAM_ALLOW_STORE_DATA_RACES.
	* common/config/avr/avr-common.c (avr_option_optimization_table)
	[OPT_LEVELS_ALL]: Turn on -fallow-store-data-races.
Index: common/config/avr/avr-common.c
===================================================================
--- common/config/avr/avr-common.c	(revision 277097)
+++ common/config/avr/avr-common.c	(working copy)
@@ -38,6 +38,11 @@ static const struct default_options avr_
     { OPT_LEVELS_ALL, OPT_fcaller_saves, NULL, 0 },
     { OPT_LEVELS_1_PLUS_NOT_DEBUG, OPT_mgas_isr_prologues, NULL, 1 },
     { OPT_LEVELS_1_PLUS, OPT_mmain_is_OS_task, NULL, 1 },
+    /* Allow optimizer to introduce store data races. This used to be the
+       default -- it was changed because bigger targets did not see any
+       performance decrease. For the AVR though, disallowing data races
+       introduces additional code in LIM and increases reg pressure.  */
+    { OPT_LEVELS_ALL, OPT_fallow_store_data_races, NULL, 1 },
     { OPT_LEVELS_NONE, 0, NULL, 0 }
   };
 
Index: config/avr/avr.c
===================================================================
--- config/avr/avr.c	(revision 277097)
+++ config/avr/avr.c	(working copy)
@@ -741,15 +741,6 @@ avr_option_override (void)
   if (avr_strict_X)
     flag_caller_saves = 0;
 
-  /* Allow optimizer to introduce store data races. This used to be the
-     default - it was changed because bigger targets did not see any
-     performance decrease. For the AVR though, disallowing data races
-     introduces additional code in LIM and increases reg pressure.  */
-
-  maybe_set_param_value (PARAM_ALLOW_STORE_DATA_RACES, 1,
-                         global_options.x_param_values,
-                         global_options_set.x_param_values);
-
   /* Unwind tables currently require a frame pointer for correctness,
      see toplev.c:process_options().  */
Richard Biener Oct. 17, 2019, 12:19 p.m. | #5
On Thu, 17 Oct 2019, Georg-Johann Lay wrote:

> Am 10/17/19 um 1:22 PM schrieb Eric Botcazou:

> >> r276985 broke avr because it removed PARAM_ALLOW_STORE_DATA_RACES from

> >> --params.  The patch fixes that by using flag_store_data_races = 1 instead.

> > 

> > Note that you'll unconditionally override the user, unlike the original

> > code.

> 

> You're right.  What about this one?


LGTM.

> Johann

> 

> 

> 	Fix breakage introduced by r276985.

> 	* config/avr/avr.c (avr_option_override): Remove set of

> 	PARAM_ALLOW_STORE_DATA_RACES.

> 	* common/config/avr/avr-common.c (avr_option_optimization_table)

> 	[OPT_LEVELS_ALL]: Turn on -fallow-store-data-races.

> 

> 


-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

Patch

Index: config/avr/avr.c
===================================================================
--- config/avr/avr.c	(revision 277097)
+++ config/avr/avr.c	(working copy)
@@ -746,9 +746,7 @@  avr_option_override (void)
      performance decrease. For the AVR though, disallowing data races
      introduces additional code in LIM and increases reg pressure.  */
 
-  maybe_set_param_value (PARAM_ALLOW_STORE_DATA_RACES, 1,
-                         global_options.x_param_values,
-                         global_options_set.x_param_values);
+  flag_store_data_races = 1;
 
   /* Unwind tables currently require a frame pointer for correctness,
      see toplev.c:process_options().  */