2016-02-01 16:56 GMT+03:00 Senthil Kumar Selvaraj
<senthil_kumar.selva...@atmel.com>:
>
> 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.
>
Committed.
Denis.
