This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] reimplement -fstrict-volatile-bitfields v4, part 1/2


On 10/18/2013 10:38 AM, Richard Biener wrote:
Sandra Loosemore <sandra@codesourcery.com> wrote:
On 10/18/2013 04:50 AM, Richard Biener wrote:
On Sat, Sep 28, 2013 at 4:19 AM, Sandra Loosemore
<sandra@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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]