On Tue, 3 Sep 2013 10:53:10, Richard Biener wrote:
> 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.
>
I'm sorry for that, but with the BIT_FIELD_REF I was not able to recover
the declaration mode of the bit field member. And this happened
for some of the test cases I wanted to have warnings on.
If I remember correctly this applies to code like:
if (x.x == 1)
{ ... }
> 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
>
Yes, that is a volatile bitfield access.
> volatile struct X x;
> x.x = 1;
>
> a volatile bitifled access?
>
Yes, that too is a volatile bitfiled access.
Someone please correct me if I am wrong.
> 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)
>
Yes, except for the above use cases.
These may be difficult to find without a warning.
> Richard.
>
>
>> Regards
>> Bernd Edlinger
