http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48124

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-03-15 
10:52:44 UTC ---
I think that is not the problem, it is fine if some variables are aligned more
than they need to be for performance reasons.  TYPE_ALIGN is still 32 bits,
while just DECL_ALIGN is 64 bits.

The bug is in expand_assignment/store_bit_field{,_1} I think.
First expand_assignment calls set_mem_attributes_minus_bitpos (to_rtx, to, 0,
bitpos);
which changes
(mem/s/c:BLK (symbol_ref:DI ("e") [flags 0x2] <var_decl 0x7ffff1f4b000 e>) [2
e+0 S12 A64])
into:
(mem/s/v/j/c:BLK (symbol_ref:DI ("e") [flags 0x2] <var_decl 0x7ffff1f4b000 e>)
[2 e+0 S2 A64])
so the information that e is 12 bytes long is harder to find (of course, one
can still look at MEM_EXPR's size).
Then store_bit_field_1 sees i386 has HAVE_insv and op_mode for -m64 insv is
DImode.  The first HAVE_insv if doesn't apply, as i386 (like also at least half
of other targets that HAVE_insv) doesn't allow memory on op0.
But then the if (HAVE_insv && MEM_P (op0)) triggers and it calls get_best_mode,
which for volatile (this case) and !targetm.narrow_bit_fields () decides to
enlarge the mode as much as possible and this "possible" takes into account
just
MEM_ALIGN (which comes from DECL_ALIGN and thus is large here) and the passed
in largest_mode (for insv it comes from insv insn mode).  It doesn't consider
the size of the decl (but see above, expand_assignment made it impossible to
use MEM_SIZE here).  Now, if this if (HAVE_insv && MEM_P (op0)) case is
disabled, it still generates wrong code, as store_fixed_bit_field does
essentially the same, just instead of insv's mode uses word_mode as
largest_mode passed to get_best_mode.

Now, the question is what we want to use as largest_mode in these cases.
Obviously it can't be too large to go beyond end of the object's size here (the
case in this testcase, the reason why sched2 reorders the two stores is because
it sees the first store using SYMBOL_REF ("e") as base and the second store
using SYMBOL_REF ("f") and assumes that those can't alias).  For OpenMP or
C++0x
memory model that isn't enough I guess, even if you have:
struct S
{
  signed a : 26;
  signed b : 16;
  signed c : 10;
  volatile signed d : 14;
  int e;
} s;
I think you can't just modify s.e when writing s.d (I think it is fine to
modify
adjacent bitfields though, Aldy?).  So for C++0x memory model we probably want
to find the next non-bitfield field and use that or something similar.

Reply via email to