From: DJ Delorie <dj@redhat.com>
To: Mark Mitchell <mark@codesourcery.com>
Cc: joseph@codesourcery.com, gcc-patches@gcc.gnu.org
Subject: Re: patch: honor volatile bitfield types
Date: Fri, 11 Jun 2010 23:23:00 -0000 [thread overview]
Message-ID: <201006112204.o5BM4wsk031264@greed.delorie.com> (raw)
In-Reply-To: <4C126123.2060004@codesourcery.com> (message from Mark Mitchell on Fri, 11 Jun 2010 09:15:31 -0700)
Ok, updated documentation, and the default value for the flag is -1.
The checks are all "flag > 0" so it can be left at -1 or changed to
some other default, but the targets can see if it's still unspecified
and change it if they wish.
2010-06-11 DJ Delorie <dj@redhat.com>
* common.opt (-fhonor-volatile-bitfield-types): new.
* doc/invoke.texi: Document it.
* fold-const.c (optimize_bit_field_compare): For volatile
bitfields, use the field's type to determine the mode, not the
field's size.
* expr.c (expand_assignment): Likewise.
(get_inner_reference): Likewise.
(expand_expr_real_1): Likewise.
* expmed.c (store_fixed_bit_field): Likewise.
(extract_bit_field_1): Likewise.
(extract_fixed_bit_field): Likewise.
* gcc.target/i386/volatile-bitfields-1.c: New.
* gcc.target/i386/volatile-bitfields-2.c: New.
Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi (revision 160589)
+++ doc/invoke.texi (working copy)
@@ -17706,12 +17706,38 @@ be thrown between DSOs must be explicitl
visibility so that the @samp{type_info} nodes will be unified between
the DSOs.
An overview of these techniques, their benefits and how to use them
is at @w{@uref{http://gcc.gnu.org/wiki/Visibility}}.
+@item -fhonor-volatile-bitfield-types
+This option should be used if accesses to volatile bitfields (or other
+structure fields, although the compiler usually honors those types
+anyway) should use a single access in a mode of the same size as the
+container's type, aligned to a natural alignment if possible. For
+example, targets with memory-mapped peripheral registers might require
+all such accesses to be 16 bits wide; with this flag the user could
+declare all peripheral bitfields as ``unsigned short'' (assuming short
+is 16 bits on these targets) to force GCC to use 16 bit accesses
+instead of, perhaps, a more efficient 32 bit access.
+
+If this option is disabled, the compiler will use the most efficient
+instruction. In the previous example, that might be a 32-bit load
+instruction, even though that will access bytes that do not contain
+any portion of the bitfield, or memory-mapped registers unrelated to
+the one being updated.
+
+If the target requires strict alignment, and honoring the container
+type would require violating this alignment, a warning is issued.
+However, the access happens as the user requested, under the
+assumption that the user knows something about the target hardware
+that GCC is unaware of.
+
+The default value of this option is determined by the application binary
+interface for the target processor.
+
@end table
@c man end
@node Environment Variables
@section Environment Variables Affecting GCC
Index: fold-const.c
===================================================================
--- fold-const.c (revision 160589)
+++ fold-const.c (working copy)
@@ -3460,17 +3460,22 @@ optimize_bit_field_compare (location_t l
|| TREE_CODE (rinner) == PLACEHOLDER_EXPR)
return 0;
}
/* See if we can find a mode to refer to this field. We should be able to,
but fail if we can't. */
- nmode = get_best_mode (lbitsize, lbitpos,
- const_p ? TYPE_ALIGN (TREE_TYPE (linner))
- : MIN (TYPE_ALIGN (TREE_TYPE (linner)),
- TYPE_ALIGN (TREE_TYPE (rinner))),
- word_mode, lvolatilep || rvolatilep);
+ if (lvolatilep
+ && GET_MODE_BITSIZE (lmode) > 0
+ && flag_honor_volatile_bitfield_types > 0)
+ nmode = lmode;
+ else
+ nmode = get_best_mode (lbitsize, lbitpos,
+ const_p ? TYPE_ALIGN (TREE_TYPE (linner))
+ : MIN (TYPE_ALIGN (TREE_TYPE (linner)),
+ TYPE_ALIGN (TREE_TYPE (rinner))),
+ word_mode, lvolatilep || rvolatilep);
if (nmode == VOIDmode)
return 0;
/* Set signed and unsigned types of the precision of this mode for the
shifts below. */
signed_type = lang_hooks.types.type_for_mode (nmode, 0);
Index: testsuite/gcc.target/i386/volatile-bitfields-2.c
===================================================================
--- testsuite/gcc.target/i386/volatile-bitfields-2.c (revision 0)
+++ testsuite/gcc.target/i386/volatile-bitfields-2.c (revision 0)
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fno-honor-volatile-bitfield-types" } */
+
+typedef struct {
+ char a:1;
+ char b:7;
+ int c;
+} BitStruct;
+
+volatile BitStruct bits;
+
+int foo ()
+{
+ return bits.b;
+}
+
+/* { dg-final { scan-assembler "movl.*bits" } } */
Index: testsuite/gcc.target/i386/volatile-bitfields-1.c
===================================================================
--- testsuite/gcc.target/i386/volatile-bitfields-1.c (revision 0)
+++ testsuite/gcc.target/i386/volatile-bitfields-1.c (revision 0)
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fhonor-volatile-bitfield-types" } */
+
+typedef struct {
+ char a:1;
+ char b:7;
+ int c;
+} BitStruct;
+
+volatile BitStruct bits;
+
+int foo ()
+{
+ return bits.b;
+}
+
+/* { dg-final { scan-assembler "movzbl.*bits" } } */
Index: expr.c
===================================================================
--- expr.c (revision 160589)
+++ expr.c (working copy)
@@ -4226,12 +4226,19 @@ expand_assignment (tree to, tree from, b
/* If we are going to use store_bit_field and extract_bit_field,
make sure to_rtx will be safe for multiple use. */
to_rtx = expand_normal (tem);
+ /* If the bitfield is volatile, we want to access it in the
+ field's mode, not the computed mode. */
+ if (volatilep
+ && GET_CODE (to_rtx) == MEM
+ && flag_honor_volatile_bitfield_types > 0)
+ to_rtx = adjust_address (to_rtx, mode1, 0);
+
if (offset != 0)
{
enum machine_mode address_mode;
rtx offset_rtx;
if (!MEM_P (to_rtx))
@@ -5987,12 +5994,18 @@ get_inner_reference (tree exp, HOST_WIDE
tree field = TREE_OPERAND (exp, 1);
size_tree = DECL_SIZE (field);
if (!DECL_BIT_FIELD (field))
mode = DECL_MODE (field);
else if (DECL_MODE (field) == BLKmode)
blkmode_bitfield = true;
+ else if (TREE_THIS_VOLATILE (exp)
+ && flag_honor_volatile_bitfield_types > 0)
+ /* Volatile bitfields should be accessed in the mode of the
+ field's type, not the mode computed based on the bit
+ size. */
+ mode = TYPE_MODE (DECL_BIT_FIELD_TYPE (field));
*punsignedp = DECL_UNSIGNED (field);
}
else if (TREE_CODE (exp) == BIT_FIELD_REF)
{
size_tree = TREE_OPERAND (exp, 1);
@@ -8963,12 +8976,20 @@ expand_expr_real_1 (tree exp, rtx target
VOIDmode,
(modifier == EXPAND_INITIALIZER
|| modifier == EXPAND_CONST_ADDRESS
|| modifier == EXPAND_STACK_PARM)
? modifier : EXPAND_NORMAL);
+
+ /* If the bitfield is volatile, we want to access it in the
+ field's mode, not the computed mode. */
+ if (volatilep
+ && GET_CODE (op0) == MEM
+ && flag_honor_volatile_bitfield_types > 0)
+ op0 = adjust_address (op0, mode1, 0);
+
mode2
= CONSTANT_P (op0) ? TYPE_MODE (TREE_TYPE (tem)) : GET_MODE (op0);
/* If we have either an offset, a BLKmode result, or a reference
outside the underlying object, we must force it to memory.
Such a case can occur in Ada if we have unchecked conversion
@@ -9088,12 +9109,15 @@ expand_expr_real_1 (tree exp, rtx target
|| REG_P (op0) || GET_CODE (op0) == SUBREG
|| (mode1 != BLKmode && ! direct_load[(int) mode1]
&& GET_MODE_CLASS (mode) != MODE_COMPLEX_INT
&& GET_MODE_CLASS (mode) != MODE_COMPLEX_FLOAT
&& modifier != EXPAND_CONST_ADDRESS
&& modifier != EXPAND_INITIALIZER)
+ /* If the field is volatile, we always want an aligned
+ access. */
+ || (volatilep && flag_honor_volatile_bitfield_types > 0)
/* If the field isn't aligned enough to fetch as a memref,
fetch it as a bit field. */
|| (mode1 != BLKmode
&& (((TYPE_ALIGN (TREE_TYPE (tem)) < GET_MODE_ALIGNMENT (mode)
|| (bitpos % GET_MODE_ALIGNMENT (mode) != 0)
|| (MEM_P (op0)
Index: expmed.c
===================================================================
--- expmed.c (revision 160589)
+++ expmed.c (working copy)
@@ -900,14 +900,20 @@ store_fixed_bit_field (rtx op0, unsigned
We don't want a mode bigger than the destination. */
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, bitpos + offset * BITS_PER_UNIT,
- MEM_ALIGN (op0), mode, MEM_VOLATILE_P (op0));
+
+ if (MEM_VOLATILE_P (op0)
+ && GET_MODE_BITSIZE (GET_MODE (op0)) > 0
+ && flag_honor_volatile_bitfield_types > 0)
+ mode = GET_MODE (op0);
+ else
+ mode = get_best_mode (bitsize, bitpos + offset * BITS_PER_UNIT,
+ MEM_ALIGN (op0), mode, MEM_VOLATILE_P (op0));
if (mode == VOIDmode)
{
/* The only way this should occur is if the field spans word
boundaries. */
store_split_bit_field (op0, bitsize, bitpos + offset * BITS_PER_UNIT,
@@ -1374,12 +1380,20 @@ extract_bit_field_1 (rtx str_rtx, unsign
we want a mode based on the size, so we must avoid calling it for FP
modes. */
mode1 = (SCALAR_INT_MODE_P (tmode)
? mode_for_size (bitsize, GET_MODE_CLASS (tmode), 0)
: mode);
+ /* If the bitfield is volatile, we need to make sure the access
+ remains on a type-aligned boundary. */
+ if (GET_CODE (op0) == MEM
+ && MEM_VOLATILE_P (op0)
+ && GET_MODE_BITSIZE (GET_MODE (op0)) > 0
+ && flag_honor_volatile_bitfield_types > 0)
+ goto no_subreg_mode_swap;
+
if (((bitsize >= BITS_PER_WORD && bitsize == GET_MODE_BITSIZE (mode)
&& bitpos % BITS_PER_WORD == 0)
|| (mode1 != BLKmode
/* ??? The big endian test here is wrong. This is correct
if the value is in a register, and if mode_for_size is not
the same mode as op0. This causes us to get unnecessarily
@@ -1726,14 +1740,25 @@ extract_fixed_bit_field (enum machine_mo
else
{
/* Get the proper mode to use for this field. We want a mode that
includes the entire field. If such a mode would be larger than
a word, we won't be doing the extraction the normal way. */
- mode = get_best_mode (bitsize, bitpos + offset * BITS_PER_UNIT,
- MEM_ALIGN (op0), word_mode, MEM_VOLATILE_P (op0));
+ if (MEM_VOLATILE_P (op0)
+ && flag_honor_volatile_bitfield_types > 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, bitpos + offset * BITS_PER_UNIT,
+ MEM_ALIGN (op0), word_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,
bitpos + offset * BITS_PER_UNIT,
@@ -1748,18 +1773,41 @@ extract_fixed_bit_field (enum machine_mo
{
offset += (bitpos / total_bits) * (total_bits / BITS_PER_UNIT);
bitpos -= ((bitpos / total_bits) * (total_bits / BITS_PER_UNIT)
* BITS_PER_UNIT);
}
- /* Get ref to an aligned byte, halfword, or word containing the field.
- Adjust BITPOS to be position within a word,
- and OFFSET to be the offset of that word.
- Then alter OP0 to refer to that word. */
- bitpos += (offset % (total_bits / BITS_PER_UNIT)) * BITS_PER_UNIT;
- offset -= (offset % (total_bits / BITS_PER_UNIT));
+ /* If we're accessing a volatile MEM, we can't do the next
+ alignment step if it results in a multi-word access where we
+ otherwise wouldn't have one. So, check for that case
+ here. */
+ if (MEM_P (op0)
+ && MEM_VOLATILE_P (op0)
+ && flag_honor_volatile_bitfield_types > 0
+ && bitpos + bitsize <= total_bits
+ && bitpos + bitsize + (offset % (total_bits / BITS_PER_UNIT)) * BITS_PER_UNIT > total_bits)
+ {
+ if (STRICT_ALIGNMENT)
+ {
+ if (bitsize == total_bits)
+ warning (0, "mis-aligned access required for structure member");
+ else
+ warning (0, "mis-aligned access required for structure bitfield");
+ }
+ }
+ else
+ {
+
+ /* Get ref to an aligned byte, halfword, or word containing the field.
+ Adjust BITPOS to be position within a word,
+ and OFFSET to be the offset of that word.
+ Then alter OP0 to refer to that word. */
+ bitpos += (offset % (total_bits / BITS_PER_UNIT)) * BITS_PER_UNIT;
+ offset -= (offset % (total_bits / BITS_PER_UNIT));
+ }
+
op0 = adjust_address (op0, mode, offset);
}
mode = GET_MODE (op0);
if (BYTES_BIG_ENDIAN)
Index: common.opt
===================================================================
--- common.opt (revision 160589)
+++ common.opt (working copy)
@@ -622,12 +622,16 @@ Common Report Var(flag_loop_interchange)
Enable Loop Interchange transformation
floop-block
Common Report Var(flag_loop_block) Optimization
Enable Loop Blocking transformation
+fhonor-volatile-bitfield-types
+Common Report Var(flag_honor_volatile_bitfield_types) Init(-1)
+Force bitfield accesses to match their type width
+
fguess-branch-probability
Common Report Var(flag_guess_branch_prob) Optimization
Enable guessing of branch probabilities
; Nonzero means ignore `#ident' directives. 0 means handle them.
; Generate position-independent code for executables if possible
next prev parent reply other threads:[~2010-06-11 22:05 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-06-11 4:42 DJ Delorie
2010-06-11 9:55 ` Joseph S. Myers
2010-06-11 14:42 ` DJ Delorie
2010-06-11 16:42 ` Mark Mitchell
2010-06-11 16:56 ` Jeff Law
2010-06-11 23:23 ` DJ Delorie [this message]
2010-06-12 2:28 ` Mark Mitchell
2010-06-12 3:45 ` DJ Delorie
2010-06-12 4:12 ` Mark Mitchell
2010-06-12 10:17 ` Richard Guenther
2010-06-15 2:40 ` DJ Delorie
2010-06-15 3:01 ` Mark Mitchell
2010-06-15 5:16 ` DJ Delorie
2010-06-15 6:00 ` Mark Mitchell
2010-06-15 22:29 ` DJ Delorie
2010-06-15 22:45 ` Manuel López-Ibáñez
2010-06-15 22:55 ` DJ Delorie
2010-06-15 22:55 ` DJ Delorie
2010-06-15 23:02 ` Mark Mitchell
2010-06-17 7:58 ` DJ Delorie
2010-06-22 10:42 ` Eric Botcazou
2010-06-22 18:32 ` DJ Delorie
2010-06-22 18:39 ` Eric Botcazou
2010-06-22 18:57 ` DJ Delorie
2010-06-22 20:04 ` Eric Botcazou
2010-06-23 0:51 ` DJ Delorie
2010-09-03 18:41 ` Jie Zhang
2010-06-15 23:03 ` Manuel López-Ibáñez
2010-06-16 0:43 ` DJ Delorie
2010-06-16 4:23 ` Mark Mitchell
2010-06-15 8:32 ` Manuel López-Ibáñez
2010-06-15 17:53 ` Richard Henderson
2010-06-15 17:26 ` DJ Delorie
2010-06-15 17:35 ` Manuel López-Ibáñez
2010-06-15 19:20 ` Richard Henderson
2010-06-15 19:27 ` Manuel López-Ibáñez
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=201006112204.o5BM4wsk031264@greed.delorie.com \
--to=dj@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=joseph@codesourcery.com \
--cc=mark@codesourcery.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).