* [PATCH] Strict volatile bit-fields clean-up
@ 2013-11-20 13:38 Bernd Edlinger
2013-12-02 14:43 ` Richard Biener
0 siblings, 1 reply; 11+ messages in thread
From: Bernd Edlinger @ 2013-11-20 13:38 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1087 bytes --]
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?
Thanks
Bernd.
FYI - these are my in-flight patches, which would be nice to go into the GCC 4.9.0 release:
http://gcc.gnu.org/ml/gcc-patches/2013-11/msg02046.html :[PATCH] reimplement -fstrict-volatile-bitfields v4, part 1/2
http://gcc.gnu.org/ml/gcc-patches/2013-11/msg02025.html :[PATCH] Fix C++0x memory model for -fno-strict-volatile-bitfields on ARM
http://gcc.gnu.org/ml/gcc-patches/2013-11/msg02291.html :
[PATCH, PR 57748] Check for out of bounds access, Part 2
http://gcc.gnu.org/ml/gcc-patches/2013-11/msg00581.html :[PATCH, PR 57748] Check for out of bounds access, Part 3
http://gcc.gnu.org/ml/gcc-patches/2013-11/msg00133.html:[PATCH] Fix PR58115
[-- Attachment #2: changelog-bitfields-cleanup.txt --]
[-- Type: text/plain, Size: 213 bytes --]
2013-11-20 Bernd Edlinger <bernd.edlinger@hotmail.de>
* expr.c (expand_assignment): Remove dependency on
flag_strict_volatile_bitfields. Always set the memory
access mode.
(expand_expr_real_1): Likewise.
[-- Attachment #3: patch-bitfields-cleanup.diff --]
[-- Type: application/octet-stream, Size: 2666 bytes --]
--- gcc/expr.c.jj 2013-11-19 16:15:50.166853838 +0100
+++ gcc/expr.c 2013-11-20 10:41:16.793695891 +0100
@@ -4731,13 +4731,13 @@ expand_assignment (tree to, tree from, b
to_rtx = expand_expr (tem, NULL_RTX, VOIDmode, EXPAND_WRITE);
- /* If the bitfield is volatile, we want to access it in the
+ /* If the field has a mode, we want to access it in the
field's mode, not the computed mode.
If a MEM has VOIDmode (external with incomplete type),
use BLKmode for it instead. */
if (MEM_P (to_rtx))
{
- if (volatilep && flag_strict_volatile_bitfields > 0)
+ if (mode1 != VOIDmode)
to_rtx = adjust_address (to_rtx, mode1, 0);
else if (GET_MODE (to_rtx) == VOIDmode)
to_rtx = adjust_address (to_rtx, BLKmode, 0);
@@ -9866,13 +9866,13 @@ expand_expr_real_1 (tree exp, rtx target
modifier == EXPAND_SUM ? EXPAND_NORMAL : modifier,
NULL, true);
- /* If the bitfield is volatile, we want to access it in the
+ /* If the field has a mode, we want to access it in the
field's mode, not the computed mode.
If a MEM has VOIDmode (external with incomplete type),
use BLKmode for it instead. */
if (MEM_P (op0))
{
- if (volatilep && flag_strict_volatile_bitfields > 0)
+ if (mode1 != VOIDmode)
op0 = adjust_address (op0, mode1, 0);
else if (GET_MODE (op0) == VOIDmode)
op0 = adjust_address (op0, BLKmode, 0);
@@ -10002,17 +10002,13 @@ expand_expr_real_1 (tree exp, rtx target
&& modifier != EXPAND_CONST_ADDRESS
&& modifier != EXPAND_INITIALIZER
&& modifier != EXPAND_MEMORY)
- /* If the field is volatile, we always want an aligned
- access. Do this in following two situations:
- 1. the access is not already naturally
- aligned, otherwise "normal" (non-bitfield) volatile fields
- become non-addressable.
- 2. the bitsize is narrower than the access size. Need
- to extract bitfields from the access. */
- || (volatilep && flag_strict_volatile_bitfields > 0
- && (bitpos % GET_MODE_ALIGNMENT (mode) != 0
- || (mode1 != BLKmode
- && bitsize < GET_MODE_SIZE (mode1) * BITS_PER_UNIT)))
+ /* If the bitfield is volatile and the bitsize
+ is narrower than the access size of the bitfield,
+ we need to extract bitfields from the access. */
+ || (volatilep && TREE_CODE (exp) == COMPONENT_REF
+ && DECL_BIT_FIELD_TYPE (TREE_OPERAND (exp, 1))
+ && mode1 != BLKmode
+ && bitsize < GET_MODE_SIZE (mode1) * BITS_PER_UNIT)
/* If the field isn't aligned enough to fetch as a memref,
fetch it as a bit field. */
|| (mode1 != BLKmode
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Strict volatile bit-fields clean-up
2013-11-20 13:38 [PATCH] Strict volatile bit-fields clean-up Bernd Edlinger
@ 2013-12-02 14:43 ` Richard Biener
2013-12-03 9:47 ` Bernd Edlinger
0 siblings, 1 reply; 11+ messages in thread
From: Richard Biener @ 2013-12-02 14:43 UTC (permalink / raw)
To: Bernd Edlinger; +Cc: gcc-patches
On Wed, Nov 20, 2013 at 11:48 AM, Bernd Edlinger
<bernd.edlinger@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.
> Thanks
> Bernd.
>
>
> FYI - these are my in-flight patches, which would be nice to go into the GCC 4.9.0 release:
>
>
> http://gcc.gnu.org/ml/gcc-patches/2013-11/msg02046.html :[PATCH] reimplement -fstrict-volatile-bitfields v4, part 1/2
> http://gcc.gnu.org/ml/gcc-patches/2013-11/msg02025.html :[PATCH] Fix C++0x memory model for -fno-strict-volatile-bitfields on ARM
> http://gcc.gnu.org/ml/gcc-patches/2013-11/msg02291.html :
> [PATCH, PR 57748] Check for out of bounds access, Part 2
> http://gcc.gnu.org/ml/gcc-patches/2013-11/msg00581.html :[PATCH, PR 57748] Check for out of bounds access, Part 3
> http://gcc.gnu.org/ml/gcc-patches/2013-11/msg00133.html:[PATCH] Fix PR58115
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH] Strict volatile bit-fields clean-up
2013-12-02 14:43 ` Richard Biener
@ 2013-12-03 9:47 ` Bernd Edlinger
2013-12-03 9:59 ` Richard Biener
2013-12-03 10:34 ` Eric Botcazou
0 siblings, 2 replies; 11+ messages in thread
From: Bernd Edlinger @ 2013-12-03 9:47 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches, Eric Botcazou
On Mon, 2 Dec 2013 15:42:48, Richard Biener wrote:
>
> On Wed, Nov 20, 2013 at 11:48 AM, Bernd Edlinger
> <bernd.edlinger@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,
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);
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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Strict volatile bit-fields clean-up
2013-12-03 9:47 ` Bernd Edlinger
@ 2013-12-03 9:59 ` Richard Biener
2013-12-03 10:04 ` Richard Biener
2013-12-03 10:09 ` Bernd Edlinger
2013-12-03 10:34 ` Eric Botcazou
1 sibling, 2 replies; 11+ messages in thread
From: Richard Biener @ 2013-12-03 9:59 UTC (permalink / raw)
To: Bernd Edlinger; +Cc: gcc-patches, Eric Botcazou
On Tue, Dec 3, 2013 at 10:47 AM, Bernd Edlinger
<bernd.edlinger@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.edlinger@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 ...
> 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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Strict volatile bit-fields clean-up
2013-12-03 9:59 ` Richard Biener
@ 2013-12-03 10:04 ` Richard Biener
2013-12-03 10:09 ` Bernd Edlinger
1 sibling, 0 replies; 11+ messages in thread
From: Richard Biener @ 2013-12-03 10:04 UTC (permalink / raw)
To: Bernd Edlinger; +Cc: gcc-patches, Eric Botcazou
On Tue, Dec 3, 2013 at 10:59 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Tue, Dec 3, 2013 at 10:47 AM, Bernd Edlinger
> <bernd.edlinger@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.edlinger@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 ...
Btw, I see DECL_BIT_FIELD and DECL_BIT_FIELD_TYPE set (but QImode
used) for 'v' on x86_64 and the testcase "works".
>> 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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH] Strict volatile bit-fields clean-up
2013-12-03 9:59 ` Richard Biener
2013-12-03 10:04 ` Richard Biener
@ 2013-12-03 10:09 ` Bernd Edlinger
2013-12-03 11:04 ` Eric Botcazou
1 sibling, 1 reply; 11+ messages in thread
From: Bernd Edlinger @ 2013-12-03 10:09 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches, Eric Botcazou
On Tue, 3 Dec 2013 10:59:15, Richard Biener wrote:
>
> On Tue, Dec 3, 2013 at 10:47 AM, Bernd Edlinger
> <bernd.edlinger@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.edlinger@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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Strict volatile bit-fields clean-up
2013-12-03 9:47 ` Bernd Edlinger
2013-12-03 9:59 ` Richard Biener
@ 2013-12-03 10:34 ` Eric Botcazou
1 sibling, 0 replies; 11+ messages in thread
From: Eric Botcazou @ 2013-12-03 10:34 UTC (permalink / raw)
To: Bernd Edlinger; +Cc: gcc-patches, Richard Biener
> 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, 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.
This needs to be investigated because the intent is very clear in gigi, see
create_field_decl and finish_record_type: we set DECL_BIT_FIELD conservatively
and clear it only when we know that we can. This code is quite old now.
> 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!)...
The test indeed raises Program_Error on x86-64.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Strict volatile bit-fields clean-up
2013-12-03 10:09 ` Bernd Edlinger
@ 2013-12-03 11:04 ` Eric Botcazou
2013-12-03 11:51 ` Bernd Edlinger
0 siblings, 1 reply; 11+ messages in thread
From: Eric Botcazou @ 2013-12-03 11:04 UTC (permalink / raw)
To: Bernd Edlinger; +Cc: gcc-patches, Richard Biener
> 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.
Yes, it should be that way, and everything else is a bug.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH] Strict volatile bit-fields clean-up
2013-12-03 11:04 ` Eric Botcazou
@ 2013-12-03 11:51 ` Bernd Edlinger
2013-12-03 12:01 ` Eric Botcazou
0 siblings, 1 reply; 11+ messages in thread
From: Bernd Edlinger @ 2013-12-03 11:51 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches, Richard Biener
>
>> 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.
>
> Yes, it should be that way, and everything else is a bug.
>
I tried something like this, and it did boot-strap, but failed in some acats tests:
/* If the bitfield is volatile, we want to access it in the
field's mode, not the computed mode.
If a MEM has VOIDmode (external with incomplete type),
use BLKmode for it instead. */
if (MEM_P (to_rtx))
{
+ gcc_assert ((TREE_CODE (to) == COMPONENT_REF && DECL_BIT_FIELD (TREE_OPERAND (to, 1)))
+ || (bitpos % BITS_PER_UNIT == 0 && bitsize % BITS_PER_UNIT == 0));
if (volatilep && flag_strict_volatile_bitfields> 0)
to_rtx = adjust_address (to_rtx, mode1, 0);
Bernd.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Strict volatile bit-fields clean-up
2013-12-03 11:51 ` Bernd Edlinger
@ 2013-12-03 12:01 ` Eric Botcazou
2013-12-03 12:04 ` Bernd Edlinger
0 siblings, 1 reply; 11+ messages in thread
From: Eric Botcazou @ 2013-12-03 12:01 UTC (permalink / raw)
To: Bernd Edlinger; +Cc: gcc-patches, Richard Biener
> I tried something like this, and it did boot-strap, but failed in some acats
> tests:
Am I supposed to guess which ones? Note that you can have non-byte-aligned
aggregate fields in Ada, i.e. arrays, so ARRAY_REFs can presumably reach here.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH] Strict volatile bit-fields clean-up
2013-12-03 12:01 ` Eric Botcazou
@ 2013-12-03 12:04 ` Bernd Edlinger
0 siblings, 0 replies; 11+ messages in thread
From: Bernd Edlinger @ 2013-12-03 12:04 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches, Richard Biener
----------------------------------------
> From: ebotcazou@adacore.com
> To: bernd.edlinger@hotmail.de
> CC: gcc-patches@gcc.gnu.org; richard.guenther@gmail.com
> Subject: Re: [PATCH] Strict volatile bit-fields clean-up
> Date: Tue, 3 Dec 2013 13:00:25 +0100
>
>> I tried something like this, and it did boot-strap, but failed in some acats
>> tests:
>
> Am I supposed to guess which ones? Note that you can have non-byte-aligned
> aggregate fields in Ada, i.e. arrays, so ARRAY_REFs can presumably reach here.
>
> --
> Eric Botcazou
hmm, probably yes. that was a long night. sorry. they all had to do with nested views,
I think.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2013-12-03 12:04 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-20 13:38 [PATCH] Strict volatile bit-fields clean-up Bernd Edlinger
2013-12-02 14:43 ` Richard Biener
2013-12-03 9:47 ` Bernd Edlinger
2013-12-03 9:59 ` Richard Biener
2013-12-03 10:04 ` Richard Biener
2013-12-03 10:09 ` Bernd Edlinger
2013-12-03 11:04 ` Eric Botcazou
2013-12-03 11:51 ` Bernd Edlinger
2013-12-03 12:01 ` Eric Botcazou
2013-12-03 12:04 ` Bernd Edlinger
2013-12-03 10:34 ` Eric Botcazou
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).