Re: Remove word_mode hack for split bitfields

2016-06-06 Thread Bernd Schmidt

On 05/26/2016 04:36 PM, Richard Sandiford wrote:

This patch is effectively reverting a change from 1994.  The reason
I think it's a hack is that store_bit_field_1 is creating a subreg
reference to one word of a field even though it has already proven that
the field spills into the following word.  We then rely on the special
SUBREG handling in store_split_bit_field to ignore the extent of op0 and
look inside the SUBREG_REG regardless.  I don't see any reason why we can't
pass the original op0 to store_split_bit_field instead.

Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?


I think it's OK. Ideally we'd know why the 1994 change was made, but 
that's beyond my archaeological abiliy. The code looked very different 
at the time and probably changed over the years to make this 
simplification possible.



Bernd



Re: Remove word_mode hack for split bitfields

2016-06-01 Thread Richard Sandiford
Bernd Schmidt  writes:
> On 05/26/2016 04:36 PM, Richard Sandiford wrote:
>> This patch is effectively reverting a change from 1994.  The reason
>> I think it's a hack is that store_bit_field_1 is creating a subreg
>> reference to one word of a field even though it has already proven that
>> the field spills into the following word.  We then rely on the special
>> SUBREG handling in store_split_bit_field to ignore the extent of op0 and
>> look inside the SUBREG_REG regardless.  I don't see any reason why we can't
>> pass the original op0 to store_split_bit_field instead.
>>
>> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
>
> Any observable effects on code generation?

Not that I can see.  I tried compiling gcc.dg, g++.dg and gcc.c-torture
before and after the patch on:

aarch64-linux-gnueabi alpha-linux-gnu arm-linux-gnueabi
arm-linux-gnueabihf avr-elf bfin-elf c6x-elf cr16-elf cris-elf
epiphany-elf fr30-elf frv-linux-gnu ft32-elf h8300-elf
ia64-linux-gnu iq2000-elf lm32-elf m32c-elf m32r-elf m68k-linux-gnu
mcore-elf microblaze-elf mips-linux-gnu mmix mn10300-elf moxie-rtems
msp430-elf nds32le-elf hppa64-hp-hpux11.23 nios2-linux-gnu
nvptx-none powerpc-linux-gnu powerpc-eabispe rl78-elf rx-elf
s390-linux-gnu sh-linux-gnu sparc-linux-gnu spu-elf tilegx-elf
tilepro-elf xstormy16-elf v850-elf vax-netbsdelf visium-elf
xtensa-elf x86_64-darwin

The only asm differences (apart from timestamps) were a handful of tests
for avr-elf:

gcc.c-torture/execute/pr53645.s
gcc.c-torture/execute/pr65427.s
gcc.dg/pr57233.s
gcc.dg/torture/pr30665-2.s
gcc.dg/tree-ssa/pr23391.s

and c6x-elf:

gcc.c-torture/compile/pr42956.s
gcc.dg/compat/vector-1a_y.s
gcc.dg/compat/vector-1b_y.s
gcc.dg/torture/vshuf-v8sf.s

In all these cases the rtl seems to be the same before reload,
but differences in addresses(?) cause different, but equivalent, RA.
None of the LRA targets seem to be affected.

Thanks,
Richard


Re: Remove word_mode hack for split bitfields

2016-05-31 Thread Bernd Schmidt

On 05/26/2016 04:36 PM, Richard Sandiford wrote:

This patch is effectively reverting a change from 1994.  The reason
I think it's a hack is that store_bit_field_1 is creating a subreg
reference to one word of a field even though it has already proven that
the field spills into the following word.  We then rely on the special
SUBREG handling in store_split_bit_field to ignore the extent of op0 and
look inside the SUBREG_REG regardless.  I don't see any reason why we can't
pass the original op0 to store_split_bit_field instead.

Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?


Any observable effects on code generation?


Bernd



Remove word_mode hack for split bitfields

2016-05-26 Thread Richard Sandiford
This patch is effectively reverting a change from 1994.  The reason
I think it's a hack is that store_bit_field_1 is creating a subreg
reference to one word of a field even though it has already proven that
the field spills into the following word.  We then rely on the special
SUBREG handling in store_split_bit_field to ignore the extent of op0 and
look inside the SUBREG_REG regardless.  I don't see any reason why we can't
pass the original op0 to store_split_bit_field instead.

Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?

Thanks,
Richard


gcc/
* expmed.c (store_bit_field_1): Do not restrict a multiword op0
to one word if the field is known to overlap other words.
(extract_bit_field_1): Likewise.
(store_split_bit_field): Remove compensating code.
(extract_split_bit_field): Likewise.

