Make more use of register insv and ext*v expanders

Make more use of register insv and ext*v expanders

[Richard Sandiford]

...on targets that also have limited memory expanders.

The insv, extv and extzv patterns can ask for the structure operand
to be a register, a memory, or either.  If the predicate rejects a
memory operand, store_bit_field and extract_bit_field try using a
register operand instead.

Unfortunately, on ports like MIPS, the structure operand isn't enough
on its own to determine whether a given memory operation is supported.
The expanders check for various combinations of operands and FAIL on
invalid ones.

If an expander FAILs on invalid memory operations, store_bit_field
and extract_bit_field just carry on as though no expander existed.
On ports that have both register and memory operations, it would be
better to try a register operation instead, just as if the memory
operand had been rejected outright.

This patch therefore refactors store_bit_field so that it falls back
on a register insv if a memory insv can't be used (either because the
predicate rejects the operand, or because the memory insv FAILed).
The change wasn't entirely straight-forward:

  - At the moment, the memory-to-register conversion code calls
    store_bit_field recursively and keeps whatever insns store_bit_field
    produces.  Similarly to the above, I think this only makes sense if
    the register insv succeeds; if the insv FAILs, we should continue as
    though no insv pattern existed.

    I therefore split out the innards of store_bit_field into a
    subroutine, store_bit_field_1, and passed it an argument to say
    whether the store_fixed_bit_field fallback should be used.  It
    should be used when called directly from store_bit_field, but not
    when called recursively from store_bit_field_1.

    (It might have been conceptually cleaner to get store_bit_field to
    call store_fixed_bit_field itself, but the argument calculations are
    mixed up with various short-cuts.  I don't think that any simple way
    around that -- such as passing back the arguments to the caller -- is
    better than the fallback_p argument.)

  - There was no documentation of what store_bit_field (and hence
    store_bit_field_1) was supposed to return.  Most cases returned
    the original source value, but the vector register case returned
    the destination value (i.e. the structure).  The vector case
    might well be a pasto from the extraction code.

    Fortunately, nothing actually uses the return value of
    store_bit_field, so I just removed the return value entirely.

  - store_bit_field had some volatile_ok hokiness:

      int save_volatile_ok = volatile_ok;

      volatile_ok = 1;

      /* If this machine's insv can only insert into a register, copy OP0
	 into a register and save it back later.  */
      if (MEM_P (op0)
	  && ! ((*insn_data[(int) CODE_FOR_insv].operand[0].predicate)
		(op0, VOIDmode)))
	{
          ...
	  store_bit_field (tempreg, bitsize, bitpos, fieldmode, orig_value);
	  emit_move_insn (op0, tempreg);
	  return value;
	}
      volatile_ok = save_volatile_ok;

    where the "..." makes no change to volatile_ok.  Thus the register
    fallback would change the prevailing volatile_ok to 1.  I doubt this
    was the intention, although I'm not sure it would cause any breakage
    in practice.

    I added a separate function for checking predicates with
    volatile_ok set to 1.

The extraction changes are similar, although the second of the problems
listed above doesn't apply there.  Instead, we have the problem that the
extv and extzv expanders are handled by two separate, but very similar,
blocks of code.  I imagine they started out as cut-&-paste copies of
each other, with the obvious signed/unsigned differences, but some
changes have since been made to one and not the other.  Some of these
differences are cosmetic, but one in particular isn't: we allow volatile
memories as the source of an extzv, but not as the source of an extv.
I don't see any reason for this distinction, and no comment implies
that it is deliberate.  (The two blocks of code date back to before
the RCS repository, as does the volatile difference, so I'm only
speculating about the order of events.)

The easiest way of refactoring the memory handling seemed to be to
combine these two blocks of code and parameterise the result based on
signedness.

I ran CSiBE with -mips32r2 -Os and got:

linux:drivers/block/ll_rw_blk                     7228     7164 :   99.11%
linux:drivers/char/pty                            2388     2372 :   99.33%
linux:drivers/char/n_tty                          6576     8528 :  129.68%
linux:drivers/char/tty_ioctl                      2496     2480 :   99.36%
linux:fs/iobuf                                    1400     1384 :   98.86%
linux:fs/exec                                     5744     5712 :   99.44%
linux:fs/lockd/clntproc                           3740     3692 :   98.72%
linux:fs/lockd/host                               2024     2008 :   99.21%
linux:fs/lockd/mon                                1340     1324 :   98.81%
linux:fs/lockd/clntlock                           1264     1232 :   97.47%
linux:fs/lockd/svclock                            3288     3272 :   99.51%
linux:fs/nfs/inode                                8580     8532 :   99.44%
linux:fs/nfsd/nfscache                            2124     2108 :   99.25%
linux:net/ipv4/tcp_diag                           4348     4300 :   98.90%
linux:net/ipv4/ip_options                         4740     4596 :   96.96%
linux:net/ipv4/tcp_minisocks                      5308     5324 :  100.30%
linux:net/ipv4/tcp_ipv4                          17188    17076 :   99.35%
linux:net/ipv4/ip_output                          5912     5784 :   97.83%
linux:net/ipv4/igmp                              14216    14152 :   99.55%
linux:net/ipv4/tcp_output                        11120    11104 :   99.86%
linux:net/ipv4/tcp                               20404    20356 :   99.76%
linux:net/sunrpc/pmap_clnt                        1036     1020 :   98.46%
linux:net/sunrpc/sched                            6144     6160 :  100.26%
linux:net/sunrpc/svcsock                          7220     7188 :   99.56%
linux:net/sunrpc/xprt                             8452     8420 :   99.62%
linux:net/sunrpc/clnt                             5824     5792 :   99.45%
linux:mm/memory                                   8828     8796 :   99.64%
linux:arch/testplatform/kernel/debug              6524     6508 :   99.75%
linux:arch/testplatform/kernel/ptrace             2024     2008 :   99.21%
linux:arch/testplatform/kernel/setup              3064     3048 :   99.48%
linux:kernel/fork                                 4768     4736 :   99.33%
linux:kernel/sys                                  6744     6632 :   98.34%
OpenTCP:ethernet                                  1416     1544 :  109.04%
----------------------------------------------------------------
Total                                          3582377  3583257 :  100.02%

The rather frightening n_tty increase turned out to be a red herring.
Asms in the CSiBE version of linux are commented out, so the headers
have things like:

/*
 * fast, non-SMP test_and_set_bit routine
 */
static __inline__ int
test_and_set_bit_simple(unsigned long nr, volatile void * addr)
{
	unsigned long reg1, reg2;
        int oldbit;
        /* ASM */
        return oldbit & 1;
}

There are therefore lots of tests based on an uninitialized oldbit & 1.
The loop hoisting code considers these expressions to be loop
invariants, and combine considers "if (oldbit & 1)" to be always
untaken.  Both assumptions are prefectly reasonable, but it so happens
that the loop bodies created by expand are simpler after the patch than
they were before.  This encourages the loop-hoisting code to hoist
"oldbit & 1" conditions that it didn't do previously, and so those
conditions are not optimised away by combine.

I tried the comparison again with -mips32r2 -Os -fno-move-loop-invariants
and got the more respectable:

