Hi Sandra,
On Sun, 20 Oct 2013 20:23:49, Sandra Loosemore wrote: > > 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 > I have attached an update to your patch, that should a) fix the recursion problem. b) restrict the -fstrict-volatile-bitfields to not violate the C++ memory model. Bernd.
patch-bitfields-update.diff
Description: Binary data