diff --git a/gcc/expmed.c b/gcc/expmed.c
index ec968da..6645a53 100644
--- a/gcc/expmed.c
+++ b/gcc/expmed.c
@@ -967,11 +967,7 @@ store_bit_field_1 (rtx str_rtx, unsigned HOST_WIDE_INT 
bitsize,
  If the region spans two words, defer to store_split_bit_field.  */
   if (!MEM_P (op0) && GET_MODE_SIZE (GET_MODE (op0)) > UNITS_PER_WORD)
 {
-  op0 = simplify_gen_subreg (word_mode, op0, GET_MODE (op0),
-bitnum / BITS_PER_WORD * UNITS_PER_WORD);
-  gcc_assert (op0);
-  bitnum %= BITS_PER_WORD;
-  if (bitnum + bitsize > BITS_PER_WORD)
+  if (bitnum % BITS_PER_WORD + bitsize > BITS_PER_WORD)
{
  if (!fallback_p)
return false;
@@ -980,6 +976,10 @@ store_bit_field_1 (rtx str_rtx, unsigned HOST_WIDE_INT 
bitsize,
 bitregion_end, value, reverse);
  return true;
}
+  op0 = simplify_gen_subreg (word_mode, op0, GET_MODE (op0),
+bitnum / BITS_PER_WORD * UNITS_PER_WORD);
+  gcc_assert (op0);
+  bitnum %= BITS_PER_WORD;
 }
 
   /* From here on we can assume that the field to be stored in fits
@@ -1383,25 +1383,8 @@ store_split_bit_field (rtx op0, unsigned HOST_WIDE_INT 
bitsize,
bitsdone, NULL_RTX, 1, false);
}
 
-  /* If OP0 is a register, then handle OFFSET here.
-
-When handling multiword bitfields, extract_bit_field may pass
-down a word_mode SUBREG of a larger REG for a bitfield that actually
-crosses a word boundary.  Thus, for a SUBREG, we must find
-the current word starting from the base register.  */
-  if (GET_CODE (op0) == SUBREG)
-   {
- int word_offset = (SUBREG_BYTE (op0) / UNITS_PER_WORD)
-   + (offset * unit / BITS_PER_WORD);
- machine_mode sub_mode = GET_MODE (SUBREG_REG (op0));
- if (sub_mode != BLKmode && GET_MODE_SIZE (sub_mode) < UNITS_PER_WORD)
-   word = word_offset ? const0_rtx : op0;
- else
-   word = operand_subword_force (SUBREG_REG (op0), word_offset,
- GET_MODE (SUBREG_REG (op0)));
- offset &= BITS_PER_WORD / unit - 1;
-   }
-  else if (REG_P (op0))
+  /* If OP0 is a register, then handle OFFSET here.  */
+  if (SUBREG_P (op0) || REG_P (op0))
{
  machine_mode op0_mode = GET_MODE (op0);
  if (op0_mode != BLKmode && GET_MODE_SIZE (op0_mode) < UNITS_PER_WORD)
@@ -1787,10 +1770,7 @@ extract_bit_field_1 (rtx str_rtx, unsigned HOST_WIDE_INT 
bitsize,
  If the region spans two words, defer to extract_split_bit_field.  */
   if (!MEM_P (op0) && GET_MODE_SIZE (GET_MODE (op0)) > UNITS_PER_WORD)
 {
-  op0 = simplify_gen_subreg (word_mode, op0, GET_MODE (op0),
-bitnum / BITS_PER_WORD * UNITS_PER_WORD);
-  bitnum %= BITS_PER_WORD;
-  if (bitnum + bitsize > BITS_PER_WORD)
+  if (bitnum % BITS_PER_WORD + bitsize > BITS_PER_WORD)
{
  if (!fallback_p)
return NULL_RTX;
@@ -1798,6 +1778,9 @@ extract_bit_field_1 (rtx str_rtx, unsigned HOST_WIDE_INT 
bitsize,
reverse);
  return convert_extracted_bit_field (target, mode, tmode, unsignedp);
}
+  op0 = simplify_gen_subreg (word_mode, op0, GET_MODE (op0),
+bitnum / BITS_PER_WORD * UNITS_PER_WORD);
+  bitnum %= BITS_PER_WORD;
 }
 
   /* From here on we know the desired field is smaller than a word.
@@ -2109,20 +2092,8 @@ extract_split_bit_field (rtx op0, unsigned HOST_WIDE_INT 
bitsize,
   thissize = MIN (bitsize - bitsdone, BITS_PER_WORD);
   thissize = MIN (thissize, unit - thispos);
 
-  /* If OP0 is a register, then handle OFFSET here.
-
-When handling multiword bitfields, extract_bit_field may pass
-down a word_mode SUBREG of a larger REG for a bitfield that actually
-crosses a word boundary.  Thus, for a SUBREG, we must find
-the current word starting from the bas