linux:drivers/block/ll_rw_blk                     7116     7052 :   99.10%
linux:drivers/char/pty                            2376     2360 :   99.33%
linux:drivers/char/n_tty                          6336     6256 :   98.74%
linux:drivers/char/tty_ioctl                      2496     2464 :   98.72%
linux:drivers/char/tty_io                        13224    13208 :   99.88%
linux:fs/exec                                     5696     5664 :   99.44%
linux:fs/lockd/svcshare                            632      616 :   97.47%
linux:fs/lockd/clntproc                           3708     3660 :   98.71%
linux:fs/lockd/host                               1992     1976 :   99.20%
linux:fs/lockd/mon                                1340     1324 :   98.81%
linux:fs/lockd/clntlock                           1232     1200 :   97.40%
linux:fs/nfs/inode                                8548     8500 :   99.44%
linux:fs/nfsd/nfscache                            2108     2092 :   99.24%
linux:net/ipv4/ip_sockglue                        6808     6792 :   99.76%
linux:net/ipv4/tcp_diag                           4316     4268 :   98.89%
linux:net/ipv4/ip_options                         4692     4580 :   97.61%
linux:net/ipv4/tcp_minisocks                      5292     5308 :  100.30%
linux:net/ipv4/tcp_ipv4                          17140    17044 :   99.44%
linux:net/ipv4/ip_output                          5896     5768 :   97.83%
linux:net/ipv4/igmp                              14056    14008 :   99.66%
linux:net/ipv4/tcp_output                        11088    11072 :   99.86%
linux:net/ipv4/tcp                               20292    20244 :   99.76%
linux:net/sunrpc/pmap_clnt                        1036     1020 :   98.46%
linux:net/sunrpc/sched                            5952     5968 :  100.27%
linux:net/sunrpc/svcsock                          7204     7172 :   99.56%
linux:net/sunrpc/xprt                             8404     8388 :   99.81%
linux:net/sunrpc/clnt                             5808     5792 :   99.72%
linux:mm/memory                                   8524     8508 :   99.81%
linux:arch/testplatform/kernel/debug              6476     6460 :   99.75%
linux:arch/testplatform/kernel/ptrace             2024     2008 :   99.21%
linux:arch/testplatform/kernel/setup              3044     3028 :   99.47%
linux:kernel/sched                                5916     5900 :   99.73%
linux:kernel/fork                                 4752     4720 :   99.33%
linux:kernel/sys                                  6744     6616 :   98.10%
OpenTCP:ethernet                                  1416     1544 :  109.04%
----------------------------------------------------------------
Total                                          3551921  3550817 :   99.97%

The ethernet difference occurs in a function where we have several
single-bit set insertions.  These insertions are applied to bytes, so
the ins-free version of the insertion just uses "ori".  The ins version
must instead force the 1 into a register, and this instruction then gets
hoisted.  Some of the insertions are applied to constant structure
values, and while combine can optimize the ori sequence into a constant,
it cannot do the same with the ins-of-a-hoisted-register sequence.  This
is IMO a separate problem: all-bits-set insertions should use ori rather
than ins for MIPS where possible.

Bootstrapped & regression-tested on x86_64-linux-gnu.  Also regression-tested
on mipsisa32r2-elfoabi and mipsisa64-elf.  OK to install?

Thanks to Nigel Stephens for pointing out the problem.

Richard


gcc/
	* expr.h (store_bit_field): Don't return a value.
	* expmed.c (check_predicate_volatile_ok): New function.
	(store_bit_field_1): New function, extracted from store_bit_field.
	Take a fallback_p argument and return true if the operation succeeded.
	Only use store_fixed_bit_field if fallback_p.  Don't recompute
	mode_for_extraction; use op_mode instead.  Try forcing memories
	into registers if the insv expander fails.
	(store_bit_field): Use store_bit_field_1 with fallback_p true.
	Don't return a value.
	(convert_extracted_bit_field): New function, extracted from
	store_bit_field.
	(extract_bit_field_1): Likewise.  Take a fallback_p argument
	and return NULL if the operation succeeded.  Only use
	extract_fixed_bit_field if fallback_p.  Only calculate one
	extraction mode.  Combine code for extv and extzv.  Try forcing
	memories into registers if the ext(z)v expander fails.
	(extract_bit_field): Use extract_bit_field_1 with fallback_p true.

gcc/testsuite/
	* gcc.target/mips/ins-1.c: New test.

Index: gcc/expr.h
===================================================================
*** gcc/expr.h	2007-07-18 04:16:39.000000000 +0100
--- gcc/expr.h	2007-07-18 10:58:33.000000000 +0100
*************** enum extraction_pattern { EP_insv, EP_ex
*** 732,739 ****
  extern enum machine_mode
  mode_for_extraction (enum extraction_pattern, int);
  
! extern rtx store_bit_field (rtx, unsigned HOST_WIDE_INT,
! 			    unsigned HOST_WIDE_INT, enum machine_mode, rtx);
  extern rtx extract_bit_field (rtx, unsigned HOST_WIDE_INT,
  			      unsigned HOST_WIDE_INT, int, rtx,
  			      enum machine_mode, enum machine_mode);
--- 732,739 ----
  extern enum machine_mode
  mode_for_extraction (enum extraction_pattern, int);
  
! extern void store_bit_field (rtx, unsigned HOST_WIDE_INT,
! 			     unsigned HOST_WIDE_INT, enum machine_mode, rtx);
  extern rtx extract_bit_field (rtx, unsigned HOST_WIDE_INT,
  			      unsigned HOST_WIDE_INT, int, rtx,
  			      enum machine_mode, enum machine_mode);
Index: gcc/expmed.c
===================================================================
*** gcc/expmed.c	2007-07-18 04:16:39.000000000 +0100
--- gcc/expmed.c	2007-07-18 10:58:33.000000000 +0100
*************** mode_for_extraction (enum extraction_pat
*** 328,353 ****
    return data->operand[opno].mode;
  }
  
  \f
! /* Generate code to store value from rtx VALUE
!    into a bit-field within structure STR_RTX
!    containing BITSIZE bits starting at bit BITNUM.
!    FIELDMODE is the machine-mode of the FIELD_DECL node for this field.
!    ALIGN is the alignment that STR_RTX is known to have.
!    TOTAL_SIZE is the size of the structure in bytes, or -1 if varying.  */
! 
! /* ??? Note that there are two different ideas here for how
!    to determine the size to count bits within, for a register.
!    One is BITS_PER_WORD, and the other is the size of operand 3
!    of the insv pattern.
  
!    If operand 3 of the insv pattern is VOIDmode, then we will use BITS_PER_WORD
!    else, we use the mode of operand 3.  */
  
! rtx
! store_bit_field (rtx str_rtx, unsigned HOST_WIDE_INT bitsize,
! 		 unsigned HOST_WIDE_INT bitnum, enum machine_mode fieldmode,
! 		 rtx value)
  {
    unsigned int unit
      = (MEM_P (str_rtx)) ? BITS_PER_UNIT : BITS_PER_WORD;
--- 328,360 ----
    return data->operand[opno].mode;
  }
  
+ /* Return true if X, of mode MODE, matches the predicate for operand
+    OPNO of instruction ICODE.  Allow volatile memories, regardless of
+    the ambient volatile_ok setting.  */
+ 
+ static bool
+ check_predicate_volatile_ok (enum insn_code icode, int opno,
+ 			     rtx x, enum machine_mode mode)
+ {
+   bool save_volatile_ok, result;
+ 
+   save_volatile_ok = volatile_ok;
+   result = insn_data[(int) icode].operand[opno].predicate (x, mode);
+   volatile_ok = save_volatile_ok;
+   return result;
+ }
  \f
! /* A subroutine of store_bit_field, with the same arguments.  Return true
!    if the operation could be implemented.
  
!    If FALLBACK_P is true, fall back to store_fixed_bit_field if we have
!    no other way of implementing the operation.  If FALLBACK_P is false,
!    return false instead.  */
  
! static bool
! store_bit_field_1 (rtx str_rtx, unsigned HOST_WIDE_INT bitsize,
! 		   unsigned HOST_WIDE_INT bitnum, enum machine_mode fieldmode,
! 		   rtx value, bool fallback_p)
  {
    unsigned int unit
      = (MEM_P (str_rtx)) ? BITS_PER_UNIT : BITS_PER_WORD;
*************** store_bit_field (rtx str_rtx, unsigned H
*** 391,397 ****
       lies completely outside that register.  This can occur if the source
       code contains an out-of-bounds access to a small array.  */
    if (REG_P (op0) && bitnum >= GET_MODE_BITSIZE (GET_MODE (op0)))
!     return value;
  
    /* Use vec_set patterns for inserting parts of vectors whenever
       available.  */
--- 398,404 ----
       lies completely outside that register.  This can occur if the source
       code contains an out-of-bounds access to a small array.  */
    if (REG_P (op0) && bitnum >= GET_MODE_BITSIZE (GET_MODE (op0)))
!     return true;
  
    /* Use vec_set patterns for inserting parts of vectors whenever
       available.  */
*************** store_bit_field (rtx str_rtx, unsigned H
*** 435,441 ****
  	{
  	  emit_insn (seq);
  	  emit_insn (pat);
! 	  return dest;
  	}
      }
  
--- 442,448 ----
  	{
  	  emit_insn (seq);
  	  emit_insn (pat);
! 	  return true;
  	}
      }
  
*************** store_bit_field (rtx str_rtx, unsigned H
*** 467,473 ****
  	op0 = simplify_gen_subreg (fieldmode, op0, GET_MODE (op0),
  				   byte_offset);
        emit_move_insn (op0, value);
!       return value;
      }
  
    /* Make sure we are playing with integral modes.  Pun with subregs
--- 474,480 ----
  	op0 = simplify_gen_subreg (fieldmode, op0, GET_MODE (op0),
  				   byte_offset);
        emit_move_insn (op0, value);
!       return true;
      }
  
    /* Make sure we are playing with integral modes.  Pun with subregs
*************** store_bit_field (rtx str_rtx, unsigned H
*** 544,550 ****
  				  + (offset * UNITS_PER_WORD)),
  				  value));
  
!       return value;
      }
  
    /* Handle fields bigger than a word.  */
--- 551,557 ----
  				  + (offset * UNITS_PER_WORD)),
  				  value));
  
