Re: [C++0x] contiguous bitfields race implementation

[Aldy Hernandez <aldyh at redhat dot com>]

On 07/22/11 13:44, Jason Merrill wrote:
> On 07/18/2011 08:02 AM, Aldy Hernandez wrote:
> > + /* If other threads can't see this value, no need to restrict
> > stores. */
> > + if (ALLOW_STORE_DATA_RACES
> > + || !DECL_THREAD_VISIBLE_P (innerdecl))
> > + {
> > + *bitstart = *bitend = 0;
> > + return;
> > + }
>
> What if get_inner_reference returns something that isn't a DECL, such as
> an INDIRECT_REF?

I had changed this already to take into account aliasing, so if we get 
an INDIRECT_REF, ptr_deref_may_alias_global_p() returns true, and we 
proceed with the restriction:

+  /* If other threads can't see this value, no need to restrict stores.  */
+  if (ALLOW_STORE_DATA_RACES
+      || (!ptr_deref_may_alias_global_p (innerdecl)
+         && (DECL_THREAD_LOCAL_P (innerdecl)
+             || !TREE_STATIC (innerdecl))))


> > + if (fld)
> > + {
> > + /* We found the end of the bit field sequence. Include the
> > + padding up to the next field and be done. */
> > + *bitend = bitpos - 1;
> > + }
>
> bitpos is the position of "field", and it seems to me we want the
> position of "fld" here.

Notice that bitpos gets recalculated at each iteration by 
get_inner_reference, so bitpos is actually the position of fld.

> > + /* If unset, no restriction. */
> > + if (!bitregion_end)
> > + maxbits = 0;
> > + else
> > + maxbits = (bitregion_end - bitregion_start) % align;
>
> Maybe use MAX_FIXED_MODE_SIZE so you don't have to test it against 0?

Fixed everywhere.

> > + if (!bitregion_end)
> > + maxbits = 0;
> > + else if (1||bitpos + offset * BITS_PER_UNIT < bitregion_start)
> > + maxbits = bitregion_end - bitregion_start;
> > + else
> > + maxbits = bitregion_end - (bitpos + offset * BITS_PER_UNIT) + 1;
>
> I assume the 1|| was there for debugging?

Fixed, plus I adjusted the calculation of maxbits everywhere because I 
found an off-by-one error.

I have also overhauled store_bit_field() to adjust the address of the 
address to point to the beginning of the bit region.  This fixed a 
myraid of corner cases pointed out by a test Hans Boehm was kind enough 
to provide.

I have added more tests.

How does this look?  (Pending tests.)

Attachment: curr
Description: Text document