On December 19, 2016 11:25:49 PM GMT+01:00, Jeff Law <l...@redhat.com> wrote:
>On 12/14/2016 03:44 AM, Richard Biener wrote:
>>
>> The following implements Jasons suggestion of using a langhook to
>> return the size of an aggregate without tail padding that might
>> be re-used when it is inherited from.
>>
>> Using this langhook we can fix the size of the representative for the
>> bitfield properly and thus generate correct code for the testcase.
>> Previously on x86_64 it was
>>
>> main:
>> 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
>>
>> while after the patch we see
>>
>> main:
>> movzbl c+5(%rip), %eax
>> andb $-4, c+6(%rip)
>> movb $2, c+7(%rip)
>> andl $15, %eax
>> orl $16, %eax
>> movb %al, c+5(%rip)
>> xorl %eax, %eax
>> ret
>>
>> in particular the store to C::B::d is now properly using a byte
>store.
>>
>> Bootstrap and regtest in progress on x86_64-unknown-linux-gnu, ok for
>> trunk?
>>
>> Thanks,
>> Richard.
>>
>> 2016-12-14 Richard Biener <rguent...@suse.de>
>>
>> PR c++/71694
>> * langhooks-def.h (lhd_unit_size_without_reusable_padding): Declare.
>> (LANG_HOOKS_UNIT_SIZE_WITHOUT_REUSABLE_PADDING): Define.
>> (LANG_HOOKS_FOR_TYPES_INITIALIZER): Adjust.
>> * langhooks.h (struct lang_hooks_for_types): Add
>> unit_size_without_reusable_padding.
>> * langhooks.c (lhd_unit_size_without_reusable_padding): New.
>> * stor-layout.c (finish_bitfield_representative): Use
>> unit_size_without_reusable_padding langhook to decide on the
>> last representatives size.
>>
>> cp/
>> * cp-objcp-common.h (cp_unit_size_without_reusable_padding):
>Declare.
>> (LANG_HOOKS_UNIT_SIZE_WITHOUT_REUSABLE_PADDING): Define.
>> * cp-objcp-common.c (cp_unit_size_without_reusable_padding): New.
>>
>> * g++.dg/pr71694.C: New testcase.
>>
>> Index: gcc/stor-layout.c
>> ===================================================================
>> *** gcc/stor-layout.c (revision 243632)
>> --- gcc/stor-layout.c (working copy)
>> *************** finish_bitfield_representative (tree rep
>> *** 1864,1876 ****
>> }
>> else
>> {
>> ! /* ??? If you consider that tail-padding of this struct
>might be
>> ! re-used when deriving from it we cannot really do the
>following
>> ! and thus need to set maxsize to bitsize? Also we cannot
>> ! generally rely on maxsize to fold to an integer constant, so
>> ! use bitsize as fallback for this case. */
>> ! tree maxsize = size_diffop (TYPE_SIZE_UNIT (DECL_CONTEXT
>(field)),
>> ! DECL_FIELD_OFFSET (repr));
>> if (tree_fits_uhwi_p (maxsize))
>> maxbitsize = (tree_to_uhwi (maxsize) * BITS_PER_UNIT
>> - tree_to_uhwi (DECL_FIELD_BIT_OFFSET (repr)));
>> --- 1864,1877 ----
>> }
>> else
>> {
>> ! /* Note that if the C++ FE sets up tail-padding to be re-used
>it
>> ! creates a as-base variant of the type with TYPE_SIZE
>adjusted
>> ! accordingly. So it is safe to include tail-padding here. */
>> ! tree aggsize =
>lang_hooks.types.unit_size_without_reusable_padding
>> ! (DECL_CONTEXT (field));
>> ! tree maxsize = size_diffop (aggsize, DECL_FIELD_OFFSET
>(repr));
>> ! /* We cannot generally rely on maxsize to fold to an integer
>constant,
>> ! so use bitsize as fallback for this case. */
>> if (tree_fits_uhwi_p (maxsize))
>> maxbitsize = (tree_to_uhwi (maxsize) * BITS_PER_UNIT
>> - tree_to_uhwi (DECL_FIELD_BIT_OFFSET (repr)));
>Looks reasonable, formatting nit in the comment above the assignment to
>
>AGGSIZE (looks like it was mucked up in the old version too). I think
>the second line is using spaces when it should have used a tab.
>
>Do we document langhooks anywhere?
Didn't find anything. Applied already after Jason's OK.
Richard.
>Jeff
