[C++0x] contiguous bitfields race implementation

Alright, I'm back and bearing patches.  Firmly ready for the crucifixion 
you will likely submit me to. :)

I've pretty much rewritten everything, taking into account all your 
suggestions, and adding a handful of tests for corner cases we will now 
handle correctly.

It seems the minimum needed is to calculate the byte offset of the start 
of the bit region, and the length of the bit region.  (Notice I say BYTE 
offset, as the start of any bit region will happily coincide with a byte 
boundary).  These will of course be adjusted as various parts of the 
bitfield infrastructure adjust offsets and memory addresses throughout.

First, it's not as easy as calling get_inner_reference() only once as 
you've suggested.  The only way to determine the padding at the end of a 
field is getting the bit position of the field following the field in 
question (or the size of the direct parent structure in the case where 
the field in question is the last field in the structure).  So we need 
two calls to get_inner_reference for the general case.  Which is at 
least better than my original call to get_inner_reference() for every field.

I have clarified the comments and made it clear what the offsets are 
relative to.

I am now handling large offsets that may appear as a tree OFFSET from 
get_inner_reference, and have added a test for one such corner case, 
including nested structures with head padding as you suggested.  I am 
still unsure that a variable length offset can happen before a bit field 
region.  So currently we assert that the final offset is host integer 
representable.  If you have a testcase that invalidates my assumption, I 
will gladly add a test and fix the code.

Honestly, the code isn't pretty, but neither is the rest of the bit 
field machinery.  I tried to make due, but I'll gladly take suggestions 
that are not in the form of "the entire bit field code needs to be 
rewritten" :-).

To aid in reviewing, the crux of everything is in the rewritten 
get_bit_range() and the first block of store_bit_field().  Everything 
else is mostly noise.  I have attached all of get_bit_range() as a 
separate attachment to aid in reviewing, since that's the main engine, 
and it has been largely rewritten.

This pacth handles all the testcases I could come up with, mostly 
inspired by your suggestions.  Eventually I would like to replace these 
target specific tests with target-agnostic tests using the gdb simulated 
thread test harness in the cxx-mem-model branch.

Finally, you had mentioned possible problems with tail padding in C++, 
and suggested I use DECL_SIZE instead of calculating the padding using 
the size of direct parent structure.  DECL_SIZE doesn't include padding, 
so I'm open to suggestions.

Fire away, but please be kind :).
* machmode.h (get_best_mode): Remove 2 arguments.
	* fold-const.c (optimize_bit_field_compare): Same.
	(fold_truthop): Same.
	* expr.c (store_field): Change argument types in prototype.
	(emit_group_store): Change argument types to store_bit_field call.
	(copy_blkmode_from_reg): Same.
	(write_complex_part): Same.
	(optimize_bitfield_assignment_op): Change argument types.
	Change arguments to get_best_mode.
	(get_bit_range): Rewrite.
	(expand_assignment): Adjust new call to get_bit_range.
	Adjust bitregion_offset when to_rtx is changed.
	Adjust calls to store_field with new argument types.
	(store_field): New argument types.
	Adjust calls to store_bit_field with new arguments.
	* expr.h (store_bit_field): Change argument types.
	* stor-layout.c (get_best_mode): Remove use of bitregion* arguments.
	* expmed.c (store_bit_field_1): Change argument types.
	Do not calculate maxbits.
	Adjust bitregion_maxbits if offset changes.
	(store_bit_field): Change argument types.
	Adjust address taking into account bitregion_offset.
	(store_fixed_bit_field): Change argument types.
	Do not calculate maxbits.
	(store_split_bit_field): Change argument types.
	(extract_bit_field_1): Adjust arguments to get_best_mode.
	(extract_fixed_bit_field): Same.
/* In the C++ memory model, consecutive bit fields in a structure are
   considered one memory location.

   Given a COMPONENT_REF, this function calculates the byte offset of
   the beginning of the memory location containing bit field being
   referenced.  The byte offset is returned in *OFFSET and is the byte
   offset from the beginning of the containing object (INNERDECL).

   The largest mode that can be used to write into the bit field will
   be returned in *LARGEST_MODE.

   For example, in the following structure, the bit region starts in
   byte 4.  In an architecture where the size of BITS gets padded to
   32-bits, SImode will be returned in *LARGEST_MODE.

     struct bits {
       int some_padding;
       struct {
         volatile char bitfield :1;
       } bits;
       char b;
     };

   EXP is the COMPONENT_REF.

   Examples.

   While storing into FOO.A here...

      struct {
        BIT 0:
          unsigned int a : 4;
	  unsigned int b : 1;
	BIT 8:
	  unsigned char c;
	  unsigned int d : 6;
      } foo;

   ...we are not allowed to store past <b>, so for the layout above,
   *OFFSET will be byte 0, and *LARGEST_MODE will be QImode.

   Here we have 3 distinct memory locations because of the zero-sized
   bit-field separating the bits:

     struct bits
     {
       char a;
       int b:7;
       int :0;
       int c:7;
     } foo;

   Here we also have 3 distinct memory locations because
   structure/union boundaries will separate contiguous bit-field
   sequences:

     struct {
       char a:3;
       struct { char b:4; } x;
       char c:5;
     } foo;  */

