* [Bug c/50521] -fstrict-volatile-bitfields is not strict
2011-09-26 7:18 [Bug c/50521] New: -fstrict-volatile-bitfields is not strict kikairoya at gmail dot com
@ 2011-09-26 11:43 ` rguenth at gcc dot gnu.org
2011-09-26 12:08 ` kikairoya at gmail dot com
` (13 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: rguenth at gcc dot gnu.org @ 2011-09-26 11:43 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50521
--- Comment #1 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-09-26 10:48:15 UTC ---
I don't think volatile union bitfield constitutes a "volatile bitfield" as
per what GCC implements.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Bug c/50521] -fstrict-volatile-bitfields is not strict
2011-09-26 7:18 [Bug c/50521] New: -fstrict-volatile-bitfields is not strict kikairoya at gmail dot com
2011-09-26 11:43 ` [Bug c/50521] " rguenth at gcc dot gnu.org
@ 2011-09-26 12:08 ` kikairoya at gmail dot com
2011-09-27 8:22 ` kikairoya at gmail dot com
` (12 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: kikairoya at gmail dot com @ 2011-09-26 12:08 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50521
--- Comment #2 from Tomohiro Kashiwada <kikairoya at gmail dot com> 2011-09-26 11:42:37 UTC ---
Variable 'bitfield' declared as volatile, so all bitfield's members are
volatile.
Even if declare 'bits' as volatile, gcc dumps same code.
However, adding option -fno-strict-volatile-bitfields instead of
-fstrict-volatile-bitfields, gcc ignores type of bit-field.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Bug c/50521] -fstrict-volatile-bitfields is not strict
2011-09-26 7:18 [Bug c/50521] New: -fstrict-volatile-bitfields is not strict kikairoya at gmail dot com
2011-09-26 11:43 ` [Bug c/50521] " rguenth at gcc dot gnu.org
2011-09-26 12:08 ` kikairoya at gmail dot com
@ 2011-09-27 8:22 ` kikairoya at gmail dot com
2011-10-20 1:54 ` kikairoya at gmail dot com
` (11 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: kikairoya at gmail dot com @ 2011-09-27 8:22 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50521
--- Comment #3 from Tomohiro Kashiwada <kikairoya at gmail dot com> 2011-09-27 04:15:19 UTC ---
Other test case (gcc generates wrong code):
volatile union {
// add volatiles tenaciously, but...
volatile unsigned int all;
volatile struct {
volatile unsigned int a: 8;
volatile unsigned int : 7;
volatile unsigned int b: 1;
volatile unsigned int : 8;
volatile unsigned int c: 8;
} bits;
} bitfield;
int main() {
bitfield.bits.a = 1;
bitfield.bits.b = 1;
bitfield.bits.c = 1;
return 0;
}
$ gcc -S -O -o - btf.c -fstrict-volatile-bitfields
.file "btf.c"
.text
.globl main
.type main, @function
main:
.LFB0:
.cfi_startproc
movl $1, bitfield(%rip) ; **** breaks other area! ****
movl bitfield(%rip), %eax
orb $128, %ah
movl %eax, bitfield(%rip)
movl $1, bitfield+3(%rip) ; **** unaligned, and breaks other area!
****
movl $0, %eax
ret
.cfi_endproc
.LFE0:
.size main, .-main
.comm bitfield,4,4
.ident "GCC: (Debian 4.6.1-3) 4.6.1"
.section .note.GNU-stack,"",@progbits
$ rx-elf-gcc -S -O -o - btf.c -fstrict-volatile-bitfields
.file "btf.c"
.section P,"ax"
.global _main
.type _main, @function
_main:
mov.L #_bitfield, r14
mov.L #1, [r14] ; **** breaks other area! ****
mov.L [r14], r4
bset #15, r4
mov.L r4, [r14]
mov.L [r14], r4
and #0xffffff, r4
or #0x1000000, r4
mov.L r4, [r14] ; ok, aligned (my misunderstanding).
mov.L #0, r1
rts
.size _main, .-_main
.comm _bitfield,4,4
.ident "GCC: (GNU) 4.6.1"
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Bug c/50521] -fstrict-volatile-bitfields is not strict
2011-09-26 7:18 [Bug c/50521] New: -fstrict-volatile-bitfields is not strict kikairoya at gmail dot com
` (2 preceding siblings ...)
2011-09-27 8:22 ` kikairoya at gmail dot com
@ 2011-10-20 1:54 ` kikairoya at gmail dot com
2011-10-27 23:23 ` henrik at henriknordstrom dot net
` (10 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: kikairoya at gmail dot com @ 2011-10-20 1:54 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50521
--- Comment #4 from Tomohiro Kashiwada <kikairoya at gmail dot com> 2011-10-20 01:53:33 UTC ---
Created attachment 25559
--> http://gcc.gnu.org/bugzilla/attachment.cgi?id=25559
proposal patch
fix generating wrong code. no longer volatile-bitfield breaks other field.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Bug c/50521] -fstrict-volatile-bitfields is not strict
2011-09-26 7:18 [Bug c/50521] New: -fstrict-volatile-bitfields is not strict kikairoya at gmail dot com
` (3 preceding siblings ...)
2011-10-20 1:54 ` kikairoya at gmail dot com
@ 2011-10-27 23:23 ` henrik at henriknordstrom dot net
2011-10-28 1:33 ` kikairoya at gmail dot com
` (9 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: henrik at henriknordstrom dot net @ 2011-10-27 23:23 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50521
Henrik Nordström <henrik at henriknordstrom dot net> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |henrik at henriknordstrom
| |dot net
--- Comment #5 from Henrik Nordström <henrik at henriknordstrom dot net> 2011-10-27 23:23:13 UTC ---
Is this related to the strict volatile bitfields change in trunk revision
171347?
http://gcc.gnu.org/viewcvs/trunk/gcc/expr.c?view=log&pathrev=171347
It's quite similar changes but at slightly different locations.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Bug c/50521] -fstrict-volatile-bitfields is not strict
2011-09-26 7:18 [Bug c/50521] New: -fstrict-volatile-bitfields is not strict kikairoya at gmail dot com
` (4 preceding siblings ...)
2011-10-27 23:23 ` henrik at henriknordstrom dot net
@ 2011-10-28 1:33 ` kikairoya at gmail dot com
2011-10-28 2:00 ` henrik at henriknordstrom dot net
` (8 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: kikairoya at gmail dot com @ 2011-10-28 1:33 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50521
--- Comment #6 from Tomohiro Kashiwada <kikairoya at gmail dot com> 2011-10-28 01:32:42 UTC ---
(In reply to comment #5)
> Is this related to the strict volatile bitfields change in trunk revision
> 171347?
> http://gcc.gnu.org/viewcvs/trunk/gcc/expr.c?view=log&pathrev=171347
No, r171347 and r171346 generates same wrong code.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Bug c/50521] -fstrict-volatile-bitfields is not strict
2011-09-26 7:18 [Bug c/50521] New: -fstrict-volatile-bitfields is not strict kikairoya at gmail dot com
` (5 preceding siblings ...)
2011-10-28 1:33 ` kikairoya at gmail dot com
@ 2011-10-28 2:00 ` henrik at henriknordstrom dot net
2011-10-28 3:59 ` kikairoya at gmail dot com
` (7 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: henrik at henriknordstrom dot net @ 2011-10-28 2:00 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50521
--- Comment #7 from Henrik Nordström <henrik at henriknordstrom dot net> 2011-10-28 01:59:34 UTC ---
Right. r171347 seem to be about fetches from bitfields while this change is
about stores?
An interesting test would be
bitfield.bits.a = bitfield.bits.c
which should load the int to a register, load the int again to another
register, copy c to a between them and store the result. I guess the double
load may be optimized away as it's an sideeffect of the aassignment.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Bug c/50521] -fstrict-volatile-bitfields is not strict
2011-09-26 7:18 [Bug c/50521] New: -fstrict-volatile-bitfields is not strict kikairoya at gmail dot com
` (6 preceding siblings ...)
2011-10-28 2:00 ` henrik at henriknordstrom dot net
@ 2011-10-28 3:59 ` kikairoya at gmail dot com
2011-10-28 7:34 ` henrik at henriknordstrom dot net
` (6 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: kikairoya at gmail dot com @ 2011-10-28 3:59 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50521
--- Comment #8 from Tomohiro Kashiwada <kikairoya at gmail dot com> 2011-10-28 03:58:45 UTC ---
(In reply to comment #7)
> Right. r171347 seem to be about fetches from bitfields while this change is
> about stores?
>
> An interesting test would be
>
> bitfield.bits.a = bitfield.bits.c
>
> which should load the int to a register, load the int again to another
> register, copy c to a between them and store the result. I guess the double
> load may be optimized away as it's an sideeffect of the aassignment.
Both r171346 and r171347 (target=rx-elf, byte and bit little-endian) generates:
mov.L #_bitfield, r14
mov.L [r14], r4
shlr #24, r4
mov.L r4, [r14]
r171347 with my patch generates:
mov.L #_bitfield, r14
mov.L [r14], r3
shlr #24, r3
mov.L [r14], r4
and #0xffffffffffffff00, r4
or r3, r4
mov.L r4, [r14]
in this code, loads twice. I don't know the Standard C or C++ requires double
load or not.
When -fno-strict-volatile-bitfields, generates
mov.L #_bitfield, r14
mov.B 3[r14], r4
mov.B r4, [r14]
this code is seems ok but GCC ignores alignment (configuration of rx-elf has
STRICT_ALIGNMENT).
Another test,
bitfield.bits.a = bitfield.bits.a;
r171346 and r171347 generates:
mov.L #_bitfield, r14
mov.L [r14], r4
movu.B r4, r4
mov.L r4, [r14]
r171347 with my patch:
mov.L #_bitfield, r14
mov.L [r14], r3
movu.B r3, r3
mov.L [r14], r4
and #0xffffffffffffff00, r4
or r3, r4
mov.L r4, [r14]
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Bug c/50521] -fstrict-volatile-bitfields is not strict
2011-09-26 7:18 [Bug c/50521] New: -fstrict-volatile-bitfields is not strict kikairoya at gmail dot com
` (7 preceding siblings ...)
2011-10-28 3:59 ` kikairoya at gmail dot com
@ 2011-10-28 7:34 ` henrik at henriknordstrom dot net
2011-10-28 8:13 ` kikairoya at gmail dot com
` (5 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: henrik at henriknordstrom dot net @ 2011-10-28 7:34 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50521
--- Comment #9 from Henrik Nordström <henrik at henriknordstrom dot net> 2011-10-28 07:32:48 UTC ---
C standard does not define any of this It's all implementation and platform ABI
dependent.
The C standard does define not storage size of a bit-field other than that it's
sufficiently large, or bit-fields of other types than _Bool and int
(+qualifiers), or if bits outside the specific bit-field is accessed as side
effect when operating on a bit-field.
For example the ARM ABI defines volatile bitfield memory access in full detail
as being equal to the base type of the bitfield, and I see now that it actually
requires a double load in the mentioned test case. See
http://infocenter.arm.com/help/topic/com.arm.doc.ihi0042d/IHI0042D_aapcs.pdf
section 7.1.7.5.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Bug c/50521] -fstrict-volatile-bitfields is not strict
2011-09-26 7:18 [Bug c/50521] New: -fstrict-volatile-bitfields is not strict kikairoya at gmail dot com
` (8 preceding siblings ...)
2011-10-28 7:34 ` henrik at henriknordstrom dot net
@ 2011-10-28 8:13 ` kikairoya at gmail dot com
2011-10-28 8:15 ` kikairoya at gmail dot com
` (4 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: kikairoya at gmail dot com @ 2011-10-28 8:13 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50521
--- Comment #10 from Tomohiro Kashiwada <kikairoya at gmail dot com> 2011-10-28 08:13:31 UTC ---
(In reply to comment #9)
> http://infocenter.arm.com/help/topic/com.arm.doc.ihi0042d/IHI0042D_aapcs.pdf
> section 7.1.7.5.
Thanks, I see.
On ARM ABI, reading or writing to volatile-bitfields should not cause double
access and regardless of volatileness, access should be aligned as container's
type.
I think that to avoid double access needs treatment of volatile object.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Bug c/50521] -fstrict-volatile-bitfields is not strict
2011-09-26 7:18 [Bug c/50521] New: -fstrict-volatile-bitfields is not strict kikairoya at gmail dot com
` (9 preceding siblings ...)
2011-10-28 8:13 ` kikairoya at gmail dot com
@ 2011-10-28 8:15 ` kikairoya at gmail dot com
2011-10-28 17:46 ` henrik at henriknordstrom dot net
` (3 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: kikairoya at gmail dot com @ 2011-10-28 8:15 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50521
--- Comment #11 from Tomohiro Kashiwada <kikairoya at gmail dot com> 2011-10-28 08:15:39 UTC ---
Created attachment 25642
--> http://gcc.gnu.org/bugzilla/attachment.cgi?id=25642
patch to honor STRICT_ALIGNMENT
honor STRICT_ALIGNMENT when accessing non-volatile-bitfields
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Bug c/50521] -fstrict-volatile-bitfields is not strict
2011-09-26 7:18 [Bug c/50521] New: -fstrict-volatile-bitfields is not strict kikairoya at gmail dot com
` (10 preceding siblings ...)
2011-10-28 8:15 ` kikairoya at gmail dot com
@ 2011-10-28 17:46 ` henrik at henriknordstrom dot net
2011-10-29 6:50 ` kikairoya at gmail dot com
` (2 subsequent siblings)
14 siblings, 0 replies; 16+ messages in thread
From: henrik at henriknordstrom dot net @ 2011-10-28 17:46 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50521
--- Comment #12 from Henrik Nordström <henrik at henriknordstrom dot net> 2011-10-28 17:46:15 UTC ---
Regarding the double load. In a statement like a = b, both a & be should be
individually accessed even if they refer to the same storage. So
bitfield.bits.a = bitfield.bits.c should load the bitfield variable twice, once
for reading the rvalue and once for masking the lvalue assignment.
See 7.1.7.5 second and third paragraph and the note just after.
Regarding STRICT_ALIGNMENT, not strictly needed on ARM i think. Smaller
accesses than the base type is acceptable, as long as it's aligned to the
matching access size (8/16/32/64 bit) and on ARMv7 unaligned access is allowed,
but at a performance penalty. And this change is technically unrelated to
strict-volatile-bitfields even if there is overlap.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Bug c/50521] -fstrict-volatile-bitfields is not strict
2011-09-26 7:18 [Bug c/50521] New: -fstrict-volatile-bitfields is not strict kikairoya at gmail dot com
` (11 preceding siblings ...)
2011-10-28 17:46 ` henrik at henriknordstrom dot net
@ 2011-10-29 6:50 ` kikairoya at gmail dot com
2011-10-29 10:46 ` henrik at henriknordstrom dot net
2011-10-29 14:37 ` kikairoya at gmail dot com
14 siblings, 0 replies; 16+ messages in thread
From: kikairoya at gmail dot com @ 2011-10-29 6:50 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50521
--- Comment #13 from Tomohiro Kashiwada <kikairoya at gmail dot com> 2011-10-29 06:50:11 UTC ---
(In reply to comment #12)
> Regarding the double load. In a statement like a = b, both a & be should be
> individually accessed even if they refer to the same storage. So
> bitfield.bits.a = bitfield.bits.c should load the bitfield variable twice, once
> for reading the rvalue and once for masking the lvalue assignment.
>
> See 7.1.7.5 second and third paragraph and the note just after.
Is that means a statement
a = b;
always should be treat as if
tmp = b;
a = tmp;
two individual statements?
> Regarding STRICT_ALIGNMENT, not strictly needed on ARM i think. Smaller
> accesses than the base type is acceptable, as long as it's aligned to the
> matching access size (8/16/32/64 bit) and on ARMv7 unaligned access is allowed,
> but at a performance penalty. And this change is technically unrelated to
> strict-volatile-bitfields even if there is overlap.
I think STRICT_ALIGNMENT is not only for ARM, but also MIPS, SH and others.
I'll create new ticket later about STRICT_ALIGNMENT.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Bug c/50521] -fstrict-volatile-bitfields is not strict
2011-09-26 7:18 [Bug c/50521] New: -fstrict-volatile-bitfields is not strict kikairoya at gmail dot com
` (12 preceding siblings ...)
2011-10-29 6:50 ` kikairoya at gmail dot com
@ 2011-10-29 10:46 ` henrik at henriknordstrom dot net
2011-10-29 14:37 ` kikairoya at gmail dot com
14 siblings, 0 replies; 16+ messages in thread
From: henrik at henriknordstrom dot net @ 2011-10-29 10:46 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50521
--- Comment #14 from Henrik Nordström <henrik at henriknordstrom dot net> 2011-10-29 10:45:53 UTC ---
(In reply to comment #13)
> > See 7.1.7.5 second and third paragraph and the note just after.
>
> Is that means a statement
> a = b;
> always should be treat as if
> tmp = b;
> a = tmp;
> two individual statements?
That's my understanding of the text.
Further, given
struct {
volatile int bits:32;
} a;
int result;
my understanding is that the note means that
result = ++a.bits;
should be translated into
int tmp = a.bits;
a.bits = tmp + 1;
result = a.bits;
and a++ into
result = a.bits;
tmp = a.bits;
a.bits = tmp + 1;
suitable expanded for aligned container loads & stores on each access to
a.bits, with each access to the a container int handled as a volatile access.
Also, from in the second sentence of the note the load of the container on
write may not be optimized away even if it's entirely masked by the write
operation. I.e.
a.bits = x;
translates into
int tmp = *(int *)(int aligned address of a.bits);
tmp &= ~0xFFFFFFFF;
tmp |= x;
*(int *)(int aligned address of a.bits) = tmp;
which is the same load & store memory access sequence as used when a.bits is
not filling the entire container.
int tmp = *(int *)(int aligned address of a.bits);
tmp &= ~a_bits_mask;
tmp |= (x << shift) & ~a_bits_mask;
*(int *)(int aligned address of a.bits) = tmp;
where it's not allowed to optimize away the initial load if the result of that
load is entirely masked away by the bit-field assignment (32 bit ~0xFFFFFFFF ==
0). Operations on tmp between the load & store of the a.bits container may be
optimized freely as tmp itself is not a volatile.
> I think STRICT_ALIGNMENT is not only for ARM, but also MIPS, SH and others.
> I'll create new ticket later about STRICT_ALIGNMENT.
agreed
none of this is only about ARM I think. But the ARM AAPCS specification is
suitable to use as reference for the implementation as it's very detailed on
how to operate on volatile bit-fields and also alignment requirements on
bit-field accesses in general. Not sure the others are as detailed, and it's
very likely the rules from the ARM specification can be applied there as well.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Bug c/50521] -fstrict-volatile-bitfields is not strict
2011-09-26 7:18 [Bug c/50521] New: -fstrict-volatile-bitfields is not strict kikairoya at gmail dot com
` (13 preceding siblings ...)
2011-10-29 10:46 ` henrik at henriknordstrom dot net
@ 2011-10-29 14:37 ` kikairoya at gmail dot com
14 siblings, 0 replies; 16+ messages in thread
From: kikairoya at gmail dot com @ 2011-10-29 14:37 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50521
--- Comment #15 from Tomohiro Kashiwada <kikairoya at gmail dot com> 2011-10-29 14:37:15 UTC ---
I see. Thanks detail exposition.
I think the behavior of my patch seems correct and it should be merged.
^ permalink raw reply [flat|nested] 16+ messages in thread