!       return true;
      }
  
    /* Handle fields bigger than a word.  */
*************** store_bit_field (rtx str_rtx, unsigned H
*** 560,565 ****
--- 567,573 ----
        unsigned int backwards = WORDS_BIG_ENDIAN && fieldmode != BLKmode;
        unsigned int nwords = (bitsize + (BITS_PER_WORD - 1)) / BITS_PER_WORD;
        unsigned int i;
+       rtx last;
  
        /* This is the mode we must force value to, so that there will be enough
  	 subwords to extract.  Note that fieldmode will often (always?) be
*************** store_bit_field (rtx str_rtx, unsigned H
*** 570,575 ****
--- 578,584 ----
        if (fieldmode == VOIDmode)
  	fieldmode = smallest_mode_for_size (nwords * BITS_PER_WORD, MODE_INT);
  
+       last = get_last_insn ();
        for (i = 0; i < nwords; i++)
  	{
  	  /* If I is 0, use the low-order word in both field and target;
*************** store_bit_field (rtx str_rtx, unsigned H
*** 580,592 ****
  					    * BITS_PER_WORD,
  					    0)
  				     : (int) i * BITS_PER_WORD);
  
! 	  store_bit_field (op0, MIN (BITS_PER_WORD,
! 				     bitsize - i * BITS_PER_WORD),
! 			   bitnum + bit_offset, word_mode,
! 			   operand_subword_force (value, wordnum, fieldmode));
  	}
!       return value;
      }
  
    /* From here on we can assume that the field to be stored in is
--- 589,606 ----
  					    * BITS_PER_WORD,
  					    0)
  				     : (int) i * BITS_PER_WORD);
+ 	  rtx value_word = operand_subword_force (value, wordnum, fieldmode);
  
! 	  if (!store_bit_field_1 (op0, MIN (BITS_PER_WORD,
! 					    bitsize - i * BITS_PER_WORD),
! 				  bitnum + bit_offset, word_mode,
! 				  value_word, fallback_p))
! 	    {
! 	      delete_insns_since (last);
! 	      return false;
! 	    }
  	}
!       return true;
      }
  
    /* From here on we can assume that the field to be stored in is
*************** store_bit_field (rtx str_rtx, unsigned H
*** 640,713 ****
        && ! ((REG_P (op0) || GET_CODE (op0) == SUBREG)
  	    && (bitsize + bitpos > GET_MODE_BITSIZE (op_mode)))
        && insn_data[CODE_FOR_insv].operand[1].predicate (GEN_INT (bitsize),
! 							VOIDmode))
      {
        int xbitpos = bitpos;
        rtx value1;
        rtx xop0 = op0;
        rtx last = get_last_insn ();
        rtx pat;
-       enum machine_mode maxmode = mode_for_extraction (EP_insv, 3);
-       int save_volatile_ok = volatile_ok;
- 
-       volatile_ok = 1;
- 
-       /* If this machine's insv can only insert into a register, copy OP0
- 	 into a register and save it back later.  */
-       if (MEM_P (op0)
- 	  && ! ((*insn_data[(int) CODE_FOR_insv].operand[0].predicate)
- 		(op0, VOIDmode)))
- 	{
- 	  rtx tempreg;
- 	  enum machine_mode bestmode;
- 
- 	  /* Get the mode to use for inserting into this field.  If OP0 is
- 	     BLKmode, get the smallest mode consistent with the alignment. If
- 	     OP0 is a non-BLKmode object that is no wider than MAXMODE, use its
- 	     mode. Otherwise, use the smallest mode containing the field.  */
- 
- 	  if (GET_MODE (op0) == BLKmode
- 	      || GET_MODE_SIZE (GET_MODE (op0)) > GET_MODE_SIZE (maxmode))
- 	    bestmode
- 	      = get_best_mode (bitsize, bitnum, MEM_ALIGN (op0), maxmode,
- 			       MEM_VOLATILE_P (op0));
- 	  else
- 	    bestmode = GET_MODE (op0);
- 
- 	  if (bestmode == VOIDmode
- 	      || GET_MODE_SIZE (bestmode) < GET_MODE_SIZE (fieldmode)
- 	      || (SLOW_UNALIGNED_ACCESS (bestmode, MEM_ALIGN (op0))
- 		  && GET_MODE_BITSIZE (bestmode) > MEM_ALIGN (op0)))
- 	    goto insv_loses;
- 
- 	  /* Adjust address to point to the containing unit of that mode.
- 	     Compute offset as multiple of this unit, counting in bytes.  */
- 	  unit = GET_MODE_BITSIZE (bestmode);
- 	  offset = (bitnum / unit) * GET_MODE_SIZE (bestmode);
- 	  bitpos = bitnum % unit;
- 	  op0 = adjust_address (op0, bestmode,  offset);
- 
- 	  /* Fetch that unit, store the bitfield in it, then store
- 	     the unit.  */
- 	  tempreg = copy_to_reg (op0);
- 	  store_bit_field (tempreg, bitsize, bitpos, fieldmode, orig_value);
- 	  emit_move_insn (op0, tempreg);
- 	  return value;
- 	}
-       volatile_ok = save_volatile_ok;
  
        /* Add OFFSET into OP0's address.  */
        if (MEM_P (xop0))
  	xop0 = adjust_address (xop0, byte_mode, offset);
  
!       /* If xop0 is a register, we need it in MAXMODE
  	 to make it acceptable to the format of insv.  */
        if (GET_CODE (xop0) == SUBREG)
  	/* We can't just change the mode, because this might clobber op0,
  	   and we will need the original value of op0 if insv fails.  */
! 	xop0 = gen_rtx_SUBREG (maxmode, SUBREG_REG (xop0), SUBREG_BYTE (xop0));
!       if (REG_P (xop0) && GET_MODE (xop0) != maxmode)
! 	xop0 = gen_rtx_SUBREG (maxmode, xop0, 0);
  
        /* On big-endian machines, we count bits from the most significant.
  	 If the bit field insn does not, we must invert.  */
--- 654,680 ----
        && ! ((REG_P (op0) || GET_CODE (op0) == SUBREG)
  	    && (bitsize + bitpos > GET_MODE_BITSIZE (op_mode)))
        && insn_data[CODE_FOR_insv].operand[1].predicate (GEN_INT (bitsize),
! 							VOIDmode)
!       && check_predicate_volatile_ok (CODE_FOR_insv, 0, op0, VOIDmode))
      {
        int xbitpos = bitpos;
        rtx value1;
        rtx xop0 = op0;
        rtx last = get_last_insn ();
        rtx pat;
  
        /* Add OFFSET into OP0's address.  */
        if (MEM_P (xop0))
  	xop0 = adjust_address (xop0, byte_mode, offset);
  
!       /* If xop0 is a register, we need it in OP_MODE
  	 to make it acceptable to the format of insv.  */
        if (GET_CODE (xop0) == SUBREG)
  	/* We can't just change the mode, because this might clobber op0,
  	   and we will need the original value of op0 if insv fails.  */
! 	xop0 = gen_rtx_SUBREG (op_mode, SUBREG_REG (xop0), SUBREG_BYTE (xop0));
!       if (REG_P (xop0) && GET_MODE (xop0) != op_mode)
! 	xop0 = gen_rtx_SUBREG (op_mode, xop0, 0);
  
        /* On big-endian machines, we count bits from the most significant.
  	 If the bit field insn does not, we must invert.  */
*************** store_bit_field (rtx str_rtx, unsigned H
*** 718,730 ****
        /* We have been counting XBITPOS within UNIT.
  	 Count instead within the size of the register.  */
        if (BITS_BIG_ENDIAN && !MEM_P (xop0))
! 	xbitpos += GET_MODE_BITSIZE (maxmode) - unit;
  
!       unit = GET_MODE_BITSIZE (maxmode);
  
!       /* Convert VALUE to maxmode (which insv insn wants) in VALUE1.  */
        value1 = value;
!       if (GET_MODE (value) != maxmode)
  	{
  	  if (GET_MODE_BITSIZE (GET_MODE (value)) >= bitsize)
  	    {
--- 685,697 ----
        /* We have been counting XBITPOS within UNIT.
  	 Count instead within the size of the register.  */
        if (BITS_BIG_ENDIAN && !MEM_P (xop0))
! 	xbitpos += GET_MODE_BITSIZE (op_mode) - unit;
  
!       unit = GET_MODE_BITSIZE (op_mode);
  
!       /* Convert VALUE to op_mode (which insv insn wants) in VALUE1.  */
        value1 = value;
!       if (GET_MODE (value) != op_mode)
  	{
  	  if (GET_MODE_BITSIZE (GET_MODE (value)) >= bitsize)
  	    {
*************** store_bit_field (rtx str_rtx, unsigned H
*** 732,754 ****
  		 if it has all the bits we will actually use.  However,
  		 if we must narrow it, be sure we do it correctly.  */
  
! 	      if (GET_MODE_SIZE (GET_MODE (value)) < GET_MODE_SIZE (maxmode))
  		{
  		  rtx tmp;
  
! 		  tmp = simplify_subreg (maxmode, value1, GET_MODE (value), 0);
  		  if (! tmp)
! 		    tmp = simplify_gen_subreg (maxmode,
  					       force_reg (GET_MODE (value),
  							  value1),
  					       GET_MODE (value), 0);
  		  value1 = tmp;
  		}
  	      else
! 		value1 = gen_lowpart (maxmode, value1);
  	    }
  	  else if (GET_CODE (value) == CONST_INT)
! 	    value1 = gen_int_mode (INTVAL (value), maxmode);
  	  else
  	    /* Parse phase is supposed to make VALUE's data type
  	       match that of the component reference, which is a type
--- 699,721 ----
  		 if it has all the bits we will actually use.  However,
  		 if we must narrow it, be sure we do it correctly.  */
  
! 	      if (GET_MODE_SIZE (GET_MODE (value)) < GET_MODE_SIZE (op_mode))
  		{
  		  rtx tmp;
  
! 		  tmp = simplify_subreg (op_mode, value1, GET_MODE (value), 0);
  		  if (! tmp)
! 		    tmp = simplify_gen_subreg (op_mode,
  					       force_reg (GET_MODE (value),
  							  value1),
  					       GET_MODE (value), 0);
  		  value1 = tmp;
  		}
  	      else
! 		value1 = gen_lowpart (op_mode, value1);
  	    }
  	  else if (GET_CODE (value) == CONST_INT)
! 	    value1 = gen_int_mode (INTVAL (value), op_mode);
  	  else
  	    /* Parse phase is supposed to make VALUE's data type
  	       match that of the component reference, which is a type
*************** store_bit_field (rtx str_rtx, unsigned H
*** 760,782 ****
        /* If this machine's insv insists on a register,
  	 get VALUE1 into a register.  */
        if (! ((*insn_data[(int) CODE_FOR_insv].operand[3].predicate)
! 	     (value1, maxmode)))
! 	value1 = force_reg (maxmode, value1);
  
        pat = gen_insv (xop0, GEN_INT (bitsize), GEN_INT (xbitpos), value1);
        if (pat)
! 	emit_insn (pat);
        else
  	{
  	  delete_insns_since (last);
- 	  store_fixed_bit_field (op0, offset, bitsize, bitpos, value);
  	}
      }
!   else
!     insv_loses:
!     /* Insv is not available; store using shifts and boolean ops.  */
!     store_fixed_bit_field (op0, offset, bitsize, bitpos, value);
!   return value;
  }
  \f
  /* Use shifts and boolean operations to store VALUE
--- 727,815 ----
        /* If this machine's insv insists on a register,
  	 get VALUE1 into a register.  */
        if (! ((*insn_data[(int) CODE_FOR_insv].operand[3].predicate)
! 	     (value1, op_mode)))
! 	value1 = force_reg (op_mode, value1);
  
        pat = gen_insv (xop0, GEN_INT (bitsize), GEN_INT (xbitpos), value1);
        if (pat)
! 	{
! 	  emit_insn (pat);
! 	  return true;
! 	}
!       delete_insns_since (last);
!     }
! 
!   /* If OP0 is a memory, try copying it to a register and seeing if a
!      cheap register alternative is available.  */
!   if (HAVE_insv && MEM_P (op0))
!     {
!       enum machine_mode bestmode;
! 
!       /* Get the mode to use for inserting into this field.  If OP0 is
! 	 BLKmode, get the smallest mode consistent with the alignment. If
! 	 OP0 is a non-BLKmode object that is no wider than OP_MODE, use its
! 	 mode. Otherwise, use the smallest mode containing the field.  */
! 
!       if (GET_MODE (op0) == BLKmode
! 	  || (op_mode != MAX_MACHINE_MODE
! 	      && GET_MODE_SIZE (GET_MODE (op0)) > GET_MODE_SIZE (op_mode)))
! 	bestmode = get_best_mode (bitsize, bitnum, MEM_ALIGN (op0),
! 				  (op_mode == MAX_MACHINE_MODE
! 				   ? VOIDmode : op_mode),
! 				  MEM_VOLATILE_P (op0));
        else
+ 	bestmode = GET_MODE (op0);
+ 
+       if (bestmode != VOIDmode
+ 	  && GET_MODE_SIZE (bestmode) >= GET_MODE_SIZE (fieldmode)
+ 	  && !(SLOW_UNALIGNED_ACCESS (bestmode, MEM_ALIGN (op0))
+ 	       && GET_MODE_BITSIZE (bestmode) > MEM_ALIGN (op0)))
  	{
+ 	  rtx last, tempreg, xop0;
+ 	  unsigned HOST_WIDE_INT xoffset, xbitpos;
+ 
+ 	  last = get_last_insn ();
+ 
+ 	  /* Adjust address to point to the containing unit of
+ 	     that mode.  Compute the offset as a multiple of this unit,
+ 	     counting in bytes.  */
+ 	  unit = GET_MODE_BITSIZE (bestmode);
+ 	  xoffset = (bitnum / unit) * GET_MODE_SIZE (bestmode);
+ 	  xbitpos = bitnum % unit;
+ 	  xop0 = adjust_address (op0, bestmode, xoffset);
+ 
+ 	  /* Fetch that unit, store the bitfield in it, then store
+ 	     the unit.  */
+ 	  tempreg = copy_to_reg (xop0);
+ 	  if (store_bit_field_1 (tempreg, bitsize, xbitpos,
+ 				 fieldmode, orig_value, false))
+ 	    {
+ 	      emit_move_insn (xop0, tempreg);
+ 	      return true;
+ 	    }
  	  delete_insns_since (last);
  	}
      }
! 
!   if (!fallback_p)
!     return false;
! 
!   store_fixed_bit_field (op0, offset, bitsize, bitpos, value);
!   return true;
! }
! 
! /* Generate code to store value from rtx VALUE
!    into a bit-field within structure STR_RTX
!    containing BITSIZE bits starting at bit BITNUM.
!    FIELDMODE is the machine-mode of the FIELD_DECL node for this field.  */
! 
! void
! store_bit_field (rtx str_rtx, unsigned HOST_WIDE_INT bitsize,
! 		 unsigned HOST_WIDE_INT bitnum, enum machine_mode fieldmode,
! 		 rtx value)
! {
!   if (!store_bit_field_1 (str_rtx, bitsize, bitnum, fieldmode, value, true))
!     gcc_unreachable ();
  }
  \f
  /* Use shifts and boolean operations to store VALUE
*************** store_split_bit_field (rtx op0, unsigned
*** 1068,1107 ****
      }
  }
  \f
! /* Generate code to extract a byte-field from STR_RTX
!    containing BITSIZE bits, starting at BITNUM,
!    and put it in TARGET if possible (if TARGET is nonzero).
!    Regardless of TARGET, we return the rtx for where the value is placed.
  
!    STR_RTX is the structure containing the byte (a REG or MEM).
!    UNSIGNEDP is nonzero if this is an unsigned bit field.
!    MODE is the natural mode of the field value once extracted.
!    TMODE is the mode the caller would like the value to have;
!    but the value may be returned with type MODE instead.
  
!    TOTAL_SIZE is the size in bytes of the containing structure,
!    or -1 if varying.
  
!    If a TARGET is specified and we can store in it at no extra cost,
!    we do so, and return TARGET.
!    Otherwise, we return a REG of mode TMODE or MODE, with TMODE preferred
!    if they are equally easy.  */
  
! rtx
! extract_bit_field (rtx str_rtx, unsigned HOST_WIDE_INT bitsize,
! 		   unsigned HOST_WIDE_INT bitnum, int unsignedp, rtx target,
! 		   enum machine_mode mode, enum machine_mode tmode)
  {
    unsigned int unit
      = (MEM_P (str_rtx)) ? BITS_PER_UNIT : BITS_PER_WORD;
    unsigned HOST_WIDE_INT offset, bitpos;
    rtx op0 = str_rtx;
-   rtx spec_target = target;
-   rtx spec_target_subreg = 0;
    enum machine_mode int_mode;
!   enum machine_mode extv_mode = mode_for_extraction (EP_extv, 0);
!   enum machine_mode extzv_mode = mode_for_extraction (EP_extzv, 0);
    enum machine_mode mode1;
    int byte_offset;
  
    if (tmode == VOIDmode)
--- 1101,1152 ----
      }
  }
  \f
