Hi,
This modified test case exposes a bug in the already approved part of the
strict-volatile-bitfields patch:
#include <stdlib.h>
typedef struct {
char pad;
int arr[0];
} __attribute__((packed)) str;
str *
foo (int* src)
{
str *s = malloc (sizeof (str) + sizeof (int));
s->arr[0] = 0x12345678;
asm volatile("":::"memory");
*src = s->arr[0];
return s;
}
As we know this test case triggered a recursion in the store_bit_field on ARM
and on PowerPC,
which is no longer reproducible after this patch is applied:
http://gcc.gnu.org/ml/gcc-patches/2013-11/msg02025.html
Additionally it triggered a recursion on extract_bit_field, but _only_ on my
local copy of the trunk.
I had this patch installed, but did not expect it to change anything unless the
values are volatile.
That was cased by this hunk in the strict-volatile-bitfields v4 patch:
@@ -1691,45 +1736,19 @@ extract_fixed_bit_field (enum machine_mo
includes the entire field. If such a mode would be larger than
a word, we won't be doing the extraction the normal way. */
- if (MEM_VOLATILE_P (op0)
- && flag_strict_volatile_bitfields> 0)
- {
- if (GET_MODE_BITSIZE (GET_MODE (op0))> 0)
- mode = GET_MODE (op0);
- else if (target && GET_MODE_BITSIZE (GET_MODE (target))> 0)
- mode = GET_MODE (target);
- else
- mode = tmode;
- }
- else
- mode = get_best_mode (bitsize, bitnum, 0, 0,
- MEM_ALIGN (op0), word_mode, MEM_VOLATILE_P (op0));
+ mode = GET_MODE (op0);
+ if (GET_MODE_BITSIZE (mode) == 0
+ || GET_MODE_BITSIZE (mode)> GET_MODE_BITSIZE (word_mode))
+ mode = word_mode;
+ mode = get_best_mode (bitsize, bitnum, 0, 0,
+ MEM_ALIGN (op0), mode, MEM_VOLATILE_P (op0));
if (mode == VOIDmode)
/* The only way this should occur is if the field spans word
boundaries. */
return extract_split_bit_field (op0, bitsize, bitnum, unsignedp);
So the problem started, because initially this function did not look at
GET_MODE(op0)
and always used word_mode. That was changed, but now also affected non-volatile
data.
Now, if we solve this differently and install the C++ memory model patch,
we can avoid to introduce the recursion in the extract path,
and remove these two hunks in the update patch at the same time:
+ else if (MEM_P (str_rtx)
+ && MEM_VOLATILE_P (str_rtx)
+ && flag_strict_volatile_bitfields> 0)
+ /* This is a case where -fstrict-volatile-bitfields doesn't apply
+ because we can't do a single access in the declared mode of the field.
+ Since the incoming STR_RTX has already been adjusted to that mode,
+ fall back to word mode for subsequent logic. */
+ str_rtx = adjust_address (str_rtx, word_mode, 0);
Attached you'll find a new version of the bitfields-update patch,
it is again relative to the already approved version of the volatile-bitfields
patch v4, part 1/2.
Boot-strapped and regression-tested on X86_64-linux-gnu.
additionally tested with an ARM cross-compiler.
OK for trunk?
Thanks
Bernd. 2013-11-18 Bernd Edlinger <bernd.edlin...@hotmail.de>
Sandra Loosemore <san...@codesourcery.com>
PR middle-end/23623
PR middle-end/48784
PR middle-end/56341
PR middle-end/56997
* expmed.c (strict_volatile_bitfield_p): Add bitregion_start
and bitregion_end parameters. Test for compliance with C++
memory model.
(store_bit_field): Adjust call to strict_volatile_bitfield_p.
Add fallback logic for cases where -fstrict-volatile-bitfields
is supposed to apply, but cannot.
(extract_bit_field): Likewise. Use narrow_bit_field_mem and
extract_fixed_bit_field_1 to do the extraction.
(extract_fixed_bit_field): Revert to previous mode selection algorithm.
Call extract_fixed_bit_field_1 to do the real work.
(extract_fixed_bit_field_1): New function.
testsuite:
2013-11-18 Bernd Edlinger <bernd.edlin...@hotmail.de>
Sandra Loosemore <san...@codesourcery.com>
* gcc.dg/pr23623.c: Update to test interaction with C++
memory model.
patch-bitfields-update-1.diff
Description: Binary data
