Re: Tidy extract_bit_field_1 co.
Here's a version with the corresponding fixes from Eric's review of the store_bit_field_1 patch. Tested as before. gcc/ * expmed.c (store_split_bit_field): Update the calls to extract_fixed_bit_field. In the big-endian case, always use the mode of OP0 to count the number of significant bits. (extract_bit_field_1): Remove unit, offset, bitpos and byte_offset from the outermost scope. Express conditions in terms of bitnum rather than offset, bitpos and byte_offset. Move the computation of MODE1 to the block that needs it. Use MODE unless the TMODE-based mode_for_size calculation succeeds. Split the plain move cases into two, one for memory accesses and one for register accesses. Generalize the memory case, freeing it from the old register-based endian checks. Move the INT_MODE calculation above the code that needs it. Use simplify_gen_subreg to handle multiword OP0s. If the field still spans several words, pass it directly to extract_split_bit_field. Assume after that point that both targets and register sources fit within a word. Replace x-prefixed variables with non-prefixed forms. Compute the bitpos for ext(z)v register operands directly in the chosen unit size, rather than going through an intermediate BITS_PER_WORD unit size. Simplify the containment check used when forcing OP0 into a register. Update the call to extract_fixed_bit_field. (extract_fixed_bit_field): Replace the bitpos and offset parameters with a single bitnum parameter, of the same form as extract_bit_field. Assume that OP0 contains the full field. Simplify the memory offset calculation and containment check for volatile bitfields. Make the offset explicit when volatile bitfields force a misaligned access. Remove WARNED and fix long lines. Assert that the processed OP0 has an integral mode. (store_split_bit_field): Update the call to store_fixed_bit_field. This looks good to me (modulo the counterpart of the corresponding change in the first patch for which I cannot really tell whether it's right or wrong). -- Eric Botcazou
Re: Tidy extract_bit_field_1 co.
Richard Sandiford rdsandif...@googlemail.com writes:
Partnering the store_bit_field_1 patch that I just posted, this patch
tidies up the extract_bit_field code in the same way.
There is one deliberate behavioural change here. The old code had a
single check for cases where the extraction could be done as a simple
move. It started:
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
inefficient code from the Thumb port when -mbig-endian. */
(BYTES_BIG_ENDIAN
? bitpos + bitsize == BITS_PER_WORD
: bitpos == 0)))
The BYTES_BIG_ENDIAN check didn't make sense for memory operands though,
because bitpos was based on byte units in that case. That might well be
what the comment was complaining about; I'm not sure.
Also, I made the MODE1 computation take failures of mode_for_size
into account.
Tested on x86_64-linux-gnu, powerpc64-linux-gnu, mipsisa64-elf (both -EL
and -EB) and mipsisa32-elf (also both -EL and -EB). OK to install?
Here's a version with the corresponding fixes from Eric's review
of the store_bit_field_1 patch. Tested as before.
gcc/
* expmed.c (store_split_bit_field): Update the calls to
extract_fixed_bit_field. In the big-endian case, always
use the mode of OP0 to count the number of significant bits.
(extract_bit_field_1): Remove unit, offset, bitpos and
byte_offset from the outermost scope. Express conditions in terms
of bitnum rather than offset, bitpos and byte_offset. Move the
computation of MODE1 to the block that needs it. Use MODE unless
the TMODE-based mode_for_size calculation succeeds. Split the
plain move cases into two, one for memory accesses and one for
register accesses. Generalize the memory case, freeing it from
the old register-based endian checks. Move the INT_MODE calculation
above the code that needs it. Use simplify_gen_subreg to handle
multiword OP0s. If the field still spans several words, pass it
directly to extract_split_bit_field. Assume after that point
that both targets and register sources fit within a word.
Replace x-prefixed variables with non-prefixed forms.
Compute the bitpos for ext(z)v register operands directly in the
chosen unit size, rather than going through an intermediate
BITS_PER_WORD unit size. Simplify the containment check
used when forcing OP0 into a register. Update the call to
extract_fixed_bit_field.
(extract_fixed_bit_field): Replace the bitpos and offset parameters
with a single bitnum parameter, of the same form as extract_bit_field.
Assume that OP0 contains the full field. Simplify the memory offset
calculation and containment check for volatile bitfields. Make the
offset explicit when volatile bitfields force a misaligned access.
Remove WARNED and fix long lines. Assert that the processed OP0
has an integral mode.
(store_split_bit_field): Update the call to store_fixed_bit_field.
Index: gcc/expmed.c
===
--- gcc/expmed.c2012-10-18 19:10:29.268718181 +0100
+++ gcc/expmed.c2012-10-18 19:13:24.134708442 +0100
@@ -57,7 +57,6 @@ static void store_split_bit_field (rtx,
rtx);
static rtx extract_fixed_bit_field (enum machine_mode, rtx,
unsigned HOST_WIDE_INT,
- unsigned HOST_WIDE_INT,
unsigned HOST_WIDE_INT, rtx, int, bool);
static rtx mask_rtx (enum machine_mode, int, int, int);
static rtx lshift_value (enum machine_mode, rtx, int, int);
@@ -1129,28 +1128,21 @@ store_split_bit_field (rtx op0, unsigned
if (BYTES_BIG_ENDIAN)
{
- int total_bits;
-
- /* We must do an endian conversion exactly the same way as it is
-done in extract_bit_field, so that the two calls to
-extract_fixed_bit_field will have comparable arguments. */
- if (!MEM_P (value) || GET_MODE (value) == BLKmode)
- total_bits = BITS_PER_WORD;
- else
- total_bits = GET_MODE_BITSIZE (GET_MODE (value));
-
/* Fetch successively less significant portions. */
if (CONST_INT_P (value))
part = GEN_INT (((unsigned HOST_WIDE_INT) (INTVAL (value))
(bitsize - bitsdone - thissize))
(((HOST_WIDE_INT) 1 thissize) - 1));
else
- /* The args are chosen so that the last part
Tidy extract_bit_field_1 co.
Partnering the store_bit_field_1 patch that I just posted, this patch
tidies up the extract_bit_field code in the same way.
There is one deliberate behavioural change here. The old code had a
single check for cases where the extraction could be done as a simple
move. It started:
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
inefficient code from the Thumb port when -mbig-endian. */
(BYTES_BIG_ENDIAN
? bitpos + bitsize == BITS_PER_WORD
: bitpos == 0)))
The BYTES_BIG_ENDIAN check didn't make sense for memory operands though,
because bitpos was based on byte units in that case. That might well be
what the comment was complaining about; I'm not sure.
Also, I made the MODE1 computation take failures of mode_for_size
into account.
Tested on x86_64-linux-gnu, powerpc64-linux-gnu, mipsisa64-elf (both -EL
and -EB) and mipsisa32-elf (also both -EL and -EB). OK to install?
Richard
gcc/
* expmed.c (store_split_bit_field): Update the calls to
extract_fixed_bit_field. In the big-endian case, always
use the mode of OP0 to count the number of significant bits.
(extract_bit_field_1): Remove unit, offset, bitpos and
byte_offset from the outermost scope. Express conditions in terms
of bitnum rather than offset, bitpos and byte_offset. Move the
computation of MODE1 to the block that needs it. Use MODE unless
the TMODE-based mode_for_size calculation succeeds. Split the
plain move cases into two, one for memory accesses and one for
register accesses. Generalize the memory case, freeing it from
the old register-based endian checks. Move the INT_MODE calculation
above the code that needs it. Use simplify_gen_subreg to handle
multiword OP0s. If the field still spans several words, pass it
directly to extract_split_bit_field. Assume after that point
that both targets and register sources fit within a word.
Replace x-prefixed variables with non-prefixed forms.
Compute the bitpos for ext(z)v register operands directly in the
chosen unit size, rather than going through an intermediate
BITS_PER_WORD unit size. Simplify the containment check
used when forcing OP0 into a register. Update the call to
extract_fixed_bit_field.
(extract_fixed_bit_field): Replace the bitpos and offset parameters
with a single bitnum parameter, of the same form as extract_bit_field.
Assume that OP0 contains the full field. Simplify the memory offset
calculation and containment check for volatile bitfields. Make the
offset explicit when volatile bitfields force a misaligned access.
Remove WARNED and fix long lines. Assert that the processed OP0
has an integral mode.
(store_split_bit_field): Update the call to store_fixed_bit_field.
Index: gcc/expmed.c
===
--- gcc/expmed.c2012-10-14 11:44:27.359686486 +0100
+++ gcc/expmed.c2012-10-14 11:44:41.770685683 +0100
@@ -57,7 +57,6 @@ static void store_split_bit_field (rtx,
rtx);
static rtx extract_fixed_bit_field (enum machine_mode, rtx,
unsigned HOST_WIDE_INT,
- unsigned HOST_WIDE_INT,
unsigned HOST_WIDE_INT, rtx, int, bool);
static rtx mask_rtx (enum machine_mode, int, int, int);
static rtx lshift_value (enum machine_mode, rtx, int, int);
@@ -1114,28 +1113,21 @@ store_split_bit_field (rtx op0, unsigned
if (BYTES_BIG_ENDIAN)
{
- int total_bits;
-
- /* We must do an endian conversion exactly the same way as it is
-done in extract_bit_field, so that the two calls to
-extract_fixed_bit_field will have comparable arguments. */
- if (!MEM_P (value) || GET_MODE (value) == BLKmode)
- total_bits = BITS_PER_WORD;
- else
- total_bits = GET_MODE_BITSIZE (GET_MODE (value));
-
/* Fetch successively less significant portions. */
if (CONST_INT_P (value))
part = GEN_INT (((unsigned HOST_WIDE_INT) (INTVAL (value))
(bitsize - bitsdone - thissize))
(((HOST_WIDE_INT) 1 thissize) - 1));
else
- /* The args are chosen so that the last part includes the
- lsb. Give extract_bit_field the value it needs (with
- endianness compensation) to fetch the piece we want. */
- part =