! /* A subroutine of extract_bit_field_1 that converts return value X
!    to either MODE or TMODE.  MODE, TMODE and UNSIGNEDP are arguments
!    to extract_bit_field.  */
  
! static rtx
! convert_extracted_bit_field (rtx x, enum machine_mode mode,
! 			     enum machine_mode tmode, bool unsignedp)
! {
!   if (GET_MODE (x) == tmode || GET_MODE (x) == mode)
!     return x;
  
!   /* If the x mode is not a scalar integral, first convert to the
!      integer mode of that size and then access it as a floating-point
!      value via a SUBREG.  */
!   if (!SCALAR_INT_MODE_P (tmode))
!     {
!       enum machine_mode smode;
  
!       smode = mode_for_size (GET_MODE_BITSIZE (tmode), MODE_INT, 0);
!       x = convert_to_mode (smode, x, unsignedp);
!       x = force_reg (smode, x);
!       return gen_lowpart (tmode, x);
!     }
  
!   return convert_to_mode (tmode, x, unsignedp);
! }
! 
! /* A subroutine of extract_bit_field, with the same arguments.
!    If FALLBACK_P is true, fall back to extract_fixed_bit_field
!    if we can find no other means of implementing the operation.
!    if FALLBACK_P is false, return NULL instead.  */
! 
! static rtx
! extract_bit_field_1 (rtx str_rtx, unsigned HOST_WIDE_INT bitsize,
! 		     unsigned HOST_WIDE_INT bitnum, int unsignedp, rtx target,
! 		     enum machine_mode mode, enum machine_mode tmode,
! 		     bool fallback_p)
  {
    unsigned int unit
      = (MEM_P (str_rtx)) ? BITS_PER_UNIT : BITS_PER_WORD;
    unsigned HOST_WIDE_INT offset, bitpos;
    rtx op0 = str_rtx;
    enum machine_mode int_mode;
!   enum machine_mode ext_mode;
    enum machine_mode mode1;
+   enum insn_code icode;
    int byte_offset;
  
    if (tmode == VOIDmode)
*************** extract_bit_field (rtx str_rtx, unsigned
*** 1410,1708 ****
      }
  
    /* Now OFFSET is nonzero only for memory operands.  */
! 
!   if (unsignedp)
      {
!       if (HAVE_extzv
! 	  && bitsize > 0
! 	  && GET_MODE_BITSIZE (extzv_mode) >= bitsize
! 	  && ! ((REG_P (op0) || GET_CODE (op0) == SUBREG)
! 		&& (bitsize + bitpos > GET_MODE_BITSIZE (extzv_mode))))
! 	{
! 	  unsigned HOST_WIDE_INT xbitpos = bitpos, xoffset = offset;
! 	  rtx bitsize_rtx, bitpos_rtx;
! 	  rtx last = get_last_insn ();
! 	  rtx xop0 = op0;
! 	  rtx xtarget = target;
! 	  rtx xspec_target = spec_target;
! 	  rtx xspec_target_subreg = spec_target_subreg;
! 	  rtx pat;
! 	  enum machine_mode maxmode = mode_for_extraction (EP_extzv, 0);
  
! 	  if (MEM_P (xop0))
! 	    {
! 	      int save_volatile_ok = volatile_ok;
! 	      volatile_ok = 1;
  
! 	      /* Is the memory operand acceptable?  */
! 	      if (! ((*insn_data[(int) CODE_FOR_extzv].operand[1].predicate)
! 		     (xop0, GET_MODE (xop0))))
! 		{
! 		  /* No, load into a reg and extract from there.  */
! 		  enum machine_mode bestmode;
  
! 		  /* Get the mode to use for inserting into this field.  If
! 		     OP0 is BLKmode, get the smallest mode consistent with the
! 		     alignment. If OP0 is a non-BLKmode object that is no
! 		     wider than MAXMODE, use its mode. Otherwise, use the
! 		     smallest mode containing the field.  */
! 
! 		  if (GET_MODE (xop0) == BLKmode
! 		      || (GET_MODE_SIZE (GET_MODE (op0))
! 			  > GET_MODE_SIZE (maxmode)))
! 		    bestmode = get_best_mode (bitsize, bitnum,
! 					      MEM_ALIGN (xop0), maxmode,
! 					      MEM_VOLATILE_P (xop0));
! 		  else
! 		    bestmode = GET_MODE (xop0);
! 
! 		  if (bestmode == VOIDmode
! 		      || (SLOW_UNALIGNED_ACCESS (bestmode, MEM_ALIGN (xop0))
! 			  && GET_MODE_BITSIZE (bestmode) > MEM_ALIGN (xop0)))
! 		    goto extzv_loses;
! 
! 		  /* Compute offset as multiple of this unit,
! 		     counting in bytes.  */
! 		  unit = GET_MODE_BITSIZE (bestmode);
! 		  xoffset = (bitnum / unit) * GET_MODE_SIZE (bestmode);
! 		  xbitpos = bitnum % unit;
! 		  xop0 = adjust_address (xop0, bestmode, xoffset);
! 
! 		  /* Make sure register is big enough for the whole field. */
! 		  if (xoffset * BITS_PER_UNIT + unit 
! 		      < offset * BITS_PER_UNIT + bitsize)
! 		    goto extzv_loses;
  
! 		  /* Fetch it to a register in that size.  */
! 		  xop0 = force_reg (bestmode, xop0);
  
! 		  /* XBITPOS counts within UNIT, which is what is expected.  */
! 		}
! 	      else
! 		/* Get ref to first byte containing part of the field.  */
! 		xop0 = adjust_address (xop0, byte_mode, xoffset);
  
! 	      volatile_ok = save_volatile_ok;
  	    }
  
! 	  /* If op0 is a register, we need it in MAXMODE (which is usually
! 	     SImode). to make it acceptable to the format of extzv.  */
! 	  if (GET_CODE (xop0) == SUBREG && GET_MODE (xop0) != maxmode)
! 	    goto extzv_loses;
! 	  if (REG_P (xop0) && GET_MODE (xop0) != maxmode)
! 	    xop0 = gen_rtx_SUBREG (maxmode, xop0, 0);
! 
! 	  /* On big-endian machines, we count bits from the most significant.
! 	     If the bit field insn does not, we must invert.  */
! 	  if (BITS_BIG_ENDIAN != BYTES_BIG_ENDIAN)
! 	    xbitpos = unit - bitsize - xbitpos;
! 
! 	  /* Now convert from counting within UNIT to counting in MAXMODE.  */
! 	  if (BITS_BIG_ENDIAN && !MEM_P (xop0))
! 	    xbitpos += GET_MODE_BITSIZE (maxmode) - unit;
  
! 	  unit = GET_MODE_BITSIZE (maxmode);
  
! 	  if (xtarget == 0)
! 	    xtarget = xspec_target = gen_reg_rtx (tmode);
  
! 	  if (GET_MODE (xtarget) != maxmode)
  	    {
! 	      if (REG_P (xtarget))
! 		{
! 		  int wider = (GET_MODE_SIZE (maxmode)
! 			       > GET_MODE_SIZE (GET_MODE (xtarget)));
! 		  xtarget = gen_lowpart (maxmode, xtarget);
! 		  if (wider)
! 		    xspec_target_subreg = xtarget;
! 		}
! 	      else
! 		xtarget = gen_reg_rtx (maxmode);
! 	    }
  
! 	  /* If this machine's extzv insists on a register target,
! 	     make sure we have one.  */
! 	  if (! ((*insn_data[(int) CODE_FOR_extzv].operand[0].predicate)
! 		 (xtarget, maxmode)))
! 	    xtarget = gen_reg_rtx (maxmode);
  
! 	  bitsize_rtx = GEN_INT (bitsize);
! 	  bitpos_rtx = GEN_INT (xbitpos);
  
- 	  pat = gen_extzv (xtarget, xop0, bitsize_rtx, bitpos_rtx);
- 	  if (pat)
- 	    {
- 	      emit_insn (pat);
- 	      target = xtarget;
- 	      spec_target = xspec_target;
- 	      spec_target_subreg = xspec_target_subreg;
- 	    }
- 	  else
- 	    {
  	      delete_insns_since (last);
- 	      target = extract_fixed_bit_field (int_mode, op0, offset, bitsize,
- 						bitpos, target, 1);
  	    }
  	}
-       else
-       extzv_loses:
- 	target = extract_fixed_bit_field (int_mode, op0, offset, bitsize,
- 					  bitpos, target, 1);
      }
-   else
-     {
-       if (HAVE_extv
- 	  && bitsize > 0
- 	  && GET_MODE_BITSIZE (extv_mode) >= bitsize
- 	  && ! ((REG_P (op0) || GET_CODE (op0) == SUBREG)
- 		&& (bitsize + bitpos > GET_MODE_BITSIZE (extv_mode))))
- 	{
- 	  int xbitpos = bitpos, xoffset = offset;
- 	  rtx bitsize_rtx, bitpos_rtx;
- 	  rtx last = get_last_insn ();
- 	  rtx xop0 = op0, xtarget = target;
- 	  rtx xspec_target = spec_target;
- 	  rtx xspec_target_subreg = spec_target_subreg;
- 	  rtx pat;
- 	  enum machine_mode maxmode = mode_for_extraction (EP_extv, 0);
- 
- 	  if (MEM_P (xop0))
- 	    {
- 	      /* Is the memory operand acceptable?  */
- 	      if (! ((*insn_data[(int) CODE_FOR_extv].operand[1].predicate)
- 		     (xop0, GET_MODE (xop0))))
- 		{
- 		  /* No, load into a reg and extract from there.  */
- 		  enum machine_mode bestmode;
- 
- 		  /* Get the mode to use for inserting into this field.  If
- 		     OP0 is BLKmode, get the smallest mode consistent with the
- 		     alignment. If OP0 is a non-BLKmode object that is no
- 		     wider than MAXMODE, use its mode. Otherwise, use the
- 		     smallest mode containing the field.  */
- 
- 		  if (GET_MODE (xop0) == BLKmode
- 		      || (GET_MODE_SIZE (GET_MODE (op0))
- 			  > GET_MODE_SIZE (maxmode)))
- 		    bestmode = get_best_mode (bitsize, bitnum,
- 					      MEM_ALIGN (xop0), maxmode,
- 					      MEM_VOLATILE_P (xop0));
- 		  else
- 		    bestmode = GET_MODE (xop0);
- 
- 		  if (bestmode == VOIDmode
- 		      || (SLOW_UNALIGNED_ACCESS (bestmode, MEM_ALIGN (xop0))
- 			  && GET_MODE_BITSIZE (bestmode) > MEM_ALIGN (xop0)))
- 		    goto extv_loses;
- 
- 		  /* Compute offset as multiple of this unit,
- 		     counting in bytes.  */
- 		  unit = GET_MODE_BITSIZE (bestmode);
- 		  xoffset = (bitnum / unit) * GET_MODE_SIZE (bestmode);
- 		  xbitpos = bitnum % unit;
- 		  xop0 = adjust_address (xop0, bestmode, xoffset);
- 
- 		  /* Make sure register is big enough for the whole field. */
- 		  if (xoffset * BITS_PER_UNIT + unit 
- 		      < offset * BITS_PER_UNIT + bitsize)
- 		    goto extv_loses;
  
! 		  /* Fetch it to a register in that size.  */
! 		  xop0 = force_reg (bestmode, xop0);
! 
! 		  /* XBITPOS counts within UNIT, which is what is expected.  */
! 		}
! 	      else
! 		/* Get ref to first byte containing part of the field.  */
! 		xop0 = adjust_address (xop0, byte_mode, xoffset);
! 	    }
! 
! 	  /* If op0 is a register, we need it in MAXMODE (which is usually
! 	     SImode) to make it acceptable to the format of extv.  */
! 	  if (GET_CODE (xop0) == SUBREG && GET_MODE (xop0) != maxmode)
! 	    goto extv_loses;
! 	  if (REG_P (xop0) && GET_MODE (xop0) != maxmode)
! 	    xop0 = gen_rtx_SUBREG (maxmode, xop0, 0);
! 
! 	  /* On big-endian machines, we count bits from the most significant.
! 	     If the bit field insn does not, we must invert.  */
! 	  if (BITS_BIG_ENDIAN != BYTES_BIG_ENDIAN)
! 	    xbitpos = unit - bitsize - xbitpos;
! 
! 	  /* XBITPOS counts within a size of UNIT.
! 	     Adjust to count within a size of MAXMODE.  */
! 	  if (BITS_BIG_ENDIAN && !MEM_P (xop0))
! 	    xbitpos += (GET_MODE_BITSIZE (maxmode) - unit);
! 
! 	  unit = GET_MODE_BITSIZE (maxmode);
! 
! 	  if (xtarget == 0)
! 	    xtarget = xspec_target = gen_reg_rtx (tmode);
  
! 	  if (GET_MODE (xtarget) != maxmode)
! 	    {
! 	      if (REG_P (xtarget))
! 		{
! 		  int wider = (GET_MODE_SIZE (maxmode)
! 			       > GET_MODE_SIZE (GET_MODE (xtarget)));
! 		  xtarget = gen_lowpart (maxmode, xtarget);
! 		  if (wider)
! 		    xspec_target_subreg = xtarget;
! 		}
! 	      else
! 		xtarget = gen_reg_rtx (maxmode);
! 	    }
  
! 	  /* If this machine's extv insists on a register target,
! 	     make sure we have one.  */
! 	  if (! ((*insn_data[(int) CODE_FOR_extv].operand[0].predicate)
! 		 (xtarget, maxmode)))
! 	    xtarget = gen_reg_rtx (maxmode);
  
! 	  bitsize_rtx = GEN_INT (bitsize);
! 	  bitpos_rtx = GEN_INT (xbitpos);
  
! 	  pat = gen_extv (xtarget, xop0, bitsize_rtx, bitpos_rtx);
! 	  if (pat)
! 	    {
! 	      emit_insn (pat);
! 	      target = xtarget;
! 	      spec_target = xspec_target;
! 	      spec_target_subreg = xspec_target_subreg;
! 	    }
! 	  else
! 	    {
! 	      delete_insns_since (last);
! 	      target = extract_fixed_bit_field (int_mode, op0, offset, bitsize,
! 						bitpos, target, 0);
! 	    }
! 	}
!       else
!       extv_loses:
! 	target = extract_fixed_bit_field (int_mode, op0, offset, bitsize,
! 					  bitpos, target, 0);
!     }
!   if (target == spec_target)
!     return target;
!   if (target == spec_target_subreg)
!     return spec_target;
!   if (GET_MODE (target) != tmode && GET_MODE (target) != mode)
!     {
!       /* If the target mode is not a scalar integral, first convert to the
! 	 integer mode of that size and then access it as a floating-point
! 	 value via a SUBREG.  */
!       if (!SCALAR_INT_MODE_P (tmode))
! 	{
! 	  enum machine_mode smode
! 	    = mode_for_size (GET_MODE_BITSIZE (tmode), MODE_INT, 0);
! 	  target = convert_to_mode (smode, target, unsignedp);
! 	  target = force_reg (smode, target);
! 	  return gen_lowpart (tmode, target);
! 	}
  
!       return convert_to_mode (tmode, target, unsignedp);
!     }
!   return target;
  }
  \f
  /* Extract a bit field using shifts and boolean operations
--- 1455,1626 ----
      }
  
    /* Now OFFSET is nonzero only for memory operands.  */
