On 10/18/2013 10:38 AM, Richard Biener wrote:
Sandra Loosemore <san...@codesourcery.com> wrote:
On 10/18/2013 04:50 AM, Richard Biener wrote:
On Sat, Sep 28, 2013 at 4:19 AM, Sandra Loosemore
<san...@codesourcery.com> wrote:
This patch fixes various -fstrict-volatile-bitfields wrong-code
bugs,
including instances where bitfield values were being quietly
truncated as
well as bugs relating to using the wrong width. The code changes
are
identical to those in the previous version of this patch series
(originally
posted in June and re-pinged many times since then), but for this
version I
have cleaned up the test cases to remove dependencies on header
files, as
Bernd requested.
Ok.
Just to clarify, is this approval unconditional, independent of part 2
and other patches or changes still under active discussion?
Yes.
Hrmmmm. After some further testing, I'm afraid this patch is still
buggy. :-(
I tried a backport to GCC 4.8 and tested on arm-none-eabi. On the new
pr56997-1.c testcase, it got stuck in infinite recursion between
store_split_bit_field/store_fixed_bit_field and/or
extract_split_bit_field/extract_fixed_bit_field. This didn't show up in
my previous mainline testing.
The difference between 4.8 and mainline head is the alignment of the
incoming str_rtx passed to store_bit_field/extract_bit_field, due to the
changes in r199898. The alignment is 8 bits on mainline, and 32 on 4.8.
It seems to me that the bitfield code ought to handle store/extract
from a more-aligned object and it's probably possible to construct an
example that fails in this way on mainline as well.
It looks like there are conflicting assumptions between get_best_mode,
the places that call it in store_fixed_bit_field and
extract_fixed_bit_field, and the code that actually does the splitting
-- which uses a unit based on the MEM_ALIGN of the incoming rtx, rather
than its mode. In the case where it's failing, get_best_mode is
deciding it can't do a HImode access without splitting, but then the
split code is assuming SImode units because of the 32-bit alignment, but
not actually changing the mode of the rtx to match that.
On top of that, this is one of the cases that strict_volatile_bitfield_p
checks for and returns false, but the callers of store_bit_field and
extract_bit_field in expr.c have already fiddled with the mode of the
incoming rtx assuming that -fstrict-volatile-bitfields does apply. It
doesn't get into that infinite recursion if it's compiled with
-fno-strict-volatile-bitfields instead; in that case the incoming rtx
has BLKmode, get_best_mode chooses SImode, and it's able to do the
access without splitting at all.
Anyway.... I tried a couple different bandaids that solved the infinite
recursion problem but caused regressions elsewhere, and now I'm not sure
of the right place to fix this. Given that there is also still ongoing
discussion about making this a 3-way switch (etc) I am going to hold off
on committing this patch for now.
-Sandra
