Created attachment 40091 [details] reproducer The test is correct C++ program with bitfield larger than its base type, i.e. signed char : 9. -O2 produces incorrect result. The C++14 standard states explicitly the following (9.3.1, [class.bit]): The value of the integral constant expression may be larger than the number of bits in the object representation (3.9) of the bit-field’s type; in such cases the extra bits are used as padding bits and do not participate in the value representation (3.9) of the bit-field. > g++ -O0 -w -o no_opt bitfield.cpp main.cpp;./no_opt 0 > g++ -O2 -w -o opt bitfield.cpp main.cpp;./opt -1 > cat bitfield.h struct S { long int : 23; long int m0 : 24; long int m1 : 10; long int m2 : 24; signed char m3 : 9; }; extern struct S s1; > cat bitfield.cpp #include "bitfield.h" void foo() { s1.m3 = 0; s1.m2 = -1193165L; } > cat main.cpp #include <stdio.h> #include "bitfield.h" struct S s1; void init () { s1.m0 = 0L; s1.m1 = 0L; s1.m2 = 0L; s1.m3 = 0; } void foo(); int main() { init(); foo(); printf("%d\n",s1.m3); return 0; } > g++ -v Using built-in specs. COLLECT_GCC=g++ COLLECT_LTO_WRAPPER=/home/dybaboki/gcc/bin/libexec/gcc/x86_64-pc-linux-gnu/7.0.0/lto-wrapper Target: x86_64-pc-linux-gnu Configured with: ../gcc/configure --with-arch=corei7 --with-cpu=corei7 --enable-clocale=gnu --with-system-zlib --enable-shared --with-demangler-in-ld --enable-cloog-backend=isl --with-fpmath=sse --prefix=/home/dybaboki/gcc/bin --enable-languages=c,c++,fortran,lto : (reconfigured) ../gcc/configure --with-arch=corei7 --with-cpu=corei7 --enable-clocale=gnu --with-system-zlib --enable-shared --with-demangler-in-ld --enable-cloog-backend=isl --with-fpmath=sse --prefix=/home/dybaboki/gcc/bin --enable-languages=c,c++,fortran,lto : (reconfigured) ../gcc/configure --with-arch=corei7 --with-cpu=corei7 --enable-clocale=gnu --with-system-zlib --enable-shared --with-demangler-in-ld --enable-cloog-backend=isl --with-fpmath=sse --prefix=/home/dybaboki/gcc/bin --enable-languages=c,c++,fortran,lto : (reconfigured) ../gcc_github/configure --with-arch=corei7 --with-cpu=corei7 --enable-clocale=gnu --with-system-zlib --enable-shared --with-demangler-in-ld --enable-cloog-backend=isl --with-fpmath=sse --prefix=/home/dybaboki/gcc/bin --enable-languages=c,c++,fortran,lto --no-create --no-recursion : (reconfigured) ../gcc/configure --with-arch=corei7 --with-cpu=corei7 --enable-clocale=gnu --with-system-zlib --enable-shared --with-demangler-in-ld --enable-cloog-backend=isl --with-fpmath=sse --prefix=/home/dybaboki/gcc/bin --enable-languages=c,c++,fortran,lto --no-create --no-recursion Thread model: posix gcc version 7.0.0 20161119 (experimental) (GCC)
Started with the introduction of the store merging pass: r241649. struct S { long int : 23; long int a : 24; long int b : 10; long int c : 24; signed char d : 9; } s; __attribute__((noinline, noclone)) void foo () { s.d = 0; s.c = -1193165L; } int main () { foo (); if (s.d != 0) __builtin_abort (); return 0; } store-merging pass turns: s.d = 0; s.c = -1193165; into: MEM[(struct S *)&s + 8B] = 4293774131;
I'll have a look.
Hmm, maybe it's DECL_SIZE (of the FIELD_DECL) vs. TYPE_PRECISION mismatch not honored by store merging (I can very well think of other places having the same confusion...).
Assignment of negative values to signed bitfields has caused me some headaches in encode_tree_to_bitpos due to native_encode_expr sign-extending the value to GET_MODE_SIZE (TYPE_MODE (...)) bytes, making it necessary to truncate the excess padding introduced. Maybe there's a but g in that logic
The over-sized bitfield isn't really needed for this, making it more severe: struct S { long int : 23; long int a : 24; long int b : 10; long int c : 24; signed char d : 8; } s; __attribute__((noinline, noclone)) void foo () { s.d = 0; s.c = -1193165L; } int main () { foo (); if (s.d != 0) __builtin_abort (); return 0; } Indeed, I think the bug is in encode_tree_to_bitpos, still debugging where exactly.
Created attachment 40096 [details] gcc7-pr78436.patch Untested fix. Most of the changes are just nits I ran into when debugging, the real fix is that byte_size is apparently intentionally 1 byte larger than the mode size, so that shifting by 0 to BITS_PER_UNIT - 1 has the extra byte to shift into. But when not shifting, we IMHO need to subtract that extra + 1 back.
Author: jakub Date: Tue Nov 22 10:15:43 2016 New Revision: 242691 URL: https://gcc.gnu.org/viewcvs?rev=242691&root=gcc&view=rev Log: PR tree-optimization/78436 * gimple-ssa-store-merging.c (zero_char_buf): Removed. (shift_bytes_in_array, shift_bytes_in_array_right, merged_store_group::apply_stores): Formatting fixes. (clear_bit_region): Likewise. Use memset. (encode_tree_to_bitpos): Formatting fixes. Fix comment typos - EPXR instead of EXPR and inerted instead of inserted. Use memset instead of zero_char_buf. For !BYTES_BIG_ENDIAN decrease byte_size by 1 if shift_amnt is 0. * gcc.c-torture/execute/pr78436.c: New test. Added: trunk/gcc/testsuite/gcc.c-torture/execute/pr78436.c Modified: trunk/gcc/ChangeLog trunk/gcc/gimple-ssa-store-merging.c trunk/gcc/testsuite/ChangeLog
Fixed.