!   ext_mode = mode_for_extraction (unsignedp ? EP_extzv : EP_extv, 0);
!   icode = unsignedp ? CODE_FOR_extzv : CODE_FOR_extv;
!   if (ext_mode != MAX_MACHINE_MODE
!       && bitsize > 0
!       && GET_MODE_BITSIZE (ext_mode) >= bitsize
!       /* If op0 is a register, we need it in EXT_MODE to make it
! 	 acceptable to the format of ext(z)v.  */
!       && !(GET_CODE (op0) == SUBREG && GET_MODE (op0) != ext_mode)
!       && !((REG_P (op0) || GET_CODE (op0) == SUBREG)
! 	   && (bitsize + bitpos > GET_MODE_BITSIZE (ext_mode)))
!       && check_predicate_volatile_ok (icode, 1, op0, GET_MODE (op0)))
      {
!       unsigned HOST_WIDE_INT xbitpos = bitpos, xoffset = offset;
!       rtx bitsize_rtx, bitpos_rtx;
!       rtx last = get_last_insn ();
!       rtx xop0 = op0;
!       rtx xtarget = target;
!       rtx xspec_target = target;
!       rtx xspec_target_subreg = 0;
!       rtx pat;
  
!       /* If op0 is a register, we need it in EXT_MODE to make it
! 	 acceptable to the format of ext(z)v.  */
!       if (REG_P (xop0) && GET_MODE (xop0) != ext_mode)
! 	xop0 = gen_rtx_SUBREG (ext_mode, xop0, 0);
!       if (MEM_P (xop0))
! 	/* Get ref to first byte containing part of the field.  */
! 	xop0 = adjust_address (xop0, byte_mode, xoffset);
  
!       /* On big-endian machines, we count bits from the most significant.
! 	 If the bit field insn does not, we must invert.  */
!       if (BITS_BIG_ENDIAN != BYTES_BIG_ENDIAN)
! 	xbitpos = unit - bitsize - xbitpos;
  
!       /* Now convert from counting within UNIT to counting in EXT_MODE.  */
!       if (BITS_BIG_ENDIAN && !MEM_P (xop0))
! 	xbitpos += GET_MODE_BITSIZE (ext_mode) - unit;
  
!       unit = GET_MODE_BITSIZE (ext_mode);
  
!       if (xtarget == 0)
! 	xtarget = xspec_target = gen_reg_rtx (tmode);
  
!       if (GET_MODE (xtarget) != ext_mode)
! 	{
! 	  if (REG_P (xtarget))
! 	    {
! 	      xtarget = gen_lowpart (ext_mode, xtarget);
! 	      if (GET_MODE_SIZE (ext_mode)
! 		  > GET_MODE_SIZE (GET_MODE (xspec_target)))
! 		xspec_target_subreg = xtarget;
  	    }
+ 	  else
+ 	    xtarget = gen_reg_rtx (ext_mode);
+ 	}
+ 
+       /* If this machine's ext(z)v insists on a register target,
+ 	 make sure we have one.  */
+       if (!insn_data[(int) icode].operand[0].predicate (xtarget, ext_mode))
+ 	xtarget = gen_reg_rtx (ext_mode);
  
!       bitsize_rtx = GEN_INT (bitsize);
!       bitpos_rtx = GEN_INT (xbitpos);
  
!       pat = (unsignedp
! 	     ? gen_extzv (xtarget, xop0, bitsize_rtx, bitpos_rtx)
! 	     : gen_extv (xtarget, xop0, bitsize_rtx, bitpos_rtx));
!       if (pat)
! 	{
! 	  emit_insn (pat);
! 	  if (xtarget == xspec_target)
! 	    return xtarget;
! 	  if (xtarget == xspec_target_subreg)
! 	    return xspec_target;
! 	  return convert_extracted_bit_field (xtarget, mode, tmode, unsignedp);
! 	}
!       delete_insns_since (last);
!     }
! 
!   /* If OP0 is a memory, try copying it to a register and seeing if a
!      cheap register alternative is available.  */
!   if (ext_mode != MAX_MACHINE_MODE && MEM_P (op0))
!     {
!       enum machine_mode bestmode;
! 
!       /* Get the mode to use for inserting into this field.  If
! 	 OP0 is BLKmode, get the smallest mode consistent with the
! 	 alignment. If OP0 is a non-BLKmode object that is no
! 	 wider than EXT_MODE, use its mode. Otherwise, use the
! 	 smallest mode containing the field.  */
! 
!       if (GET_MODE (op0) == BLKmode
! 	  || (ext_mode != MAX_MACHINE_MODE
! 	      && GET_MODE_SIZE (GET_MODE (op0)) > GET_MODE_SIZE (ext_mode)))
! 	bestmode = get_best_mode (bitsize, bitnum, MEM_ALIGN (op0),
! 				  (ext_mode == MAX_MACHINE_MODE
! 				   ? VOIDmode : ext_mode),
! 				  MEM_VOLATILE_P (op0));
!       else
! 	bestmode = GET_MODE (op0);
  
!       if (bestmode != VOIDmode
! 	  && !(SLOW_UNALIGNED_ACCESS (bestmode, MEM_ALIGN (op0))
! 	       && GET_MODE_BITSIZE (bestmode) > MEM_ALIGN (op0)))
! 	{
! 	  unsigned HOST_WIDE_INT xoffset, xbitpos;
  
! 	  /* Compute the offset as a multiple of this unit,
! 	     counting in bytes.  */
! 	  unit = GET_MODE_BITSIZE (bestmode);
! 	  xoffset = (bitnum / unit) * GET_MODE_SIZE (bestmode);
! 	  xbitpos = bitnum % unit;
! 
! 	  /* Make sure the register is big enough for the whole field.  */
! 	  if (xoffset * BITS_PER_UNIT + unit
! 	      >= offset * BITS_PER_UNIT + bitsize)
  	    {
! 	      rtx last, result, xop0;
  
! 	      last = get_last_insn ();
  
! 	      /* Fetch it to a register in that size.  */
! 	      xop0 = adjust_address (op0, bestmode, xoffset);
! 	      xop0 = force_reg (bestmode, xop0);
! 	      result = extract_bit_field_1 (xop0, bitsize, xbitpos,
! 					    unsignedp, target,
! 					    mode, tmode, false);
! 	      if (result)
! 		return result;
  
  	      delete_insns_since (last);
  	    }
  	}
      }
  
