On Tue, Sep 3, 2013 at 2:05 AM, Bernd Edlinger
<bernd.edlin...@hotmail.de> wrote:
> This is a follow-up patch for Sandra Loosemore's patch in this
> thread: "reimplement -fstrict-volatile-bitfields, v3".
> It was already posted a few weeks ago, but in the wrong thread.
> Therfore I re-post it herewith.
> It was initially suggested by Hans-Peter Nilsson, and I had much
> help from him in putting everything together. Thanks again H-P.
> Here is a short specification:
> The -Wportable-volatility warning is an optional warning, to warn
> about code for which separate incompatbile definitions on different
> platforms (or between C and C++) exist even within gcc.
> It will be usable for driver code you want to be portable on different
> architectures.
> This warning should only be emitted if the code is significantly
> different when -fstrict-volatile-bitfields is used or not.
> It should not be emitted for the code which is not affected by this
> option.
> In other words, it should be emitted on all bit-fields when the
> definition or the context is volatile, except when the whole
> structure is not AAPCS ABI compliant, i.e. packed or unaligned.
> On the other hand you should always get the same
> warnings if you combine -Wportable-volatility with
> -fstrict-volatile-bitfields or not. And of course it should not
> depend on the specific target that is used to compile.
> I boot-strapped this on a i686-pc-linux-gnu and all Wportable-volatility
> test cases are passed for C and C++.
> I used a cross-compiler for arm-eabi to manually cross-check that
> the warnings are independent of the target platform.
Just a quick first note:
@@ -3470,6 +3470,13 @@ optimize_bit_field_compare (location_t l
|| offset != 0 || TREE_CODE (linner) == PLACEHOLDER_EXPR)
return 0;
+ /* Do not try to optimize on volatiles, as it would defeat the
+ -Wportable-volatility warning later:
+ If the COMPONENT_REF is replaced by a BIT_FIELD_REF we loose
+ information and thus the warning cannot be generated correctly. */
+ if (warn_portable_volatility && lvolatilep)
+ return 0;
changing code-generation with a warning option looks wrong to me. Note
that I completely removed this code once (it also generates strange
BIT_FIELD_REFs) but re-instantiated it on request because of lost
optimizations.
Similar for
@@ -609,11 +609,14 @@ layout_decl (tree decl, unsigned int kno
occupying a complete byte or bytes on proper boundary,
and not -fstrict-volatile-bitfields. If the latter is set,
we unfortunately can't check TREE_THIS_VOLATILE, as a cast
- may make a volatile object later. */
+ may make a volatile object later.
+ Handle -Wportable-volatility analogous, as we would loose
+ information and defeat the warning later. */
if (TYPE_SIZE (type) != 0
&& TREE_CODE (TYPE_SIZE (type)) == INTEGER_CST
&& GET_MODE_CLASS (TYPE_MODE (type)) == MODE_INT
- && flag_strict_volatile_bitfields <= 0)
+ && flag_strict_volatile_bitfields <= 0
+ && warn_portable_volatility == 0)
{
enum machine_mode xmode
= mode_for_size_tree (DECL_SIZE (decl), MODE_INT, 1);
I also wonder about "we unfortunately can't check TREE_THIS_VOLATILE, as a cast
may make a volatile object later." - does that mean that AAPCS requires
struct X { int x : 8; char c; };
void foo (struct X *p)
{
((volatile struct X *)p)->x = 1;
}
to be treated as volatile bitfield access? Similar, is
volatile struct X x;
x.x = 1;
a volatile bitifled access?
I think the warning can be completely implemented inside struct-layout.c
for example in finish_bitfield_representative (if you pass that the first field
in the group, too). Of course that is at the expense of warning for
struct declarations rather than actual code differences (the struct may
not be used)
Richard.
> Regards
> Bernd Edlinger
