[Patch, avr] Restore default value of PARAM_ALLOW_STORE_DATA_RACES to 1

[Senthil Kumar Selvaraj]

Hi,

  This patch sets PARAM_ALLOW_STORE_DATA_RACES to 1 (the default until
  a year back), to avoid code size regressions in trunk (and probably
  5.x )for the AVR target.

  Consider the following piece of code
  
volatile int z;
void foo(int x)
{
    static char i;
    for (i=0; i< 4; ++i)
    {
        if (x > 2)
            z = 1;
        else
            z = 2;
    }
}

Unmodified gcc trunk generates this

        movw r20,r24
        sts i.1495,__zero_reg__
        ldi r25,0
        ldi r18,0
        ldi r22,lo8(2)
        ldi r23,0
        ldi r30,lo8(1)
        ldi r31,0
.L2:
        cpi r25,lo8(4)
        brne .L5
        cpse r18,__zero_reg__
        sts i.1495,r25
.L1:
        ret
.L5:
        cpi r20,3
        cpc r21,__zero_reg__
        brlt .L3
        sts z+1,r31
        sts z,r30
.L4:
        subi r25,lo8(-(1))
        ldi r18,lo8(1)
        rjmp .L2
.L3:
        sts z+1,r23
        sts z,r22
        rjmp .L4
        .size   foo, .-foo
        .local  i.1495
        .comm   i.1495,1,1
        .comm   z,2,1
        .ident  "GCC: (GNU) 6.0.0 20160201 (experimental)"

Note the usage of an extra reg (r18) that is used as a flag to
record loop entry (in .L4), and the conditional store of r25 to i in .L2.

In 4.x, there is no extra reg usage - only a single unconditional set of
i to r25 at the end of the loop.

Digging into the code, I found that LIM checks
PARAM_ALLOW_STORE_DATA_RACES and introduces the flag to avoid store data
races - see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52558. The
default value of the param was set to zero a year and a half back - see
https://gcc.gnu.org/ml/gcc-patches/2014-06/msg01548.html.

For AVR, I guess assuming any store can cause a data race is too
pessimistic for the general case. Globals shared with interrupts will
need special handling for atomic access anyway, so I thought we should
revert the default back to allow store data races.

If this is ok, could someone commit please? I don't have commit access.

Regards
Senthil

gcc/ChangeLog

2016-02-01  Senthil Kumar Selvaraj  <senthil_kumar.selva...@atmel.com>

        * config/avr/avr.c (avr_option_override): Set
    PARAM_ALLOW_STORE_DATA_RACES to 1.


diff --git gcc/config/avr/avr.c gcc/config/avr/avr.c
index e557772..a7728e3 100644
--- gcc/config/avr/avr.c
+++ gcc/config/avr/avr.c
@@ -43,6 +43,7 @@
 #include "expr.h"
 #include "langhooks.h"
 #include "cfgrtl.h"
+#include "params.h"
 #include "builtins.h"
 #include "context.h"
 #include "tree-pass.h"
@@ -410,6 +411,15 @@ 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().  */