!   if (!fallback_p)
!     return NULL;
  
!   target = extract_fixed_bit_field (int_mode, op0, offset, bitsize,
! 				    bitpos, target, unsignedp);
!   return convert_extracted_bit_field (target, mode, tmode, unsignedp);
! }
  
! /* Generate code to extract a byte-field from STR_RTX
!    containing BITSIZE bits, starting at BITNUM,
!    and put it in TARGET if possible (if TARGET is nonzero).
!    Regardless of TARGET, we return the rtx for where the value is placed.
  
!    STR_RTX is the structure containing the byte (a REG or MEM).
!    UNSIGNEDP is nonzero if this is an unsigned bit field.
!    MODE is the natural mode of the field value once extracted.
!    TMODE is the mode the caller would like the value to have;
!    but the value may be returned with type MODE instead.
  
!    If a TARGET is specified and we can store in it at no extra cost,
!    we do so, and return TARGET.
!    Otherwise, we return a REG of mode TMODE or MODE, with TMODE preferred
!    if they are equally easy.  */
  
! rtx
! extract_bit_field (rtx str_rtx, unsigned HOST_WIDE_INT bitsize,
! 		   unsigned HOST_WIDE_INT bitnum, int unsignedp, rtx target,
! 		   enum machine_mode mode, enum machine_mode tmode)
! {
!   return extract_bit_field_1 (str_rtx, bitsize, bitnum, unsignedp,
! 			      target, mode, tmode, true);
  }
  \f
  /* Extract a bit field using shifts and boolean operations
Index: gcc/testsuite/gcc.target/mips/ins-1.c
===================================================================
*** /dev/null	2007-07-16 08:54:14.588099250 +0100
--- gcc/testsuite/gcc.target/mips/ins-1.c	2007-07-18 11:04:38.000000000 +0100
***************
*** 0 ****
--- 1,16 ----
+ /* { dg-do compile } */
+ /* { dg-mips-options "-O -march=mips32r2 -mno-mips16" } */
+ /* { dg-final { scan-assembler "\tins\t" } } */
+ 
+ struct
+ {
+   unsigned int i : 2;
+   unsigned int j : 3;
+   unsigned int k : 4;
+ } s;
+ 
+ void
+ foo (void)
+ {
+   s.j = 1;
+ }

