[Bug target/62180] New: (RX600) - compiler doesn't honor -fstrict-volatile-bitfields and generates incorrect machine code for I/O register access

[jan.capek at braiins dot cz]

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

Bug ID: 62180
   Summary: (RX600) - compiler doesn't honor
-fstrict-volatile-bitfields and generates incorrect
machine code for I/O register access
   Product: gcc
   Version: 4.9.0
Status: UNCONFIRMED
  Severity: normal
  Priority: P3
 Component: target
  Assignee: unassigned at gcc dot gnu.org
  Reporter: jan.capek at braiins dot cz

Created attachment 33359
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=33359&action=edit
broken output from gcc 4.9.0

I have come across this issue while testing 4.9.x series of the compiler with
Renesas RX600 target
The compiler now generates byte accesses when writing bitfields completely
ignoring their 'type'. The strict-volatile-bitfields options seems to have no
effect. Our original compiler version 4.7.2 seems to work correctly and
generates accesses to the bitfields based on their type. This is essentially
what the documentation for the 'strict-volatile-bitfields' says.
I have tested this with 4.8.2 and 4.8.3, too with the same result. The test
case demonstrates access to a 16-bit memory mapped peripheral register. It is
absolutely necessary that the register is always being written in 16-bit
quantities.


A simple testcase:

struct st_cmt0 {
  union {
unsigned short WORD;
struct {
  volatile unsigned short CKS:2;
  volatile unsigned short :4;
  volatile unsigned short CMIE:1;
  volatile unsigned short :9;
} BIT;
  } CMCR;
  unsigned short CMCNT;
  unsigned short CMCOR;
};



void test(volatile struct st_cmt0 *reg)
{
  reg->CMCR.BIT.CMIE = 1;
}


The output for gcc 4.7.2 is:

.file"test.c"
.section P,"ax"
.global_test
.type_test, @function
_test:
pushmr6-r11
add#-4, r0, r6
mov.Lr6, r0
mov.Lr1, [r6]
mov.L[r6], r11
mov.W[r11], r10 ; HONORS the 16-bit register size
  bset#6, r10
 mov.Wr10, [r11] ; HONORS the 16-bit register size
 rtsd#28, r6-r11
.size_test, .-_test
.ident"GCC: (GNU) 4.7.2"

The output for gcc 4.9.0 is:

.file"test.c"
.section P,"ax"
.global_test
.type_test, @function
_test:
pushmr6-r11
add#-4, r0, r6
mov.Lr6, r0
mov.Lr1, [r6]
mov.L[r6], r11
mov.B[r11], r10 ; broken -  reads only part of the register
bset#6, r10
mov.Br10, [r11] ; broken - writes only part of the register
rtsd#28, r6-r11
.size_test, .-_test
.ident"GCC: (GNU) 4.9.0"

[Bug target/62180] (RX600) - compiler doesn't honor -fstrict-volatile-bitfields and generates incorrect machine code for I/O register access

[jan.capek at braiins dot cz]

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

--- Comment #2 from Jan Čapek  ---
(In reply to Andrew Pinski from comment #1)
> C11 says something different here.

Can you be a bit more specific?

[Bug target/62180] (RX600) - compiler doesn't honor -fstrict-volatile-bitfields and generates incorrect machine code for I/O register access

[jan.capek at braiins dot cz]

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

--- Comment #3 from Jan Čapek  ---
(In reply to Andrew Pinski from comment #1)
> C11 says something different here.

I can see the following the -fstrict-volatile-bitfields documentation:

"This option should be used if accesses to volatile bit-fields (or other
structure fields, although the compiler usually honors those types anyway)
should use a single access of the width of the field's type, aligned to a
 natural alignment if possible."

And this is actually the case with gcc 4.7.2, it ignore the -fstrict
option, and also doesn't need volatile on the individual bit-fields and it
still generates correct code due to specifying the bitfield types.

[Bug target/62180] (RX600) - compiler doesn't honor -fstrict-volatile-bitfields and generates incorrect machine code for I/O register access

[jan.capek at braiins dot cz]

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

--- Comment #5 from Jan Čapek  ---
(In reply to DJ Delorie from comment #4)
> Perhaps you need this patch:
> 
> https://gcc.gnu.org/ml/gcc-patches/2014-06/msg00993.html

DJ Delorie,

you are the man! The patch works as expected. Interestingly, the compiler now
honors the bitfield sizes even without explicitely specifying this options. The
resulting machine code output is:

.file   "test.c"
.section P,"ax"
.global _test
.type   _test, @function
_test:
pushm   r6-r11
add #-4, r0, r6
mov.L   r6, r0
mov.L   r1, [r6]
mov.L   [r6], r10
mov.W   [r10], r11 ; YES, read the entire register
or  #64, r11
mov.W   r11, [r10] ; YES, write the entire register
rtsd#28, r6-r11
.size   _test, .-_test
.ident  "GCC: (GNU) 4.9.0"


I have checked the 4.9 official branch and it still has not been included will
it go to the trunk/master branch?

Thanks again,

Jan