On Wed, 20 Jul 2016, Jeff Law wrote:
> On 06/29/2016 05:54 AM, Richard Biener wrote:
> >
> > Currently as-base classes lack DECL_BIT_FIELD_REPRESENTATIVEs which
> > means RTL expansion doesn't honor the C++ memory model for bitfields
> > in them thus for the following testcase
> >
> > struct B {
> > B() {}
> > int x;
> > int a : 6;
> > int b : 6;
> > int c : 6;
> > };
> >
> > struct C : B {
> > char d;
> > };
> >
> > C c;
> >
> > int main()
> > {
> > c.c = 1;
> > c.d = 2;
> > }
> >
> > on x86 we happily store to c.c in a way creating a store data race with
> > c.d:
> >
> > main:
> > .LFB6:
> > .cfi_startproc
> > movl c+4(%rip), %eax
> > andl $-258049, %eax
> > orb $16, %ah
> > movl %eax, c+4(%rip)
> > movb $2, c+7(%rip)
> > xorl %eax, %eax
> > ret
> >
> > Fixing the lack of DECL_BIT_FIELD_REPRESENTATIVEs in as-base
> > classes doesn't help though as the C++ FE builds access trees
> > for c.c using the non-as-base class FIELD_DECLs which is because
> > of layout_class_type doing
> >
> > /* Now that we're done with layout, give the base fields the real types.
> > */
> > for (field = TYPE_FIELDS (t); field; field = DECL_CHAIN (field))
> > if (DECL_ARTIFICIAL (field) && IS_FAKE_BASE_TYPE (TREE_TYPE (field)))
> > TREE_TYPE (field) = TYPE_CONTEXT (TREE_TYPE (field));
> >
> > this would basically require us to always treat tail-padding in a
> > struct conservatively in finish_bitfield_representative (according
> > to the doubt by the ??? comment I patch out below).
> >
> > Simply commenting out the above makes fixing build_simple_base_path
> > necessary but even after that it then complains in the verifier
> > later ("type mismatch in component reference" - as-base to class
> > assignment).
> >
> > But it still somehow ends up using the wrong FIELD_DECL in the end.
> >
> > Now I think we need to fix the wrong-code issue somehow and
> > doing so in stor-layout.c by conservatively treating tail-padding
> > is a possibility. But that will pessimize PODs and other languages
> > unless we have a way to know whether a RECORD_TYPE possibly can
> > have its tail-padding re-used (I'd hate to put a lang_hooks.name
> > check there and even that would pessimize C++ PODs).
> >
> > Any guidance here?
> Note that if we change tail-padding re-use properties, then we effectively
> have an ABI change. Given that, the only path forward is to use smaller
> memory operations.
Yes, we can't change the ABI. It seems the most straigt-forward solution
is to add a langhook we can query from stor-layout that says whether
tail padding of an aggregate might be re-used.
> Do any other compilers gets this right (LLVM?)
clang++ 3.6.1 gives me
movzbl c+6(%rip), %eax
shll $16, %eax
movzwl c+4(%rip), %ecx
orl %eax, %ecx
andl $16519167, %ecx # imm = 0xFC0FFF
leal 4096(%rcx), %eax
shrl $16, %ecx
movb %cl, c+6(%rip)
movw %ax, c+4(%rip)
movb $2, c+7(%rip)
xorl %eax, %eax
retq
so it avoids the store data race (it even avoids load data races).
Not sure if it does because of the C++ memory model or lack of
optimization.
Richard.
> jeff
>
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