static void
get_bit_range (tree exp, tree *offset, HOST_WIDE_INT *maxbits)
{
  tree field, record_type, fld;
  bool found_field = false;
  bool prev_field_is_bitfield;
  tree start_offset, end_offset, maxbits_tree;
  tree start_bitpos_direct_parent = NULL_TREE;
  HOST_WIDE_INT start_bitpos, end_bitpos;
  HOST_WIDE_INT cumulative_bitsize = 0;

  gcc_assert (TREE_CODE (exp) == COMPONENT_REF);

  /* Bit field we're storing into.  */
  field = TREE_OPERAND (exp, 1);
  record_type = DECL_FIELD_CONTEXT (field);

  /* Count the contiguous bitfields for the memory location that
     contains FIELD.  */
  start_offset = size_zero_node;
  start_bitpos = 0;
  prev_field_is_bitfield = false;
  for (fld = TYPE_FIELDS (record_type); fld; fld = DECL_CHAIN (fld))
    {
      if (TREE_CODE (fld) != FIELD_DECL)
	continue;

      if (field == fld)
	found_field = true;

      /* If we have a bit-field with a bitsize > 0... */
      if (DECL_BIT_FIELD_TYPE (fld)
	  && (!host_integerp (DECL_SIZE (fld), 1)
	      || tree_low_cst (DECL_SIZE (fld), 1) > 0))
	{
	  /* Start of a new bit region.  */
	  if (prev_field_is_bitfield == false)
	    {
	      HOST_WIDE_INT bitsize;
	      enum machine_mode mode;
	      int unsignedp, volatilep;

	      /* Save starting bitpos and offset.  */
	      get_inner_reference (build3 (COMPONENT_REF,
					   TREE_TYPE (exp),
					   TREE_OPERAND (exp, 0),
					   fld, NULL_TREE),
				   &bitsize, &start_bitpos, &start_offset,
				   &mode, &unsignedp, &volatilep, true);
	      /* Save the bit offset of the current structure.  */
	      start_bitpos_direct_parent = DECL_FIELD_BIT_OFFSET (fld);
	      prev_field_is_bitfield = true;
	      cumulative_bitsize = 0;
	    }

	  cumulative_bitsize += tree_low_cst (DECL_SIZE (fld), 1);

	  /* Short-circuit out if we have the max bits allowed.  */
	  /* ?? Is this even worth it.  ?? */
	  if (cumulative_bitsize >= MAX_FIXED_MODE_SIZE)
	    {
	      *maxbits = MAX_FIXED_MODE_SIZE;
	      /* Calculate byte offset to the beginning of the bit region.  */
	      gcc_assert (start_bitpos % BITS_PER_UNIT == 0);
	      *offset = fold_build2 (PLUS_EXPR, TREE_TYPE (start_offset),
				     start_offset,
				     build_int_cst (integer_type_node,
						    start_bitpos / BITS_PER_UNIT));
	      return;
	    }
	}
      else
	{
	  prev_field_is_bitfield = false;
	  if (found_field)
	    break;
	}
    }
  gcc_assert (found_field);

  /* Calculate byte offset to the beginning of the bit region.  */
  /* OFFSET = START_OFFSET + (START_BITPOS / BITS_PER_UNIT) */
  gcc_assert (start_bitpos % BITS_PER_UNIT == 0);
  if (!start_offset)
    start_offset = size_zero_node;
  *offset = fold_build2 (PLUS_EXPR, TREE_TYPE (start_offset),
			 start_offset,
			 build_int_cst (integer_type_node,
					start_bitpos / BITS_PER_UNIT));
  if (fld)
    {
      HOST_WIDE_INT bitsize;
      enum machine_mode mode;
      int unsignedp, volatilep;

      /* We found the end of the bit field sequence.  Include the
	 padding up to the next field.  */

      /* Calculate bitpos and offset of the next field.  */
      get_inner_reference (build3 (COMPONENT_REF,
				   TREE_TYPE (exp),
				   TREE_OPERAND (exp, 0),
				   fld, NULL_TREE),
			   &bitsize, &end_bitpos, &end_offset,
			   &mode, &unsignedp, &volatilep, true);
      gcc_assert (end_bitpos % BITS_PER_UNIT == 0);

      if (end_offset)
	{
	  tree type = TREE_TYPE (end_offset), end;

	  /* Calculate byte offset to the end of the bit region.  */
	  end = fold_build2 (PLUS_EXPR, type,
			     end_offset,
			     build_int_cst (type,
					    end_bitpos / BITS_PER_UNIT));
	  maxbits_tree = fold_build2 (MINUS_EXPR, type, end, *offset);
	}
      else
	maxbits_tree = build_int_cst (integer_type_node,
				      end_bitpos - start_bitpos);

      /* ?? Can we get a variable-lengthened offset here ?? */
      gcc_assert (host_integerp (maxbits_tree, 1));
      *maxbits = TREE_INT_CST_LOW (maxbits_tree);
    }
  else
    {
      /* If this is the last element in the structure, include the padding
	 at the end of structure.  */
      *maxbits = TREE_INT_CST_LOW (TYPE_SIZE (record_type))
	- TREE_INT_CST_LOW (start_bitpos_direct_parent);
    }
}

[C++0x] contiguous bitfields race implementation

Commit Message

Comments

Patch