Re: Make more use of register insv and ext*v expanders

[Adam Nemet]

Richard Sandiford <richard@codesourcery.com> writes:
> Unfortunately, on ports like MIPS, the structure operand isn't enough
> on its own to determine whether a given memory operation is supported.
> The expanders check for various combinations of operands and FAIL on
> invalid ones.

Yes, we also have a similar change in our tree that I was meaning to
prepare for submission.  Thanks for working on this!

> If an expander FAILs on invalid memory operations, store_bit_field
> and extract_bit_field just carry on as though no expander existed.
> On ports that have both register and memory operations, it would be
> better to try a register operation instead, just as if the memory
> operand had been rejected outright.

I haven't applied the patch and the diff is kind of hard to read so
let me just ask.  Once we fall back on the register case do you
reverse the effect of advancing the memory operand to the first byte
via adjust_address?  We really want to fetch the original aligned
whole word on SLOW_BYTE_ACCESS targets (MIPS).

Adam

Re: Make more use of register insv and ext*v expanders

[Richard Sandiford]

Thanks for the support.

Adam Nemet <anemet@caviumnetworks.com> writes:
> Richard Sandiford <richard@codesourcery.com> writes:
>> If an expander FAILs on invalid memory operations, store_bit_field
>> and extract_bit_field just carry on as though no expander existed.
>> On ports that have both register and memory operations, it would be
>> better to try a register operation instead, just as if the memory
>> operand had been rejected outright.
>
> I haven't applied the patch and the diff is kind of hard to read so

