Hi,
On Sun, 20 Oct 2013 20:23:49, Sandra Loosemore wrote:
> 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
>
You are right,
if I modify pr56997-1 the patch crashes on trunk:
typedef struct s{
char Prefix;
short Type;
}__attribute__((packed,aligned(4))) ss;
please add a test case for this.
This way we have op0 aligned 32 and HI mode selected bitoffset=8
numbits=16.
crashes only when reading this value, because the access tries to use
SImode.
For some reason the alignment seems to be wrong in 4.8.
Thanks
Bernd.
