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 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 las
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. */
-