[Yeah, it is, sorry about that.  It looked even worse in unified form.]

> let me just ask.  Once we fall back on the register case do you
> reverse the effect of advancing the memory operand to the first byte
> via adjust_address?  We really want to fetch the original aligned
> whole word on SLOW_BYTE_ACCESS targets (MIPS).

Do you mean this bit:

      /* Add OFFSET into OP0's address.  */
      if (MEM_P (xop0))
 	xop0 = adjust_address (xop0, byte_mode, offset);

?  If so, then yeah, we discard that adjustment for the register case.
The register code runs with op0, bitpos, etc. set to the same values
they have when entering the original MEM code; all the changes made
by the MEM attempt are stored in block-local variables.

Richard

Re: Make more use of register insv and ext*v expanders

[Adam Nemet]

Richard Sandiford writes:
> Do you mean this bit:
> 
>       /* Add OFFSET into OP0's address.  */
>       if (MEM_P (xop0))
>  	xop0 = adjust_address (xop0, byte_mode, offset);
> 
> ?  If so, then yeah, we discard that adjustment for the register case.

Yes, that one.  Thanks and hope it gets approved soon.

Adam

Re: Make more use of register insv and ext*v expanders

[Roman Zippel]

Hi,

On Wed, 18 Jul 2007, Richard Sandiford wrote:

> ...on targets that also have limited memory expanders.
> 
> The insv, extv and extzv patterns can ask for the structure operand
> to be a register, a memory, or either.  If the predicate rejects a
> memory operand, store_bit_field and extract_bit_field try using a
> register operand instead.

Hmm, I already wanted to get the patch reverted I mentioned here:

http://gcc.gnu.org/ml/gcc-patches/2007-07/msg01169.html

The problem is that this would conflict with your patch and since you 
already work in this area I hope you can give some help how to resolve 
this. :)
Any idea how I can get gcc again to use and/or instructions for constant 
single-bit bitfields instead of bitfield instructions? Should I just 
reject this in the expander or is there something better?

bye, Roman

Re: Make more use of register insv and ext*v expanders

[Richard Sandiford]

Roman Zippel <zippel@linux-m68k.org> writes:
>> ...on targets that also have limited memory expanders.
>> 
>> The insv, extv and extzv patterns can ask for the structure operand
>> to be a register, a memory, or either.  If the predicate rejects a
>> memory operand, store_bit_field and extract_bit_field try using a
>> register operand instead.
>
> Hmm, I already wanted to get the patch reverted I mentioned here:
>
> http://gcc.gnu.org/ml/gcc-patches/2007-07/msg01169.html
>
> The problem is that this would conflict with your patch and since you 
> already work in this area I hope you can give some help how to resolve 
> this. :)

I think it's an orthogonal issue.  Let's not try to solve everything
at once. ;)  If we want to add that check back, we can easily add it
back to either the pre-patch or post-patch sources.

> Any idea how I can get gcc again to use and/or instructions for constant 
> single-bit bitfields instead of bitfield instructions? Should I just 
> reject this in the expander or is there something better?

This is related to the MIPS all-bits-set case I mentioned.
Perhaps the conceptually cleanest thing is to choose between
insv and store_fixed_bit_field based on rtx costs, but I haven't
really thought about it much.

Richard

Re: Make more use of register insv and ext*v expanders

[Ian Lance Taylor]

Richard Sandiford <richard@codesourcery.com> writes:

> gcc/
> 	* expr.h (store_bit_field): Don't return a value.
> 	* expmed.c (check_predicate_volatile_ok): New function.
> 	(store_bit_field_1): New function, extracted from store_bit_field.
> 	Take a fallback_p argument and return true if the operation succeeded.
> 	Only use store_fixed_bit_field if fallback_p.  Don't recompute
> 	mode_for_extraction; use op_mode instead.  Try forcing memories
> 	into registers if the insv expander fails.
> 	(store_bit_field): Use store_bit_field_1 with fallback_p true.
> 	Don't return a value.
> 	(convert_extracted_bit_field): New function, extracted from
> 	store_bit_field.
> 	(extract_bit_field_1): Likewise.  Take a fallback_p argument
> 	and return NULL if the operation succeeded.  Only use
> 	extract_fixed_bit_field if fallback_p.  Only calculate one
> 	extraction mode.  Combine code for extv and extzv.  Try forcing
> 	memories into registers if the ext(z)v expander fails.
> 	(extract_bit_field): Use extract_bit_field_1 with fallback_p true.
> 
> gcc/testsuite/
> 	* gcc.target/mips/ins-1.c: New test.

This is OK.

Thanks.

Ian