On Tue, 3 Dec 2013 10:59:15, Richard Biener wrote:
>
> On Tue, Dec 3, 2013 at 10:47 AM, Bernd Edlinger
> <bernd.edlin...@hotmail.de> wrote:
>> On Mon, 2 Dec 2013 15:42:48, Richard Biener wrote:
>>>
>>> On Wed, Nov 20, 2013 at 11:48 AM, Bernd Edlinger
>>> <bernd.edlin...@hotmail.de> 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 ...
>
I agree. But I hope the back-ends agree too.
>> 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.
>
I absolutely agree. I want it this way everything else is nonsense.
> So whatever issue we run into downstream points to the bug ...
>
That's for sure. I think I have a good idea, how to fix the root of the
recursion.
I just have to write a change-log and will send an update in a few moments.
Thanks
Bernd.
> 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 ;))
>
Yes. I somehow did expect DECL_BIT_FIELD to be something simple,
without dependency on if -fstrict-volatile-bitfields is given, or what
Language front-end generated it, or if STRICT_ALIGNMENT is defined.
> 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.
