On Tue, Dec 3, 2013 at 10:47 AM, Bernd Edlinger
<[email protected]> wrote:
> On Mon, 2 Dec 2013 15:42:48, Richard Biener wrote:
>>
>> On Wed, Nov 20, 2013 at 11:48 AM, Bernd Edlinger
>> <[email protected]> wrote:
>>> Hello Richard,
>>>
>>> as a follow-up patch to the bit-fields patch(es), I wanted to remove the
>>> dependencies on
>>> the variable flag_strict_volatile_bitfields from expand_assignment and
>>> expand_expr_real_1.
>>> Additionally I want the access mode of the field to be selected in the
>>> memory context,
>>> instead of the structure's mode.
>>>
>>> Boot-strapped and regression-tested on x86_64-linux-gnu.
>>>
>>> OK for trunk?
>>
>> Ok.
>>
>> Thanks,
>> Richard.
>>
>
>
> Oops....
>
> Sorry, tonight this patch caused an Ada regression, in pack17.adb and
> misaligned_nest.adb
>
> So I'll have to put that on hold at the moment.
>
> This ICE is very similar to PR59134.
> It is again the recursion between store_fixed_bit_field and
> store_split_bit_field.
>
> The trigger is a latent problem in the ada gcc_interface.
>
> That is we have a bit-field of exactly 8 bit size, which is not byte-aligned,
> but DECL_MODE=QImode, DECL_BIT_FIELD=false which looks quite strange,
It's not required that DECL_BIT_FIELD is set I think, and the mode
is that of the type if it's not a bitfield ...
> and is totally different from how C structures look like. I should mention
> that there
> are even some places in the target back-ends, where the attribute
> DECL_BIT_FIELD is
> used for whatever.
>
> Now, due to this hunk in the cleanup-patch we get the QImode selected in the
> memory
> context:
>
> if (MEM_P (to_rtx))
> {
> - if (volatilep && flag_strict_volatile_bitfields> 0)
> + if (mode1 != VOIDmode)
> to_rtx = adjust_address (to_rtx, mode1, 0);
Which I think is correct - the memory access is in QImode - that's what
get_inner_reference said.
So whatever issue we run into downstream points to the bug ...
OTOH Eric may know better what the middle-end can expect for
bit-aligned Ada records (apart from "all bets are off" which isn't
really a good solution ;))
Richard.
> However even without that patch, I can arrange for "volatilep &&
> flag_strict_volatile_bitfields> 0"
> to be true in Ada (even on X86_64, or whatever platform you like):
>
> -- { dg-do run }
> -- { dg-options "-gnatp -fstrict-volatile-bitfields" }
>
> procedure Misaligned_Volatile is
>
> type Byte is mod 2**8;
>
> type Block is record
> B : Boolean;
> V : Byte;
> end record;
> pragma Volatile (Block);
> pragma Pack (Block);
> for Block'Alignment use 1;
>
> type Pair is array (1 .. 2) of Block;
>
> P : Pair;
> begin
> for K in P'Range loop
> P(K).V := 237;
> end loop;
> for K in P'Range loop
> if P(K).V /= 237 then
> raise Program_error;
> end if;
> end loop;
> end;
>
>
> This Ada test case causes either wrong code generation or an ICE at compile
> time,
> if the -fstrict-volatile-bitfields option is either given by the user,
> or by the target-specific default as it is on ARM for instance (which is
> completely
> pointless on Ada, I know!)...
>
> Now I am preparing a new bitfields-update-patch which fixes this above test
> case and the
> latent recursion problem.
>
>
> Thanks ... for you patience :-(
> Bernd.
