diff mbox series

[3/3] MSP430: Simplify and extend shift instruction patterns

Message ID 20200721182953.wh2sprenianogipq@jozef-acer-manjaro
State New
Headers show
Series MSP430: Improve code-generation for shift instructions | expand

Commit Message

Jozef Lawrynowicz July 21, 2020, 6:29 p.m. UTC
The implementation of define_expand and define_insn patterns to handle
shifts in the MSP430 backend is inconsistent, resulting in missed
opportunities to make best use of the architecture's features.

There's now a single define_expand used as the entry point for all valid
shifts, and the decision to either use a helper function to perform the
shift (often required for the 430 ISA), or fall through to the
define_insn patterns can be made from that expander function.

Shifts by a constant amount have been grouped into one define_insn for
each type of shift, instead of having different define_insn patterns for
shifts by different amounts.

A new target option "-mmax-inline-shift=" has been added to allow tuning
of the number of shift instructions to emit inline, instead of using
a library helper function.

Successfully regtested on trunk for msp430-elf, ok to apply?
From a3c62c448c7f359bad85c86c35f712ca1fccf219 Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
Date: Wed, 15 Jul 2020 11:43:36 +0100
Subject: [PATCH 3/3] MSP430: Simplify and extend shift instruction patterns

The implementation of define_expand and define_insn patterns to handle
shifts in the MSP430 backend is inconsistent, resulting in missed
opportunities to make best use of the architecture's features.

There's now a single define_expand used as the entry point for all valid
shifts, and the decision to either use a helper function to perform the
shift (often required for the 430 ISA), or fall through to the
define_insn patterns can be made from that expander function.

Shifts by a constant amount have been grouped into one define_insn for
each type of shift, instead of having different define_insn patterns for
shifts by different amounts.

A new target option "-mmax-inline-shift=" has been added to allow tuning
of the number of shift instructions to emit inline, instead of using
a library helper function.

gcc/ChangeLog:

	* config/msp430/constraints.md (K): Change unused constraint to
	constraint to a const_int between 1 and 19.
	(P): New constraint.
	* config/msp430/msp430-protos.h (msp430x_logical_shift_right): Remove.
	(msp430_expand_shift): New.
	(msp430_output_asm_shift_insns): New.
	* config/msp430/msp430.c (msp430_rtx_costs): Remove shift costs.
	(CSH): Remove.
	(msp430_expand_helper): Remove hard-coded generation of some inline
	shift insns.
	(use_helper_for_const_shift): New.
	(msp430_expand_shift): New.
	(msp430_output_asm_shift_insns): New.
	(msp430_print_operand): Add new 'W' operand selector.
	(msp430x_logical_shift_right): Remove.
	* config/msp430/msp430.md (HPSI): New define_mode_iterator.
	(HDI): Likewise.
	(any_shift): New define_code_iterator.
	(shift_insn): New define_code_attr.
	Adjust unnamed insn patterns searched for by combine.
	(ashlhi3): Remove.
	(slli_1): Remove.
	(430x_shift_left): Remove.
	(slll_1): Remove.
	(slll_2): Remove.
	(ashlsi3): Remove.
	(ashldi3): Remove.
	(ashrhi3): Remove.
	(srai_1): Remove.
	(430x_arithmetic_shift_right): Remove.
	(srap_1): Remove.
	(srap_2): Remove.
	(sral_1): Remove.
	(sral_2): Remove.
	(ashrsi3): Remove.
	(ashrdi3): Remove.
	(lshrhi3): Remove.
	(srli_1): Remove.
	(430x_logical_shift_right): Remove.
	(srlp_1): Remove.
	(srll_1): Remove.
	(srll_2x): Remove.
	(lshrsi3): Remove.
	(lshrdi3): Remove.
	(<shift_insn><mode>3): New define_expand.
	(<shift_insn>hi3_430): New define_insn.
	(<shift_insn>si3_const): Likewise.
	(ashl<mode>3_430x): Likewise.
	(ashr<mode>3_430x): Likewise.
	(lshr<mode>3_430x): Likewise.
	(*bitbranch<mode>4_z): Replace renamed predicate msp430_bitpos with
	const_0_to_15_operand.
	* config/msp430/msp430.opt: New option -mmax-inline-shift=.
	* config/msp430/predicates.md (const_1_to_8_operand): New predicate.
	(const_0_to_15_operand): Rename msp430_bitpos predicate.
	(const_1_to_19_operand): New predicate.
	* doc/invoke.texi: Document -mmax-inline-shift=.

libgcc/ChangeLog:

	* config/msp430/slli.S (__gnu_mspabi_sllp): New.
	* config/msp430/srai.S (__gnu_mspabi_srap): New.
	* config/msp430/srli.S (__gnu_mspabi_srlp): New.

gcc/testsuite/ChangeLog:

	* gcc.target/msp430/emulate-srli.c: Fix expected assembler text.
	* gcc.target/msp430/max-inline-shift-430-no-opt.c: New test.
	* gcc.target/msp430/max-inline-shift-430.c: New test.
	* gcc.target/msp430/max-inline-shift-430x.c: New test.

---
 gcc/config/msp430/constraints.md              |  10 +-
 gcc/config/msp430/msp430-protos.h             |   6 +-
 gcc/config/msp430/msp430.c                    | 272 +++++++++----
 gcc/config/msp430/msp430.md                   | 381 +++++-------------
 gcc/config/msp430/msp430.opt                  |   6 +
 gcc/config/msp430/predicates.md               |  13 +-
 gcc/doc/invoke.texi                           |  15 +-
 .../gcc.target/msp430/emulate-srli.c          |   2 +-
 .../msp430/max-inline-shift-430-no-opt.c      |  52 +++
 .../gcc.target/msp430/max-inline-shift-430.c  |  50 +++
 .../gcc.target/msp430/max-inline-shift-430x.c |  48 +++
 libgcc/config/msp430/slli.S                   |  15 +
 libgcc/config/msp430/srai.S                   |  15 +
 libgcc/config/msp430/srli.S                   |  16 +
 14 files changed, 527 insertions(+), 374 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/msp430/max-inline-shift-430-no-opt.c
 create mode 100644 gcc/testsuite/gcc.target/msp430/max-inline-shift-430.c
 create mode 100644 gcc/testsuite/gcc.target/msp430/max-inline-shift-430x.c

Comments

Jozef Lawrynowicz Aug. 7, 2020, 10:55 a.m. UTC | #1
Pinging for this patch.

Thanks,
Jozef

On Tue, Jul 21, 2020 at 07:29:53PM +0100, Jozef Lawrynowicz wrote:
> The implementation of define_expand and define_insn patterns to handle
> shifts in the MSP430 backend is inconsistent, resulting in missed
> opportunities to make best use of the architecture's features.
> 
> There's now a single define_expand used as the entry point for all valid
> shifts, and the decision to either use a helper function to perform the
> shift (often required for the 430 ISA), or fall through to the
> define_insn patterns can be made from that expander function.
> 
> Shifts by a constant amount have been grouped into one define_insn for
> each type of shift, instead of having different define_insn patterns for
> shifts by different amounts.
> 
> A new target option "-mmax-inline-shift=" has been added to allow tuning
> of the number of shift instructions to emit inline, instead of using
> a library helper function.
> 
> Successfully regtested on trunk for msp430-elf, ok to apply?

> From a3c62c448c7f359bad85c86c35f712ca1fccf219 Mon Sep 17 00:00:00 2001
> From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
> Date: Wed, 15 Jul 2020 11:43:36 +0100
> Subject: [PATCH 3/3] MSP430: Simplify and extend shift instruction patterns
> 
> The implementation of define_expand and define_insn patterns to handle
> shifts in the MSP430 backend is inconsistent, resulting in missed
> opportunities to make best use of the architecture's features.
> 
> There's now a single define_expand used as the entry point for all valid
> shifts, and the decision to either use a helper function to perform the
> shift (often required for the 430 ISA), or fall through to the
> define_insn patterns can be made from that expander function.
> 
> Shifts by a constant amount have been grouped into one define_insn for
> each type of shift, instead of having different define_insn patterns for
> shifts by different amounts.
> 
> A new target option "-mmax-inline-shift=" has been added to allow tuning
> of the number of shift instructions to emit inline, instead of using
> a library helper function.
> 
> gcc/ChangeLog:
> 
> 	* config/msp430/constraints.md (K): Change unused constraint to
> 	constraint to a const_int between 1 and 19.
> 	(P): New constraint.
> 	* config/msp430/msp430-protos.h (msp430x_logical_shift_right): Remove.
> 	(msp430_expand_shift): New.
> 	(msp430_output_asm_shift_insns): New.
> 	* config/msp430/msp430.c (msp430_rtx_costs): Remove shift costs.
> 	(CSH): Remove.
> 	(msp430_expand_helper): Remove hard-coded generation of some inline
> 	shift insns.
> 	(use_helper_for_const_shift): New.
> 	(msp430_expand_shift): New.
> 	(msp430_output_asm_shift_insns): New.
> 	(msp430_print_operand): Add new 'W' operand selector.
> 	(msp430x_logical_shift_right): Remove.
> 	* config/msp430/msp430.md (HPSI): New define_mode_iterator.
> 	(HDI): Likewise.
> 	(any_shift): New define_code_iterator.
> 	(shift_insn): New define_code_attr.
> 	Adjust unnamed insn patterns searched for by combine.
> 	(ashlhi3): Remove.
> 	(slli_1): Remove.
> 	(430x_shift_left): Remove.
> 	(slll_1): Remove.
> 	(slll_2): Remove.
> 	(ashlsi3): Remove.
> 	(ashldi3): Remove.
> 	(ashrhi3): Remove.
> 	(srai_1): Remove.
> 	(430x_arithmetic_shift_right): Remove.
> 	(srap_1): Remove.
> 	(srap_2): Remove.
> 	(sral_1): Remove.
> 	(sral_2): Remove.
> 	(ashrsi3): Remove.
> 	(ashrdi3): Remove.
> 	(lshrhi3): Remove.
> 	(srli_1): Remove.
> 	(430x_logical_shift_right): Remove.
> 	(srlp_1): Remove.
> 	(srll_1): Remove.
> 	(srll_2x): Remove.
> 	(lshrsi3): Remove.
> 	(lshrdi3): Remove.
> 	(<shift_insn><mode>3): New define_expand.
> 	(<shift_insn>hi3_430): New define_insn.
> 	(<shift_insn>si3_const): Likewise.
> 	(ashl<mode>3_430x): Likewise.
> 	(ashr<mode>3_430x): Likewise.
> 	(lshr<mode>3_430x): Likewise.
> 	(*bitbranch<mode>4_z): Replace renamed predicate msp430_bitpos with
> 	const_0_to_15_operand.
> 	* config/msp430/msp430.opt: New option -mmax-inline-shift=.
> 	* config/msp430/predicates.md (const_1_to_8_operand): New predicate.
> 	(const_0_to_15_operand): Rename msp430_bitpos predicate.
> 	(const_1_to_19_operand): New predicate.
> 	* doc/invoke.texi: Document -mmax-inline-shift=.
> 
> libgcc/ChangeLog:
> 
> 	* config/msp430/slli.S (__gnu_mspabi_sllp): New.
> 	* config/msp430/srai.S (__gnu_mspabi_srap): New.
> 	* config/msp430/srli.S (__gnu_mspabi_srlp): New.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.target/msp430/emulate-srli.c: Fix expected assembler text.
> 	* gcc.target/msp430/max-inline-shift-430-no-opt.c: New test.
> 	* gcc.target/msp430/max-inline-shift-430.c: New test.
> 	* gcc.target/msp430/max-inline-shift-430x.c: New test.
> 
> ---
>  gcc/config/msp430/constraints.md              |  10 +-
>  gcc/config/msp430/msp430-protos.h             |   6 +-
>  gcc/config/msp430/msp430.c                    | 272 +++++++++----
>  gcc/config/msp430/msp430.md                   | 381 +++++-------------
>  gcc/config/msp430/msp430.opt                  |   6 +
>  gcc/config/msp430/predicates.md               |  13 +-
>  gcc/doc/invoke.texi                           |  15 +-
>  .../gcc.target/msp430/emulate-srli.c          |   2 +-
>  .../msp430/max-inline-shift-430-no-opt.c      |  52 +++
>  .../gcc.target/msp430/max-inline-shift-430.c  |  50 +++
>  .../gcc.target/msp430/max-inline-shift-430x.c |  48 +++
>  libgcc/config/msp430/slli.S                   |  15 +
>  libgcc/config/msp430/srai.S                   |  15 +
>  libgcc/config/msp430/srli.S                   |  16 +
>  14 files changed, 527 insertions(+), 374 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/msp430/max-inline-shift-430-no-opt.c
>  create mode 100644 gcc/testsuite/gcc.target/msp430/max-inline-shift-430.c
>  create mode 100644 gcc/testsuite/gcc.target/msp430/max-inline-shift-430x.c
> 
> diff --git a/gcc/config/msp430/constraints.md b/gcc/config/msp430/constraints.md
> index 14368f7be6d..b8f9674b2fe 100644
> --- a/gcc/config/msp430/constraints.md
> +++ b/gcc/config/msp430/constraints.md
> @@ -25,15 +25,16 @@ (define_register_constraint "R13" "R13_REGS"
>    "Register R13.")
>  
>  (define_constraint "K"
> -  "Integer constant 1."
> +  "Integer constant 1-19."
>    (and (match_code "const_int")
> -       (match_test "IN_RANGE (ival, 1, 1)")))
> +       (match_test "IN_RANGE (ival, 1, 19)")))
>  
>  (define_constraint "L"
>    "Integer constant -1^20..1^19."
>    (and (match_code "const_int")
>         (match_test "IN_RANGE (ival, HOST_WIDE_INT_M1U << 20, 1 << 19)")))
>  
> +;; Valid shift amount for RRUM, RRAM, RLAM, RRCM.
>  (define_constraint "M"
>    "Integer constant 1-4."
>    (and (match_code "const_int")
> @@ -49,6 +50,11 @@ (define_constraint "O"
>    (and (match_code "const_int")
>         (match_test "IN_RANGE (ival, 256, 65535)")))
>  
> +(define_constraint "P"
> +  "Integer constant 1-16."
> +  (and (match_code "const_int")
> +       (match_test "IN_RANGE (ival, 1, 16)")))
> +
>  ;; We do not allow arbitrary constants, eg symbols or labels,
>  ;; because their address may be above the 16-bit address limit
>  ;; supported by the offset used in the MOVA instruction.
> diff --git a/gcc/config/msp430/msp430-protos.h b/gcc/config/msp430/msp430-protos.h
> index a13a94cb92c..0b4d9a42b41 100644
> --- a/gcc/config/msp430/msp430-protos.h
> +++ b/gcc/config/msp430/msp430-protos.h
> @@ -35,7 +35,6 @@ rtx	msp430_incoming_return_addr_rtx (void);
>  void	msp430_init_cumulative_args (CUMULATIVE_ARGS *, tree, rtx, tree, int);
>  int	msp430_initial_elimination_offset (int, int);
>  bool    msp430_is_interrupt_func (void);
> -const char * msp430x_logical_shift_right (rtx);
>  const char * msp430_mcu_name (void);
>  void    msp430_output_aligned_decl_common (FILE *, const tree, const char *,
>  					   unsigned HOST_WIDE_INT, unsigned,
> @@ -51,4 +50,9 @@ bool    msp430_use_f5_series_hwmult (void);
>  bool	msp430_has_hwmult (void);
>  bool msp430_op_not_in_high_mem (rtx op);
>  
> +#ifdef RTX_CODE
> +int msp430_expand_shift (enum rtx_code code, machine_mode mode, rtx *operands);
> +const char * msp430_output_asm_shift_insns (enum rtx_code code, machine_mode mode, rtx *operands);
> +#endif
> +
>  #endif /* GCC_MSP430_PROTOS_H */
> diff --git a/gcc/config/msp430/msp430.c b/gcc/config/msp430/msp430.c
> index 455b4af3dad..c2b24974364 100644
> --- a/gcc/config/msp430/msp430.c
> +++ b/gcc/config/msp430/msp430.c
> @@ -1064,15 +1064,6 @@ static bool msp430_rtx_costs (rtx	   x ATTRIBUTE_UNUSED,
>  	  return true;
>  	}
>        break;
> -    case ASHIFT:
> -    case ASHIFTRT:
> -    case LSHIFTRT:
> -      if (!msp430x)
> -	{
> -	  *total = COSTS_N_INSNS (100);
> -	  return true;
> -	}
> -      break;
>      }
>    return false;
>  }
> @@ -2674,32 +2665,6 @@ msp430_init_dwarf_reg_sizes_extra (tree address)
>      }
>  }
>  
> -/* This is a list of MD patterns that implement fixed-count shifts.  */
> -static struct
> -{
> -  const char *name;
> -  int count;
> -  int need_430x;
> -  rtx (*genfunc)(rtx,rtx);
> -}
> -const_shift_helpers[] =
> -{
> -#define CSH(N,C,X,G) { "__mspabi_" N, C, X, gen_##G }
> -
> -  CSH ("slli", 1, 1, slli_1),
> -  CSH ("slll", 1, 1, slll_1),
> -  CSH ("slll", 2, 1, slll_2),
> -
> -  CSH ("srai", 1, 0, srai_1),
> -  CSH ("sral", 1, 0, sral_1),
> -  CSH ("sral", 2, 0, sral_2),
> -
> -  CSH ("srll", 1, 0, srll_1),
> -  CSH ("srll", 2, 1, srll_2x),
> -  { 0, 0, 0, 0 }
> -#undef CSH
> -};
> -
>  /* The MSP430 ABI defines a number of helper functions that should be
>     used for, for example, 32-bit shifts.  This function is called to
>     emit such a function, using the table above to optimize some
> @@ -2716,31 +2681,12 @@ msp430_expand_helper (rtx *operands, const char *helper_name,
>    machine_mode arg0mode = GET_MODE (operands[0]);
>    machine_mode arg1mode = GET_MODE (operands[1]);
>    machine_mode arg2mode = GET_MODE (operands[2]);
> -  int have_430x = msp430x ? 1 : 0;
>    int expand_mpy = strncmp (helper_name, "__mspabi_mpy",
>  			    sizeof ("__mspabi_mpy") - 1) == 0;
>    /* This function has been used incorrectly if CONST_VARIANTS is TRUE for a
>       hwmpy function.  */
>    gcc_assert (!(expand_mpy && const_variants));
>  
> -  /* Emit size-optimal insns for small shifts we can easily do inline.  */
> -  if (CONST_INT_P (operands[2]) && !expand_mpy)
> -    {
> -      int i;
> -
> -      for (i=0; const_shift_helpers[i].name; i++)
> -	{
> -	  if (const_shift_helpers[i].need_430x <= have_430x
> -	      && strcmp (helper_name, const_shift_helpers[i].name) == 0
> -	      && INTVAL (operands[2]) == const_shift_helpers[i].count)
> -	    {
> -	      emit_insn (const_shift_helpers[i].genfunc (operands[0],
> -							 operands[1]));
> -	      return;
> -	    }
> -	}
> -    }
> -
>    if (arg1mode != VOIDmode && arg2mode != VOIDmode)
>      /* Modes of arguments must be equal if not constants.  */
>      gcc_assert (arg1mode == arg2mode);
> @@ -2835,6 +2781,190 @@ msp430_expand_helper (rtx *operands, const char *helper_name,
>  		  gen_rtx_REG (arg0mode, 12));
>  }
>  
> +/* Return TRUE if the helper function should be used and FALSE if the shifts
> +   insns should be emitted inline.  */
> +static bool
> +use_helper_for_const_shift (enum rtx_code code, machine_mode mode,
> +			    HOST_WIDE_INT amt)
> +{
> +  const int default_inline_shift = 4;
> +  /* We initialize the option to 65 so we know if the user set it or not.  */
> +  int user_set_max_inline = (msp430_max_inline_shift == 65 ? 0 : 1);
> +  int max_inline = (user_set_max_inline ? msp430_max_inline_shift
> +		    : default_inline_shift);
> +  /* 32-bit shifts are roughly twice as costly as 16-bit shifts so we adjust
> +     the heuristic accordingly.  */
> +  int max_inline_32 = max_inline / 2;
> +
> +  /* Don't use helpers for these modes on 430X, when optimizing for speed, or
> +     when emitting a small number of insns.  */
> +  if ((mode == E_QImode || mode == E_HImode || mode == E_PSImode)
> +      && (msp430x
> +	  /* If the user set max_inline then we always obey that number.
> +	     Otherwise we always emit the shifts inline at -O2 and above.  */
> +	  || amt <= max_inline
> +	  || (!user_set_max_inline
> +	      && (optimize >= 2 && !optimize_size))))
> +    return false;
> +
> +  /* 430 and 430X codegen for SImode shifts is the same.
> +     Set a hard limit of 15 for the number of shifts that will be emitted
> +     inline by default, even at -O2 and above, to prevent code size
> +     explosion.  */
> +  if (mode == E_SImode
> +      && (amt <= max_inline_32
> +	  || (!user_set_max_inline
> +	      && (optimize >= 2 && !optimize_size)
> +	      && amt <= 15)))
> +    return false;
> +
> +  return true;
> +}
> +
> +/* For shift operations which will use an mspabi helper function, setup the
> +   call to msp430_expand helper.  Return 1 to indicate we have finished with
> +   this insn and invoke "DONE".
> +   Otherwise return 0 to indicate the insn should fallthrough.
> +   Never FAIL.  */
> +int
> +msp430_expand_shift (enum rtx_code code, machine_mode mode, rtx *operands)
> +{
> +  /* Always use the helper function when the shift amount is not a
> +     constant.  */
> +  if (!CONST_INT_P (operands[2])
> +      || mode == E_DImode
> +      || use_helper_for_const_shift (code, mode, INTVAL (operands[2])))
> +    {
> +      const char *helper_name = NULL;
> +      /* The const variants of mspabi shifts have significantly larger code
> +	 size than the generic version, so use the generic version if
> +	 optimizing for size.  */
> +      bool const_variant = !optimize_size;
> +      switch (mode)
> +	{
> +	case E_HImode:
> +	  helper_name = (code == ASHIFT ? "__mspabi_slli" :
> +			 (code == ASHIFTRT ? "__mspabi_srai" :
> +			  (code == LSHIFTRT ? "__mspabi_srli" :
> +			   NULL)));
> +	  break;
> +	case E_PSImode:
> +	  helper_name = (code == ASHIFT ? "__gnu_mspabi_sllp" :
> +			 (code == ASHIFTRT ? "__gnu_mspabi_srap" :
> +			  (code == LSHIFTRT ? "__gnu_mspabi_srlp" :
> +			   NULL)));
> +	  /* No const variant for PSImode shifts FIXME.  */
> +	  const_variant = false;
> +	  break;
> +	case E_SImode:
> +	  helper_name = (code == ASHIFT ? "__mspabi_slll" :
> +			 (code == ASHIFTRT ? "__mspabi_sral" :
> +			  (code == LSHIFTRT ? "__mspabi_srll" :
> +			   NULL)));
> +	  break;
> +	case E_DImode:
> +	  helper_name = (code == ASHIFT ? "__mspabi_sllll" :
> +			 (code == ASHIFTRT ? "__mspabi_srall" :
> +			  (code == LSHIFTRT ? "__mspabi_srlll" :
> +			   NULL)));
> +	  /* No const variant for DImode shifts.  */
> +	  const_variant = false;
> +	  break;
> +	default:
> +	  gcc_unreachable ();
> +	  break;
> +	}
> +      gcc_assert (helper_name);
> +      msp430_expand_helper (operands, helper_name, const_variant);
> +      return 1;
> +    }
> +  /* When returning 0, there must be an insn to match the RTL pattern
> +     otherwise there will be an unrecognizeable insn.  */
> +  return 0;
> +}
> +
> +/* Helper function to emit a sequence of shift instructions.  The amount of
> +   shift instructions to emit is in OPERANDS[2].
> +   For 430 we output copies of identical inline shifts for all modes.
> +   For 430X it is inneficient to do so for any modes except SI and DI, since we
> +   can make use of R*M insns or RPT with 430X insns, so this function is only
> +   used for SImode in that case.  */
> +const char *
> +msp430_output_asm_shift_insns (enum rtx_code code, machine_mode mode,
> +			       rtx *operands)
> +{
> +  int i;
> +  int amt;
> +  int max_shift = GET_MODE_BITSIZE (mode) - 1;
> +  gcc_assert (CONST_INT_P (operands[2]));
> +  amt = INTVAL (operands[2]);
> +
> +  if (amt == 0 || amt > max_shift)
> +    {
> +      switch (code)
> +	{
> +	case ASHIFT:
> +	  output_asm_insn ("# ignored undefined behaviour left shift "
> +			   "of %1 by %2", operands);
> +	  break;
> +	case ASHIFTRT:
> +	  output_asm_insn ("# ignored undefined behaviour arithmetic right "
> +			   "shift of %1 by %2", operands);
> +	  break;
> +	case LSHIFTRT:
> +	  output_asm_insn ("# ignored undefined behaviour logical right shift "
> +			   "of %1 by %2", operands);
> +	  break;
> +	default:
> +	  gcc_unreachable ();
> +	}
> +      return "";
> +    }
> +
> +  if (code == ASHIFT)
> +    {
> +      if (!msp430x && mode == HImode)
> +	for (i = 0; i < amt; i++)
> +	  output_asm_insn ("RLA.W\t%0", operands);
> +      else if (mode == SImode)
> +	for (i = 0; i < amt; i++)
> +	  output_asm_insn ("RLA%X0.W\t%L0 { RLC%X0.W\t%H0", operands);
> +      else
> +	/* Catch unhandled cases.  */
> +	gcc_unreachable ();
> +    }
> +  else if (code == ASHIFTRT)
> +    {
> +      if (!msp430x && mode == HImode)
> +	for (i = 0; i < amt; i++)
> +	  output_asm_insn ("RRA.W\t%0", operands);
> +      else if (mode == SImode)
> +	for (i = 0; i < amt; i++)
> +	  output_asm_insn ("RRA%X0.W\t%H0 { RRC%X0.W\t%L0", operands);
> +      else
> +	gcc_unreachable ();
> +    }
> +  else if (code == LSHIFTRT)
> +    {
> +      if (!msp430x && mode == HImode)
> +	for (i = 0; i < amt; i++)
> +	  output_asm_insn ("CLRC { RRC.W\t%0", operands);
> +      else if (mode == SImode)
> +	for (i = 0; i < amt; i++)
> +	  output_asm_insn ("CLRC { RRC%X0.W\t%H0 { RRC%X0.W\t%L0", operands);
> +      /* FIXME: Why doesn't "RRUX.W\t%H0 { RRC%X0.W\t%L0" work for msp430x?
> +	 It causes execution timeouts e.g. pr41963.c.  */
> +#if 0
> +      else if (msp430x && mode == SImode)
> +	for (i = 0; i < amt; i++)
> +	  output_asm_insn ("RRUX.W\t%H0 { RRC%X0.W\t%L0", operands);
> +#endif
> +      else
> +	gcc_unreachable ();
> +    }
> +  return "";
> +}
> +
>  /* Called by cbranch<mode>4 to coerce operands into usable forms.  */
>  void
>  msp430_fixup_compare_operands (machine_mode my_mode, rtx * operands)
> @@ -3368,6 +3498,7 @@ msp430_op_not_in_high_mem (rtx op)
>     O   offset of the top of the stack
>     Q   like X but generates an A postfix
>     R   inverse of condition code, unsigned.
> +   W   value - 16
>     X   X instruction postfix in large mode
>     Y   value - 4
>     Z   value - 1
> @@ -3394,6 +3525,11 @@ msp430_print_operand (FILE * file, rtx op, int letter)
>        /* Print the constant value, less four.  */
>        fprintf (file, "#%ld", INTVAL (op) - 4);
>        return;
> +    case 'W':
> +      gcc_assert (CONST_INT_P (op));
> +      /* Print the constant value, less 16.  */
> +      fprintf (file, "#%ld", INTVAL (op) - 16);
> +      return;
>      case 'I':
>        if (GET_CODE (op) == CONST_INT)
>  	{
> @@ -3711,34 +3847,6 @@ msp430x_extendhisi (rtx * operands)
>    return "MOV.W\t%1, %L0 { MOV.W\t%1, %H0 { RPT\t#15 { RRAX.W\t%H0";
>  }
>  
> -/* Likewise for logical right shifts.  */
> -const char *
> -msp430x_logical_shift_right (rtx amount)
> -{
> -  /* The MSP430X's logical right shift instruction - RRUM - does
> -     not use an extension word, so we cannot encode a repeat count.
> -     Try various alternatives to work around this.  If the count
> -     is in a register we are stuck, hence the assert.  */
> -  gcc_assert (CONST_INT_P (amount));
> -
> -  if (INTVAL (amount) <= 0
> -      || INTVAL (amount) >= 16)
> -    return "# nop logical shift.";
> -
> -  if (INTVAL (amount) > 0
> -      && INTVAL (amount) < 5)
> -    return "rrum.w\t%2, %0"; /* Two bytes.  */
> -
> -  if (INTVAL (amount) > 4
> -      && INTVAL (amount) < 9)
> -    return "rrum.w\t#4, %0 { rrum.w\t%Y2, %0 "; /* Four bytes.  */
> -
> -  /* First we logically shift right by one.  Now we know
> -     that the top bit is zero and we can use the arithmetic
> -     right shift instruction to perform the rest of the shift.  */
> -  return "rrum.w\t#1, %0 { rpt\t%Z2 { rrax.w\t%0"; /* Six bytes.  */
> -}
> -
>  /* Stop GCC from thinking that it can eliminate (SUBREG:PSI (SI)).  */
>  
>  #undef TARGET_CAN_CHANGE_MODE_CLASS
> diff --git a/gcc/config/msp430/msp430.md b/gcc/config/msp430/msp430.md
> index ed21eb02868..f70e61b97dd 100644
> --- a/gcc/config/msp430/msp430.md
> +++ b/gcc/config/msp430/msp430.md
> @@ -65,6 +65,15 @@ (define_attr "length" "" (const_int 4))
>  (include "constraints.md")
>  
>  (define_mode_iterator QHI [QI HI PSI])
> +(define_mode_iterator HPSI [HI PSI])
> +(define_mode_iterator HDI [HI PSI SI DI])
> +
> +;; Mapping of all shift operators
> +(define_code_iterator any_shift [ashift ashiftrt lshiftrt])
> +
> +;; Base name for define_insn
> +(define_code_attr shift_insn
> +  [(ashift "ashl") (lshiftrt "lshr") (ashiftrt "ashr")])
>  
>  ;; There are two basic "family" tests we do here:
>  ;;
> @@ -689,31 +698,42 @@ (define_insn ""
>     MOV%X1.B\t%1, %0"
>  )
>  
> +;; The next three insns emit identical assembly code.
> +;; They take a QImode and shift it in SImode.  Only shift counts <= 8
> +;; are handled since that is the simple case where the high 16-bits (i.e. the
> +;; high register) are always 0.
>  (define_insn ""
> -  [(set (match_operand:SI 0 "register_operand" "=r")
> -	(ashift:SI (zero_extend:SI (match_operand:QI 1 "general_operand" "rm"))
> -		   (match_operand:HI 2 "immediate_operand" "M")))]
> +  [(set (match_operand:SI			     0 "register_operand" "=r,r,r")
> +	(ashift:SI (zero_extend:SI (match_operand:QI 1 "general_operand" "0,rm,rm"))
> +		   (match_operand:HI		     2 "const_1_to_8_operand" "M,M,i")))]
>    "msp430x"
> -  "MOV%X1.B %1, %L0 { RLAM.W %2, %L0 { CLR %H0"
> +  "@
> +  RLAM.W %2, %L0 { CLR %H0
> +  MOV%X1.B %1, %L0 { RLAM.W %2, %L0 { CLR %H0
> +  MOV%X1.B %1, %L0 { RPT %2 { RLAX.W %L0 { CLR %H0"
>  )
>  
> -;; We are taking a char and shifting it and putting the result in 2 registers.
> -;; the high register will always be for 0 shift counts < 8.
>  (define_insn ""
> -  [(set (match_operand:SI 0 "register_operand" "=r")
> -	(ashift:SI (zero_extend:SI (subreg:HI (match_operand:QI 1 "general_operand" "rm") 0))
> -		   (match_operand:HI 2 "immediate_operand" "M")))]
> +  [(set (match_operand:SI			     		0 "register_operand" "=r,r,r")
> +	(ashift:SI (zero_extend:SI (subreg:HI (match_operand:QI 1 "general_operand" "0,rm,rm") 0))
> +		   (match_operand:HI		     		2 "const_1_to_8_operand" "M,M,i")))]
>    "msp430x"
> -  "MOV%X1.B %1, %L0 { RLAM.W %2, %L0 { CLR %H0"
> +  "@
> +  RLAM.W %2, %L0 { CLR %H0
> +  MOV%X1.B %1, %L0 { RLAM.W %2, %L0 { CLR %H0
> +  MOV%X1.B %1, %L0 { RPT %2 { RLAX.W %L0 { CLR %H0"
>  )
>  
>  ;; Same as above but with a NOP sign_extend round the subreg
>  (define_insn ""
> -  [(set (match_operand:SI 0 "register_operand" "=r")
> -	(ashift:SI (zero_extend:SI (sign_extend:PSI (subreg:HI (match_operand:QI 1 "general_operand" "rm") 0)))
> -		   (match_operand:HI 2 "immediate_operand" "M")))]
> +  [(set (match_operand:SI			     				 0 "register_operand" "=r,r,r")
> +	(ashift:SI (zero_extend:SI (sign_extend:PSI (subreg:HI (match_operand:QI 1 "general_operand" "0,rm,rm") 0)))
> +		   (match_operand:HI		     				 2 "const_1_to_8_operand" "M,M,i")))]
>    "msp430x"
> -  "MOV%X1.B %1, %L0 { RLAM.W %2, %L0 { CLR %H0"
> +  "@
> +  RLAM.W %2, %L0 { CLR %H0
> +  MOV%X1.B %1, %L0 { RLAM.W %2, %L0 { CLR %H0
> +  MOV%X1.B %1, %L0 { RPT %2 { RLAX.W %L0 { CLR %H0"
>  )
>  
>  (define_insn ""
> @@ -724,11 +744,14 @@ (define_insn ""
>  )
>  
>  (define_insn ""
> -  [(set (match_operand:PSI 0 "register_operand" "=r")
> -	(ashift:PSI (sign_extend:PSI (subreg:HI (match_operand:QI 1 "general_operand" "rm") 0))
> -		    (match_operand:HI 2 "immediate_operand" "M")))]
> +  [(set (match_operand:PSI					  0 "register_operand" "=r,r,r")
> +	(ashift:PSI (sign_extend:PSI (subreg:HI (match_operand:QI 1 "general_operand" "0,rm,rm") 0))
> +		    (match_operand:HI				  2 "const_1_to_19_operand" "M,M,i")))]
>    "msp430x"
> -  "MOV%X1.B %1, %0 { RLAM.W %2, %0"
> +  "@
> +  RLAM.W %2, %0
> +  MOV%X1.B %1, %0 { RLAM.W %2, %0
> +  MOV%X1.B %1, %0 { RPT %2 { RLAX.A %0"
>  )
>  ;; END msp430 pointer manipulation combine insn patterns
>  
> @@ -840,287 +863,75 @@ (define_insn "truncsipsi2"
>  ;; Note - we ignore shift counts of less than one or more than 15.
>  ;; This is permitted by the ISO C99 standard as such shifts result
>  ;; in "undefined" behavior.  [6.5.7 (3)]
> +;;
> +;; We avoid emitting insns in msp430_expand_shift, since we would have to handle
> +;; many extra cases such as op0 != op1, or, op0 or op1 in memory.  Instead we
> +;; let reload coerce op0 and op1 into the same register.
>  
> -;; signed A << C
> -
> -(define_expand "ashlhi3"
> -  [(set (match_operand:HI	     0 "msp430_general_dst_nonv_operand")
> -	(ashift:HI (match_operand:HI 1 "general_operand")
> -		   (match_operand:HI 2 "general_operand")))]
> +(define_expand "<shift_insn><mode>3"
> +  [(set (match_operand:HDI		  0 "msp430_general_dst_nonv_operand")
> +	(any_shift:HDI (match_operand:HDI 1 "general_operand")
> +		       (match_operand:HDI 2 "general_operand")))]
>    ""
>    {
> -    if ((GET_CODE (operands[1]) == SUBREG
> -	 && REG_P (XEXP (operands[1], 0)))
> -	|| MEM_P (operands[1]))
> -      operands[1] = force_reg (HImode, operands[1]);
> -    if (msp430x
> -        && REG_P (operands[0])
> -        && REG_P (operands[1])
> -        && CONST_INT_P (operands[2]))
> -      emit_insn (gen_430x_shift_left (operands[0], operands[1], operands[2]));
> -    else if (CONST_INT_P (operands[2])
> -	     && INTVAL (operands[2]) == 1)
> -      emit_insn (gen_slli_1 (operands[0], operands[1]));
> -    else		 
> -      /* The const variants of mspabi shifts have larger code size than the
> -	 generic version, so use the generic version if optimizing for
> -	 size.  */
> -      msp430_expand_helper (operands, \"__mspabi_slli\", !optimize_size);
> -    DONE;
> +    if (msp430_expand_shift (<CODE>, <MODE>mode, operands))
> +      DONE;
> +    /* Otherwise, fallthrough.  */
>    }
>  )
>  
> -(define_insn "slli_1"
> -  [(set (match_operand:HI	     0 "msp430_general_dst_nonv_operand" "=rm")
> -	(ashift:HI (match_operand:HI 1 "general_operand"       "0")
> -		   (const_int 1)))]
> -  ""
> -  "RLA%X0.W\t%0" ;; Note - this is a macro for ADD
> -)
> -
> -(define_insn "430x_shift_left"
> -  [(set (match_operand:HI            0 "register_operand" "=r")
> -	(ashift:HI (match_operand:HI 1 "register_operand"  "0")
> -		   (match_operand    2 "immediate_operand" "n")))]
> -  "msp430x"
> -  "*
> -  if (INTVAL (operands[2]) > 0 && INTVAL (operands[2]) < 5)
> -    return \"RLAM.W\t%2, %0\";
> -  else if (INTVAL (operands[2]) >= 5 && INTVAL (operands[2]) < 16)
> -    return \"RPT\t%2 { RLAX.W\t%0\";
> -  return \"# nop left shift\";
> -  "
> +;; All 430 HImode constant shifts
> +(define_insn "<shift_insn>hi3_430"
> +  [(set (match_operand:HI		0 "msp430_general_dst_nonv_operand" "=rm")
> +	(any_shift:HI (match_operand:HI 1 "general_operand"       "0")
> +		      (match_operand:HI 2 "const_int_operand"     "n")))]
> +  "!msp430x"
> +  "* return msp430_output_asm_shift_insns (<CODE>, HImode, operands);"
>  )
>  
> -(define_insn "slll_1"
> -  [(set (match_operand:SI	     0 "msp430_general_dst_nonv_operand" "=rm")
> -	(ashift:SI (match_operand:SI 1 "general_operand"       "0")
> -		   (const_int 1)))]
> +;; All 430 and 430X SImode constant shifts
> +(define_insn "<shift_insn>si3_const"
> +  [(set (match_operand:SI		0 "msp430_general_dst_nonv_operand" "=rm")
> +	(any_shift:SI (match_operand:SI 1 "general_operand"       "0")
> +		      (match_operand:SI 2 "const_int_operand"     "n")))]
>    ""
> -  "RLA%X0.W\t%L0 { RLC%X0.W\t%H0"
> -)
> -
> -(define_insn "slll_2"
> -  [(set (match_operand:SI	     0 "msp430_general_dst_nonv_operand" "=rm")
> -	(ashift:SI (match_operand:SI 1 "general_operand"       "0")
> -		   (const_int 2)))]
> -  ""
> -  "RLA%X0.W\t%L0 { RLC%X0.W\t%H0 { RLA%X0.W\t%L0 { RLC%X0.W\t%H0"
> -)
> -
> -(define_expand "ashlsi3"
> -  [(set (match_operand:SI	     0 "msp430_general_dst_nonv_operand")
> -	(ashift:SI (match_operand:SI 1 "general_operand")
> -		   (match_operand:SI 2 "general_operand")))]
> -  ""
> -  "msp430_expand_helper (operands, \"__mspabi_slll\", !optimize_size);
> -   DONE;"
> -)
> -
> -(define_expand "ashldi3"
> -  [(set (match_operand:DI	     0 "msp430_general_dst_nonv_operand")
> -	(ashift:DI (match_operand:DI 1 "general_operand")
> -		   (match_operand:DI 2 "general_operand")))]
> -  ""
> -  {
> -    /* No const_variant for 64-bit shifts.  */
> -    msp430_expand_helper (operands, \"__mspabi_sllll\", false);
> -    DONE;
> -  }
> -)
> -
> -;;----------
> -
> -;; signed A >> C
> -
> -(define_expand "ashrhi3"
> -  [(set (match_operand:HI	       0 "msp430_general_dst_nonv_operand")
> -	(ashiftrt:HI (match_operand:HI 1 "general_operand")
> -		     (match_operand:HI 2 "general_operand")))]
> -  ""
> -  {
> -    if ((GET_CODE (operands[1]) == SUBREG
> -	 && REG_P (XEXP (operands[1], 0)))
> -	|| MEM_P (operands[1]))
> -      operands[1] = force_reg (HImode, operands[1]);
> -    if (msp430x
> -        && REG_P (operands[0])
> -        && REG_P (operands[1])
> -        && CONST_INT_P (operands[2]))
> -      emit_insn (gen_430x_arithmetic_shift_right (operands[0], operands[1], operands[2]));
> -    else if (CONST_INT_P (operands[2])
> -	     && INTVAL (operands[2]) == 1)
> -      emit_insn (gen_srai_1 (operands[0], operands[1]));
> -    else		 
> -       msp430_expand_helper (operands, \"__mspabi_srai\", !optimize_size);
> -   DONE;
> -   }
> -)
> -
> -(define_insn "srai_1"
> -  [(set (match_operand:HI	       0 "msp430_general_dst_operand" "=rm")
> -	(ashiftrt:HI (match_operand:HI 1 "msp430_general_operand"      "0")
> -		     (const_int 1)))]
> -  ""
> -  "RRA%X0.W\t%0"
> -)
> -
> -(define_insn "430x_arithmetic_shift_right"
> -  [(set (match_operand:HI              0 "register_operand" "=r")
> -	(ashiftrt:HI (match_operand:HI 1 "register_operand"  "0")
> -		     (match_operand    2 "immediate_operand" "n")))]
> -  "msp430x"
> -  "*
> -  if (INTVAL (operands[2]) > 0 && INTVAL (operands[2]) < 5)
> -    return \"RRAM.W\t%2, %0\";
> -  else if (INTVAL (operands[2]) >= 5 && INTVAL (operands[2]) < 16)
> -    return \"RPT\t%2 { RRAX.W\t%0\";
> -  return \"# nop arith right shift\";
> -  "
> -)
> -
> -(define_insn "srap_1"
> -  [(set (match_operand:PSI              0 "register_operand" "=r")
> -	(ashiftrt:PSI (match_operand:PSI 1 "general_operand" "0")
> -		      (const_int 1)))]
> -  "msp430x"
> -  "RRAM.A #1,%0"
> +  "* return msp430_output_asm_shift_insns (<CODE>, SImode, operands);"
>  )
>  
> -(define_insn "srap_2"
> -  [(set (match_operand:PSI              0 "register_operand" "=r")
> -	(ashiftrt:PSI (match_operand:PSI 1 "general_operand" "0")
> -		      (const_int 2)))]
> +(define_insn "ashl<mode>3_430x"
> +  [(set (match_operand:HPSI		 0 "msp430_general_dst_nonv_operand" "=r,r,r,r")
> +	(ashift:HPSI (match_operand:HPSI 1 "general_operand" 		     "0 ,0,0,0")
> +		     (match_operand:HPSI 2 "const_int_operand" 		     "M ,P,K,i")))]
>    "msp430x"
> -  "RRAM.A #2,%0"
> -)
> -
> -(define_insn "sral_1"
> -  [(set (match_operand:SI	       0 "msp430_general_dst_nonv_operand" "=rm")
> -	(ashiftrt:SI (match_operand:SI 1 "general_operand"       "0")
> -		     (const_int 1)))]
> -  ""
> -  "RRA%X0.W\t%H0 { RRC%X0.W\t%L0"
> -)
> -
> -(define_insn "sral_2"
> -  [(set (match_operand:SI	       0 "msp430_general_dst_nonv_operand" "=rm")
> -	(ashiftrt:SI (match_operand:SI 1 "general_operand"       "0")
> -		     (const_int 2)))]
> -  ""
> -  "RRA%X0.W\t%H0 { RRC%X0.W\t%L0 { RRA%X0.W\t%H0 { RRC%X0.W\t%L0"
> -)
> -
> -(define_expand "ashrsi3"
> -  [(set (match_operand:SI	       0 "msp430_general_dst_nonv_operand")
> -	(ashiftrt:SI (match_operand:SI 1 "general_operand")
> -		     (match_operand:SI 2 "general_operand")))]
> -  ""
> -  "msp430_expand_helper (operands, \"__mspabi_sral\", !optimize_size);
> -   DONE;"
> -)
> -
> -(define_expand "ashrdi3"
> -  [(set (match_operand:DI	     0 "msp430_general_dst_nonv_operand")
> -	(ashift:DI (match_operand:DI 1 "general_operand")
> -		   (match_operand:DI 2 "general_operand")))]
> -  ""
> -  {
> -    /* No const_variant for 64-bit shifts.  */
> -    msp430_expand_helper (operands, \"__mspabi_srall\", false);
> -    DONE;
> -  }
> -)
> -
> -;;----------
> -
> -;; unsigned A >> C
> -
> -(define_expand "lshrhi3"
> -  [(set (match_operand:HI	       0 "msp430_general_dst_nonv_operand")
> -	(lshiftrt:HI (match_operand:HI 1 "general_operand")
> -		     (match_operand:HI 2 "general_operand")))]
> -  ""
> -  {
> -    if ((GET_CODE (operands[1]) == SUBREG
> -	 && REG_P (XEXP (operands[1], 0)))
> -	|| MEM_P (operands[1]))
> -      operands[1] = force_reg (HImode, operands[1]);
> -    if (msp430x
> -        && REG_P (operands[0])
> -        && REG_P (operands[1])
> -        && CONST_INT_P (operands[2]))
> -      emit_insn (gen_430x_logical_shift_right (operands[0], operands[1], operands[2]));
> -    else if (CONST_INT_P (operands[2])
> -	     && INTVAL (operands[2]) == 1)
> -      emit_insn (gen_srli_1 (operands[0], operands[1]));
> -    else		 
> -      msp430_expand_helper (operands, \"__mspabi_srli\", !optimize_size);
> -    DONE;
> -  }
> -)
> -
> -(define_insn "srli_1"
> -  [(set (match_operand:HI	       0 "msp430_general_dst_nonv_operand" "=rm")
> -	(lshiftrt:HI (match_operand:HI 1 "general_operand"       "0")
> -		     (const_int 1)))]
> -  ""
> -  "CLRC { RRC%X0.W\t%0"
> +  "@
> +  RLAM%b0\t%2, %0
> +  RPT\t%2 { RLAX%b0\t%0
> +  RPT\t#16 { RLAX%b0\t%0 { RPT\t%W2 { RLAX%b0\t%0
> +  # undefined behavior left shift of %1 by %2"
>  )
>  
> -(define_insn "430x_logical_shift_right"
> -  [(set (match_operand:HI              0 "register_operand" "=r")
> -	(lshiftrt:HI (match_operand:HI 1 "register_operand"  "0")
> -		     (match_operand    2 "immediate_operand" "n")))]
> +(define_insn "ashr<mode>3_430x"
> +  [(set (match_operand:HPSI		   0 "msp430_general_dst_nonv_operand" "=r,r,r,r")
> +	(ashiftrt:HPSI (match_operand:HPSI 1 "general_operand"	  	     "0,0,0,0")
> +		       (match_operand:HPSI 2 "const_int_operand" 	     "M,P,K,i")))]
>    "msp430x"
> -  {
> -    return msp430x_logical_shift_right (operands[2]);
> -  }
> -)
> -
> -(define_insn "srlp_1"
> -  [(set (match_operand:PSI              0 "register_operand" "=r")
> -	(lshiftrt:PSI (match_operand:PSI 1 "general_operand" "0")
> -		      (const_int 1)))]
> -  ""
> -  "RRUM.A #1,%0"
> -)
> -
> -(define_insn "srll_1"
> -  [(set (match_operand:SI	       0 "msp430_general_dst_nonv_operand" "=rm")
> -	(lshiftrt:SI (match_operand:SI 1 "general_operand"       "0")
> -		     (const_int 1)))]
> -  ""
> -  "CLRC { RRC%X0.W\t%H0 { RRC%X0.W\t%L0"
> +  "@
> +  RRAM%b0\t%2, %0
> +  RPT\t%2 { RRAX%b0\t%0
> +  RPT\t#16 { RRAX%b0\t%0 { RPT\t%W2 { RRAX%b0\t%0
> +  # undefined behavior arithmetic right shift of %1 by %2"
>  )
>  
> -(define_insn "srll_2x"
> -  [(set (match_operand:SI	       0 "msp430_general_dst_nonv_operand" "=r")
> -	(lshiftrt:SI (match_operand:SI 1 "general_operand"       "0")
> -		     (const_int 2)))]
> +(define_insn "lshr<mode>3_430x"
> +  [(set (match_operand:HPSI		   0 "msp430_general_dst_nonv_operand" "=r,r,r,r")
> +	(lshiftrt:HPSI (match_operand:HPSI 1 "general_operand"	  	     "0,0,0,0")
> +		       (match_operand:HPSI 2 "const_int_operand" 	     "M,P,K,i")))]
>    "msp430x"
> -  "RRUX.W\t%H0 { RRC.W\t%L0 { RRUX.W\t%H0 { RRC.W\t%L0"
> -)
> -
> -(define_expand "lshrsi3"
> -  [(set (match_operand:SI	       0 "msp430_general_dst_nonv_operand")
> -	(lshiftrt:SI (match_operand:SI 1 "general_operand")
> -		     (match_operand:SI 2 "general_operand")))]
> -  ""
> -  "msp430_expand_helper (operands, \"__mspabi_srll\", !optimize_size);
> -   DONE;"
> -)
> -
> -(define_expand "lshrdi3"
> -  [(set (match_operand:DI	     0 "msp430_general_dst_nonv_operand")
> -	(ashift:DI (match_operand:DI 1 "general_operand")
> -		   (match_operand:DI 2 "general_operand")))]
> -  ""
> -  {
> -    /* No const_variant for 64-bit shifts.  */
> -    msp430_expand_helper (operands, \"__mspabi_srlll\", false);
> -    DONE;
> -  }
> +  "@
> +  RRUM%b0\t%2, %0
> +  RPT\t%2 { RRUX%b0\t%0
> +  RPT\t#16 { RRUX%b0\t%0 { RPT\t%W2 { RRUX%b0\t%0
> +  # undefined behavior logical right shift of %1 by %2"
>  )
>  
>  ;;------------------------------------------------------------
> @@ -1427,7 +1238,7 @@ (define_insn "*bitbranch<mode>4_z"
>    [(set (pc) (if_then_else
>  	      (ne (zero_extract:HI (match_operand:QHI 0 "msp430_general_dst_operand" "rYs,rm")
>  				    (const_int 1)
> -				    (match_operand 1 "msp430_bitpos" "i,i"))
> +				    (match_operand 1 "const_0_to_15_operand" "i,i"))
>  		  (const_int 0))
>                (label_ref (match_operand 2 "" ""))
>  	      (pc)))
> @@ -1443,7 +1254,7 @@ (define_insn "*bitbranch<mode>4_z"
>    [(set (pc) (if_then_else
>  	      (eq (zero_extract:HI (match_operand:QHI 0 "msp430_general_dst_operand" "rm")
>  				   (const_int 1)
> -				   (match_operand 1 "msp430_bitpos" "i"))
> +				   (match_operand 1 "const_0_to_15_operand" "i"))
>  		  (const_int 0))
>                (label_ref (match_operand 2 "" ""))
>  	      (pc)))
> @@ -1457,7 +1268,7 @@ (define_insn "*bitbranch<mode>4_z"
>    [(set (pc) (if_then_else
>  	      (eq (zero_extract:HI (match_operand:QHI 0 "msp430_general_dst_operand" "rm")
>  				   (const_int 1)
> -				   (match_operand 1 "msp430_bitpos" "i"))
> +				   (match_operand 1 "const_0_to_15_operand" "i"))
>  		  (const_int 0))
>                (pc)
>  	      (label_ref (match_operand 2 "" ""))))
> @@ -1471,7 +1282,7 @@ (define_insn "*bitbranch<mode>4_z"
>    [(set (pc) (if_then_else
>  	      (ne (zero_extract:HI (match_operand:QHI 0 "msp430_general_dst_operand" "rm")
>  				   (const_int 1)
> -				   (match_operand 1 "msp430_bitpos" "i"))
> +				   (match_operand 1 "const_0_to_15_operand" "i"))
>  		  (const_int 0))
>                (pc)
>  	      (label_ref (match_operand 2 "" ""))))
> diff --git a/gcc/config/msp430/msp430.opt b/gcc/config/msp430/msp430.opt
> index b451174c3d1..8134ca7ac95 100644
> --- a/gcc/config/msp430/msp430.opt
> +++ b/gcc/config/msp430/msp430.opt
> @@ -109,3 +109,9 @@ mdevices-csv-loc=
>  Target Joined Var(msp430_devices_csv_loc) RejectNegative Report
>  The path to devices.csv.  The GCC driver can normally locate devices.csv itself
>  and pass this option to the compiler, so the user shouldn't need to pass this.
> +
> +mmax-inline-shift=
> +Target RejectNegative Joined UInteger IntegerRange(0,65) Var(msp430_max_inline_shift) Init(65) Report
> +For shift operations by a constant amount, which require an individual instruction to shift by one
> +position, set the maximum number of inline shift instructions (maximum value 64) to emit instead of using the corresponding __mspabi helper function.
> +The default value is 4.
> diff --git a/gcc/config/msp430/predicates.md b/gcc/config/msp430/predicates.md
> index 408d56f610a..4bfa0c0f2d5 100644
> --- a/gcc/config/msp430/predicates.md
> +++ b/gcc/config/msp430/predicates.md
> @@ -113,12 +113,21 @@ (define_predicate "msp430_nonsubregnonpostinc_or_imm_operand"
>         (ior (match_code "reg,mem")
>  	    (match_operand 0 "immediate_operand"))))
>  
> -; TRUE for constants which are bit positions for zero_extract
> -(define_predicate "msp430_bitpos"
> +(define_predicate "const_1_to_8_operand"
> +  (and (match_code "const_int")
> +       (match_test ("   INTVAL (op) >= 1
> +		     && INTVAL (op) <= 8 "))))
> +
> +(define_predicate "const_0_to_15_operand"
>    (and (match_code "const_int")
>         (match_test ("   INTVAL (op) >= 0
>  		     && INTVAL (op) <= 15 "))))
>  
> +(define_predicate "const_1_to_19_operand"
> +  (and (match_code "const_int")
> +       (match_test ("   INTVAL (op) >= 1
> +		     && INTVAL (op) <= 19 "))))
> +
>  (define_predicate "msp430_symbol_operand"
>    (match_code "symbol_ref")
>  )
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 09bcc5b0f78..885c7aae3a5 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -1066,7 +1066,7 @@ Objective-C and Objective-C++ Dialects}.
>  -mwarn-mcu @gol
>  -mcode-region=  -mdata-region= @gol
>  -msilicon-errata=  -msilicon-errata-warn= @gol
> --mhwmult=  -minrt  -mtiny-printf}
> +-mhwmult=  -minrt  -mtiny-printf  -mmax-inline-shift=}
>  
>  @emph{NDS32 Options}
>  @gccoptlist{-mbig-endian  -mlittle-endian @gol
> @@ -24728,6 +24728,19 @@ buffered before it is sent to write.
>  This option requires Newlib Nano IO, so GCC must be configured with
>  @samp{--enable-newlib-nano-formatted-io}.
>  
> +@item -mmax-inline-shift=
> +@opindex mmax-inline-shift=
> +This option takes an integer between 0 and 64 inclusive, and sets
> +the maximum number of inline shift instructions which should be emitted to
> +perform a shift operation by a constant amount.  When this value needs to be
> +exceeded, an mspabi helper function is used instead.  The default value is 4.
> +
> +This only affects cases where a shift by multiple positions cannot be
> +completed with a single instruction (e.g. all shifts >1 on the 430 ISA).
> +
> +Shifts of a 32-bit value are at least twice as costly, so the value passed for
> +this option is divided by 2 and the resulting value used instead.
> +
>  @item -mcode-region=
>  @itemx -mdata-region=
>  @opindex mcode-region
> diff --git a/gcc/testsuite/gcc.target/msp430/emulate-srli.c b/gcc/testsuite/gcc.target/msp430/emulate-srli.c
> index f870d13f86b..35207b7c458 100644
> --- a/gcc/testsuite/gcc.target/msp430/emulate-srli.c
> +++ b/gcc/testsuite/gcc.target/msp430/emulate-srli.c
> @@ -2,7 +2,7 @@
>  /* { dg-skip-if "" { *-*-* } { "-mcpu=msp430" } { "" } } */
>  /* { dg-options "-Os" } */
>  /* { dg-final { scan-assembler-not "mspabi_srli" } } */
> -/* { dg-final { scan-assembler "rrum" } } */
> +/* { dg-final { scan-assembler "RRUM" } } */
>  
>  /* Ensure that HImode shifts with source operand in memory are emulated with a
>     rotate instructions.  */
> diff --git a/gcc/testsuite/gcc.target/msp430/max-inline-shift-430-no-opt.c b/gcc/testsuite/gcc.target/msp430/max-inline-shift-430-no-opt.c
> new file mode 100644
> index 00000000000..c795f7570d7
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/msp430/max-inline-shift-430-no-opt.c
> @@ -0,0 +1,52 @@
> +/* { dg-do compile } */
> +/* { dg-skip-if "" { *-*-* } { "-mcpu=msp430x" "-mlarge" } { "" } } */
> +/* { dg-options "-mcpu=msp430" } */
> +/* { dg-final { scan-assembler-not "__mspabi_slli_4" } } */
> +/* { dg-final { scan-assembler-not "__mspabi_sral_2" } } */
> +/* { dg-final { scan-assembler "__mspabi_slli_5" } } */
> +/* { dg-final { scan-assembler "__mspabi_sral_3" } } */
> +
> +/* Test the default value of 4 for -mmax-inline-shift has been observed.  */
> +
> +volatile int a1, a2, a3, a4, a5, a6, a7, a8, a9, a10, a11, a12, a13, a14, a15;
> +volatile long l1, l2, l3, l4, l5, l6, l7, l8, l9, l10, l11, l12, l13, l14, l15;
> +
> +void
> +ashift (void)
> +{
> +  a1 <<= 1;
> +  a2 <<= 2;
> +  a3 <<= 3;
> +  a4 <<= 4;
> +  a5 <<= 5;
> +  a6 <<= 6;
> +  a7 <<= 7;
> +  a8 <<= 8;
> +  a9 <<= 9;
> +  a10 <<= 10;
> +  a11 <<= 11;
> +  a12 <<= 12;
> +  a13 <<= 13;
> +  a14 <<= 14;
> +  a15 <<= 15;
> +}
> +
> +void
> +ashiftrt (void)
> +{
> +  l1  >>= 1;
> +  l2  >>= 2;
> +  l3  >>= 3;
> +  l4  >>= 4;
> +  l5  >>= 5;
> +  l6  >>= 6;
> +  l7  >>= 7;
> +  l8  >>= 8;
> +  l9  >>= 9;
> +  l10 >>= 10;
> +  l11 >>= 11;
> +  l12 >>= 12;
> +  l13 >>= 13;
> +  l14 >>= 14;
> +  l15 >>= 15;
> +}
> diff --git a/gcc/testsuite/gcc.target/msp430/max-inline-shift-430.c b/gcc/testsuite/gcc.target/msp430/max-inline-shift-430.c
> new file mode 100644
> index 00000000000..7a519eec663
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/msp430/max-inline-shift-430.c
> @@ -0,0 +1,50 @@
> +/* { dg-do compile } */
> +/* { dg-skip-if "" { *-*-* } { "-mcpu=msp430x" "-mlarge" } { "" } } */
> +/* { dg-options "-mcpu=msp430 -mmax-inline-shift=10" } */
> +/* { dg-final { scan-assembler-not "__mspabi_slli_10" } } */
> +/* { dg-final { scan-assembler-not "__mspabi_sral_5" } } */
> +/* { dg-final { scan-assembler "__mspabi_slli_11" } } */
> +/* { dg-final { scan-assembler "__mspabi_sral_6" } } */
> +
> +volatile int a1, a2, a3, a4, a5, a6, a7, a8, a9, a10, a11, a12, a13, a14, a15;
> +volatile long l1, l2, l3, l4, l5, l6, l7, l8, l9, l10, l11, l12, l13, l14, l15;
> +
> +void
> +ashift (void)
> +{
> +  a1 <<= 1;
> +  a2 <<= 2;
> +  a3 <<= 3;
> +  a4 <<= 4;
> +  a5 <<= 5;
> +  a6 <<= 6;
> +  a7 <<= 7;
> +  a8 <<= 8;
> +  a9 <<= 9;
> +  a10 <<= 10;
> +  a11 <<= 11;
> +  a12 <<= 12;
> +  a13 <<= 13;
> +  a14 <<= 14;
> +  a15 <<= 15;
> +}
> +
> +void
> +ashiftrt (void)
> +{
> +  l1  >>= 1;
> +  l2  >>= 2;
> +  l3  >>= 3;
> +  l4  >>= 4;
> +  l5  >>= 5;
> +  l6  >>= 6;
> +  l7  >>= 7;
> +  l8  >>= 8;
> +  l9  >>= 9;
> +  l10 >>= 10;
> +  l11 >>= 11;
> +  l12 >>= 12;
> +  l13 >>= 13;
> +  l14 >>= 14;
> +  l15 >>= 15;
> +}
> diff --git a/gcc/testsuite/gcc.target/msp430/max-inline-shift-430x.c b/gcc/testsuite/gcc.target/msp430/max-inline-shift-430x.c
> new file mode 100644
> index 00000000000..074b3af5539
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/msp430/max-inline-shift-430x.c
> @@ -0,0 +1,48 @@
> +/* { dg-do compile } */
> +/* { dg-skip-if "" { *-*-* } { "-mcpu=msp430" } { "" } } */
> +/* { dg-options "-mcpu=msp430x -mmax-inline-shift=10" } */
> +/* { dg-final { scan-assembler-not "__mspabi_slli" } } */
> +/* { dg-final { scan-assembler "__mspabi_sral_6" } } */
> +
> +volatile int a1, a2, a3, a4, a5, a6, a7, a8, a9, a10, a11, a12, a13, a14, a15;
> +volatile long l1, l2, l3, l4, l5, l6, l7, l8, l9, l10, l11, l12, l13, l14, l15;
> +
> +void
> +ashift (void)
> +{
> +  a1 <<= 1;
> +  a2 <<= 2;
> +  a3 <<= 3;
> +  a4 <<= 4;
> +  a5 <<= 5;
> +  a6 <<= 6;
> +  a7 <<= 7;
> +  a8 <<= 8;
> +  a9 <<= 9;
> +  a10 <<= 10;
> +  a11 <<= 11;
> +  a12 <<= 12;
> +  a13 <<= 13;
> +  a14 <<= 14;
> +  a15 <<= 15;
> +}
> +
> +void
> +ashiftrt (void)
> +{
> +  l1  >>= 1;
> +  l2  >>= 2;
> +  l3  >>= 3;
> +  l4  >>= 4;
> +  l5  >>= 5;
> +  l6  >>= 6;
> +  l7  >>= 7;
> +  l8  >>= 8;
> +  l9  >>= 9;
> +  l10 >>= 10;
> +  l11 >>= 11;
> +  l12 >>= 12;
> +  l13 >>= 13;
> +  l14 >>= 14;
> +  l15 >>= 15;
> +}
> diff --git a/libgcc/config/msp430/slli.S b/libgcc/config/msp430/slli.S
> index c31e2d5db9b..b22622e0bf5 100644
> --- a/libgcc/config/msp430/slli.S
> +++ b/libgcc/config/msp430/slli.S
> @@ -65,6 +65,21 @@ __mspabi_slli:
>  	RET
>  #endif
>  
> +#ifdef __MSP430X__
> +	.section	.text.__gnu_mspabi_sllp
> +1:	ADDA	#-1,R13
> +	ADDA	R12,R12
> +	.global	__gnu_mspabi_sllp
> +__gnu_mspabi_sllp:
> +	CMP	#0,R13
> +	JNZ	1b
> +#ifdef __MSP430X_LARGE__
> +	RETA
> +#else
> +	RET
> +#endif /* __MSP430X_LARGE__ */
> +#endif /* __MSP430X__ */
> +
>  /* Logical Left Shift - R12:R13 -> R12:R13.  */
>  
>  	.section	.text.__mspabi_slll_n
> diff --git a/libgcc/config/msp430/srai.S b/libgcc/config/msp430/srai.S
> index d4a47fa985a..0100a368365 100644
> --- a/libgcc/config/msp430/srai.S
> +++ b/libgcc/config/msp430/srai.S
> @@ -64,6 +64,21 @@ __mspabi_srai:
>  	RET
>  #endif
>  
> +#ifdef __MSP430X__
> +	.section	.text.__gnu_mspabi_srap
> +1:	ADDA	#-1,R13
> +	RRAX.A	R12,R12
> +	.global	__gnu_mspabi_srap
> +__gnu_mspabi_srap:
> +	CMP	#0,R13
> +	JNZ	1b
> +#ifdef __MSP430X_LARGE__
> +	RETA
> +#else
> +	RET
> +#endif /* __MSP430X_LARGE__ */
> +#endif /* __MSP430X__ */
> +
>  /* Arithmetic Right Shift - R12:R13 -> R12:R13.  */
>  
>  	.section	.text.__mspabi_sral_n
> diff --git a/libgcc/config/msp430/srli.S b/libgcc/config/msp430/srli.S
> index 838c4bc0617..50db47c9938 100644
> --- a/libgcc/config/msp430/srli.S
> +++ b/libgcc/config/msp430/srli.S
> @@ -66,6 +66,22 @@ __mspabi_srli:
>  	RET
>  #endif
>  
> +#ifdef __MSP430X__
> +	.section	.text.__gnu_mspabi_srlp
> +1:	ADDA	#-1,R13
> +	CLRC
> +	RRCX.A	R12,R12
> +	.global	__gnu_mspabi_srlp
> +__gnu_mspabi_srlp:
> +	CMP	#0,R13
> +	JNZ	1b
> +#ifdef __MSP430X_LARGE__
> +	RETA
> +#else
> +	RET
> +#endif /* __MSP430X_LARGE__ */
> +#endif /* __MSP430X__ */
> +
>  /* Logical Right Shift - R12:R13 -> R12:R13.  */
>  
>  	.section	.text.__mspabi_srll_n
> -- 
> 2.27.0
Li, Pan2 via Gcc-patches Aug. 25, 2020, 7:16 p.m. UTC | #2
On Tue, 2020-07-21 at 19:29 +0100, Jozef Lawrynowicz wrote:
> The implementation of define_expand and define_insn patterns to handle
> shifts in the MSP430 backend is inconsistent, resulting in missed
> opportunities to make best use of the architecture's features.
> 
> There's now a single define_expand used as the entry point for all valid
> shifts, and the decision to either use a helper function to perform the
> shift (often required for the 430 ISA), or fall through to the
> define_insn patterns can be made from that expander function.
> 
> Shifts by a constant amount have been grouped into one define_insn for
> each type of shift, instead of having different define_insn patterns for
> shifts by different amounts.
> 
> A new target option "-mmax-inline-shift=" has been added to allow tuning
> of the number of shift instructions to emit inline, instead of using
> a library helper function.
> 
> Successfully regtested on trunk for msp430-elf, ok to apply?
OK
jeff
diff mbox series

Patch

diff --git a/gcc/config/msp430/constraints.md b/gcc/config/msp430/constraints.md
index 14368f7be6d..b8f9674b2fe 100644
--- a/gcc/config/msp430/constraints.md
+++ b/gcc/config/msp430/constraints.md
@@ -25,15 +25,16 @@  (define_register_constraint "R13" "R13_REGS"
   "Register R13.")
 
 (define_constraint "K"
-  "Integer constant 1."
+  "Integer constant 1-19."
   (and (match_code "const_int")
-       (match_test "IN_RANGE (ival, 1, 1)")))
+       (match_test "IN_RANGE (ival, 1, 19)")))
 
 (define_constraint "L"
   "Integer constant -1^20..1^19."
   (and (match_code "const_int")
        (match_test "IN_RANGE (ival, HOST_WIDE_INT_M1U << 20, 1 << 19)")))
 
+;; Valid shift amount for RRUM, RRAM, RLAM, RRCM.
 (define_constraint "M"
   "Integer constant 1-4."
   (and (match_code "const_int")
@@ -49,6 +50,11 @@  (define_constraint "O"
   (and (match_code "const_int")
        (match_test "IN_RANGE (ival, 256, 65535)")))
 
+(define_constraint "P"
+  "Integer constant 1-16."
+  (and (match_code "const_int")
+       (match_test "IN_RANGE (ival, 1, 16)")))
+
 ;; We do not allow arbitrary constants, eg symbols or labels,
 ;; because their address may be above the 16-bit address limit
 ;; supported by the offset used in the MOVA instruction.
diff --git a/gcc/config/msp430/msp430-protos.h b/gcc/config/msp430/msp430-protos.h
index a13a94cb92c..0b4d9a42b41 100644
--- a/gcc/config/msp430/msp430-protos.h
+++ b/gcc/config/msp430/msp430-protos.h
@@ -35,7 +35,6 @@  rtx	msp430_incoming_return_addr_rtx (void);
 void	msp430_init_cumulative_args (CUMULATIVE_ARGS *, tree, rtx, tree, int);
 int	msp430_initial_elimination_offset (int, int);
 bool    msp430_is_interrupt_func (void);
-const char * msp430x_logical_shift_right (rtx);
 const char * msp430_mcu_name (void);
 void    msp430_output_aligned_decl_common (FILE *, const tree, const char *,
 					   unsigned HOST_WIDE_INT, unsigned,
@@ -51,4 +50,9 @@  bool    msp430_use_f5_series_hwmult (void);
 bool	msp430_has_hwmult (void);
 bool msp430_op_not_in_high_mem (rtx op);
 
+#ifdef RTX_CODE
+int msp430_expand_shift (enum rtx_code code, machine_mode mode, rtx *operands);
+const char * msp430_output_asm_shift_insns (enum rtx_code code, machine_mode mode, rtx *operands);
+#endif
+
 #endif /* GCC_MSP430_PROTOS_H */
diff --git a/gcc/config/msp430/msp430.c b/gcc/config/msp430/msp430.c
index 455b4af3dad..c2b24974364 100644
--- a/gcc/config/msp430/msp430.c
+++ b/gcc/config/msp430/msp430.c
@@ -1064,15 +1064,6 @@  static bool msp430_rtx_costs (rtx	   x ATTRIBUTE_UNUSED,
 	  return true;
 	}
       break;
-    case ASHIFT:
-    case ASHIFTRT:
-    case LSHIFTRT:
-      if (!msp430x)
-	{
-	  *total = COSTS_N_INSNS (100);
-	  return true;
-	}
-      break;
     }
   return false;
 }
@@ -2674,32 +2665,6 @@  msp430_init_dwarf_reg_sizes_extra (tree address)
     }
 }
 
-/* This is a list of MD patterns that implement fixed-count shifts.  */
-static struct
-{
-  const char *name;
-  int count;
-  int need_430x;
-  rtx (*genfunc)(rtx,rtx);
-}
-const_shift_helpers[] =
-{
-#define CSH(N,C,X,G) { "__mspabi_" N, C, X, gen_##G }
-
-  CSH ("slli", 1, 1, slli_1),
-  CSH ("slll", 1, 1, slll_1),
-  CSH ("slll", 2, 1, slll_2),
-
-  CSH ("srai", 1, 0, srai_1),
-  CSH ("sral", 1, 0, sral_1),
-  CSH ("sral", 2, 0, sral_2),
-
-  CSH ("srll", 1, 0, srll_1),
-  CSH ("srll", 2, 1, srll_2x),
-  { 0, 0, 0, 0 }
-#undef CSH
-};
-
 /* The MSP430 ABI defines a number of helper functions that should be
    used for, for example, 32-bit shifts.  This function is called to
    emit such a function, using the table above to optimize some
@@ -2716,31 +2681,12 @@  msp430_expand_helper (rtx *operands, const char *helper_name,
   machine_mode arg0mode = GET_MODE (operands[0]);
   machine_mode arg1mode = GET_MODE (operands[1]);
   machine_mode arg2mode = GET_MODE (operands[2]);
-  int have_430x = msp430x ? 1 : 0;
   int expand_mpy = strncmp (helper_name, "__mspabi_mpy",
 			    sizeof ("__mspabi_mpy") - 1) == 0;
   /* This function has been used incorrectly if CONST_VARIANTS is TRUE for a
      hwmpy function.  */
   gcc_assert (!(expand_mpy && const_variants));
 
-  /* Emit size-optimal insns for small shifts we can easily do inline.  */
-  if (CONST_INT_P (operands[2]) && !expand_mpy)
-    {
-      int i;
-
-      for (i=0; const_shift_helpers[i].name; i++)
-	{
-	  if (const_shift_helpers[i].need_430x <= have_430x
-	      && strcmp (helper_name, const_shift_helpers[i].name) == 0
-	      && INTVAL (operands[2]) == const_shift_helpers[i].count)
-	    {
-	      emit_insn (const_shift_helpers[i].genfunc (operands[0],
-							 operands[1]));
-	      return;
-	    }
-	}
-    }
-
   if (arg1mode != VOIDmode && arg2mode != VOIDmode)
     /* Modes of arguments must be equal if not constants.  */
     gcc_assert (arg1mode == arg2mode);
@@ -2835,6 +2781,190 @@  msp430_expand_helper (rtx *operands, const char *helper_name,
 		  gen_rtx_REG (arg0mode, 12));
 }
 
+/* Return TRUE if the helper function should be used and FALSE if the shifts
+   insns should be emitted inline.  */
+static bool
+use_helper_for_const_shift (enum rtx_code code, machine_mode mode,
+			    HOST_WIDE_INT amt)
+{
+  const int default_inline_shift = 4;
+  /* We initialize the option to 65 so we know if the user set it or not.  */
+  int user_set_max_inline = (msp430_max_inline_shift == 65 ? 0 : 1);
+  int max_inline = (user_set_max_inline ? msp430_max_inline_shift
+		    : default_inline_shift);
+  /* 32-bit shifts are roughly twice as costly as 16-bit shifts so we adjust
+     the heuristic accordingly.  */
+  int max_inline_32 = max_inline / 2;
+
+  /* Don't use helpers for these modes on 430X, when optimizing for speed, or
+     when emitting a small number of insns.  */
+  if ((mode == E_QImode || mode == E_HImode || mode == E_PSImode)
+      && (msp430x
+	  /* If the user set max_inline then we always obey that number.
+	     Otherwise we always emit the shifts inline at -O2 and above.  */
+	  || amt <= max_inline
+	  || (!user_set_max_inline
+	      && (optimize >= 2 && !optimize_size))))
+    return false;
+
+  /* 430 and 430X codegen for SImode shifts is the same.
+     Set a hard limit of 15 for the number of shifts that will be emitted
+     inline by default, even at -O2 and above, to prevent code size
+     explosion.  */
+  if (mode == E_SImode
+      && (amt <= max_inline_32
+	  || (!user_set_max_inline
+	      && (optimize >= 2 && !optimize_size)
+	      && amt <= 15)))
+    return false;
+
+  return true;
+}
+
+/* For shift operations which will use an mspabi helper function, setup the
+   call to msp430_expand helper.  Return 1 to indicate we have finished with
+   this insn and invoke "DONE".
+   Otherwise return 0 to indicate the insn should fallthrough.
+   Never FAIL.  */
+int
+msp430_expand_shift (enum rtx_code code, machine_mode mode, rtx *operands)
+{
+  /* Always use the helper function when the shift amount is not a
+     constant.  */
+  if (!CONST_INT_P (operands[2])
+      || mode == E_DImode
+      || use_helper_for_const_shift (code, mode, INTVAL (operands[2])))
+    {
+      const char *helper_name = NULL;
+      /* The const variants of mspabi shifts have significantly larger code
+	 size than the generic version, so use the generic version if
+	 optimizing for size.  */
+      bool const_variant = !optimize_size;
+      switch (mode)
+	{
+	case E_HImode:
+	  helper_name = (code == ASHIFT ? "__mspabi_slli" :
+			 (code == ASHIFTRT ? "__mspabi_srai" :
+			  (code == LSHIFTRT ? "__mspabi_srli" :
+			   NULL)));
+	  break;
+	case E_PSImode:
+	  helper_name = (code == ASHIFT ? "__gnu_mspabi_sllp" :
+			 (code == ASHIFTRT ? "__gnu_mspabi_srap" :
+			  (code == LSHIFTRT ? "__gnu_mspabi_srlp" :
+			   NULL)));
+	  /* No const variant for PSImode shifts FIXME.  */
+	  const_variant = false;
+	  break;
+	case E_SImode:
+	  helper_name = (code == ASHIFT ? "__mspabi_slll" :
+			 (code == ASHIFTRT ? "__mspabi_sral" :
+			  (code == LSHIFTRT ? "__mspabi_srll" :
+			   NULL)));
+	  break;
+	case E_DImode:
+	  helper_name = (code == ASHIFT ? "__mspabi_sllll" :
+			 (code == ASHIFTRT ? "__mspabi_srall" :
+			  (code == LSHIFTRT ? "__mspabi_srlll" :
+			   NULL)));
+	  /* No const variant for DImode shifts.  */
+	  const_variant = false;
+	  break;
+	default:
+	  gcc_unreachable ();
+	  break;
+	}
+      gcc_assert (helper_name);
+      msp430_expand_helper (operands, helper_name, const_variant);
+      return 1;
+    }
+  /* When returning 0, there must be an insn to match the RTL pattern
+     otherwise there will be an unrecognizeable insn.  */
+  return 0;
+}
+
+/* Helper function to emit a sequence of shift instructions.  The amount of
+   shift instructions to emit is in OPERANDS[2].
+   For 430 we output copies of identical inline shifts for all modes.
+   For 430X it is inneficient to do so for any modes except SI and DI, since we
+   can make use of R*M insns or RPT with 430X insns, so this function is only
+   used for SImode in that case.  */
+const char *
+msp430_output_asm_shift_insns (enum rtx_code code, machine_mode mode,
+			       rtx *operands)
+{
+  int i;
+  int amt;
+  int max_shift = GET_MODE_BITSIZE (mode) - 1;
+  gcc_assert (CONST_INT_P (operands[2]));
+  amt = INTVAL (operands[2]);
+
+  if (amt == 0 || amt > max_shift)
+    {
+      switch (code)
+	{
+	case ASHIFT:
+	  output_asm_insn ("# ignored undefined behaviour left shift "
+			   "of %1 by %2", operands);
+	  break;
+	case ASHIFTRT:
+	  output_asm_insn ("# ignored undefined behaviour arithmetic right "
+			   "shift of %1 by %2", operands);
+	  break;
+	case LSHIFTRT:
+	  output_asm_insn ("# ignored undefined behaviour logical right shift "
+			   "of %1 by %2", operands);
+	  break;
+	default:
+	  gcc_unreachable ();
+	}
+      return "";
+    }
+
+  if (code == ASHIFT)
+    {
+      if (!msp430x && mode == HImode)
+	for (i = 0; i < amt; i++)
+	  output_asm_insn ("RLA.W\t%0", operands);
+      else if (mode == SImode)
+	for (i = 0; i < amt; i++)
+	  output_asm_insn ("RLA%X0.W\t%L0 { RLC%X0.W\t%H0", operands);
+      else
+	/* Catch unhandled cases.  */
+	gcc_unreachable ();
+    }
+  else if (code == ASHIFTRT)
+    {
+      if (!msp430x && mode == HImode)
+	for (i = 0; i < amt; i++)
+	  output_asm_insn ("RRA.W\t%0", operands);
+      else if (mode == SImode)
+	for (i = 0; i < amt; i++)
+	  output_asm_insn ("RRA%X0.W\t%H0 { RRC%X0.W\t%L0", operands);
+      else
+	gcc_unreachable ();
+    }
+  else if (code == LSHIFTRT)
+    {
+      if (!msp430x && mode == HImode)
+	for (i = 0; i < amt; i++)
+	  output_asm_insn ("CLRC { RRC.W\t%0", operands);
+      else if (mode == SImode)
+	for (i = 0; i < amt; i++)
+	  output_asm_insn ("CLRC { RRC%X0.W\t%H0 { RRC%X0.W\t%L0", operands);
+      /* FIXME: Why doesn't "RRUX.W\t%H0 { RRC%X0.W\t%L0" work for msp430x?
+	 It causes execution timeouts e.g. pr41963.c.  */
+#if 0
+      else if (msp430x && mode == SImode)
+	for (i = 0; i < amt; i++)
+	  output_asm_insn ("RRUX.W\t%H0 { RRC%X0.W\t%L0", operands);
+#endif
+      else
+	gcc_unreachable ();
+    }
+  return "";
+}
+
 /* Called by cbranch<mode>4 to coerce operands into usable forms.  */
 void
 msp430_fixup_compare_operands (machine_mode my_mode, rtx * operands)
@@ -3368,6 +3498,7 @@  msp430_op_not_in_high_mem (rtx op)
    O   offset of the top of the stack
    Q   like X but generates an A postfix
    R   inverse of condition code, unsigned.
+   W   value - 16
    X   X instruction postfix in large mode
    Y   value - 4
    Z   value - 1
@@ -3394,6 +3525,11 @@  msp430_print_operand (FILE * file, rtx op, int letter)
       /* Print the constant value, less four.  */
       fprintf (file, "#%ld", INTVAL (op) - 4);
       return;
+    case 'W':
+      gcc_assert (CONST_INT_P (op));
+      /* Print the constant value, less 16.  */
+      fprintf (file, "#%ld", INTVAL (op) - 16);
+      return;
     case 'I':
       if (GET_CODE (op) == CONST_INT)
 	{
@@ -3711,34 +3847,6 @@  msp430x_extendhisi (rtx * operands)
   return "MOV.W\t%1, %L0 { MOV.W\t%1, %H0 { RPT\t#15 { RRAX.W\t%H0";
 }
 
-/* Likewise for logical right shifts.  */
-const char *
-msp430x_logical_shift_right (rtx amount)
-{
-  /* The MSP430X's logical right shift instruction - RRUM - does
-     not use an extension word, so we cannot encode a repeat count.
-     Try various alternatives to work around this.  If the count
-     is in a register we are stuck, hence the assert.  */
-  gcc_assert (CONST_INT_P (amount));
-
-  if (INTVAL (amount) <= 0
-      || INTVAL (amount) >= 16)
-    return "# nop logical shift.";
-
-  if (INTVAL (amount) > 0
-      && INTVAL (amount) < 5)
-    return "rrum.w\t%2, %0"; /* Two bytes.  */
-
-  if (INTVAL (amount) > 4
-      && INTVAL (amount) < 9)
-    return "rrum.w\t#4, %0 { rrum.w\t%Y2, %0 "; /* Four bytes.  */
-
-  /* First we logically shift right by one.  Now we know
-     that the top bit is zero and we can use the arithmetic
-     right shift instruction to perform the rest of the shift.  */
-  return "rrum.w\t#1, %0 { rpt\t%Z2 { rrax.w\t%0"; /* Six bytes.  */
-}
-
 /* Stop GCC from thinking that it can eliminate (SUBREG:PSI (SI)).  */
 
 #undef TARGET_CAN_CHANGE_MODE_CLASS
diff --git a/gcc/config/msp430/msp430.md b/gcc/config/msp430/msp430.md
index ed21eb02868..f70e61b97dd 100644
--- a/gcc/config/msp430/msp430.md
+++ b/gcc/config/msp430/msp430.md
@@ -65,6 +65,15 @@  (define_attr "length" "" (const_int 4))
 (include "constraints.md")
 
 (define_mode_iterator QHI [QI HI PSI])
+(define_mode_iterator HPSI [HI PSI])
+(define_mode_iterator HDI [HI PSI SI DI])
+
+;; Mapping of all shift operators
+(define_code_iterator any_shift [ashift ashiftrt lshiftrt])
+
+;; Base name for define_insn
+(define_code_attr shift_insn
+  [(ashift "ashl") (lshiftrt "lshr") (ashiftrt "ashr")])
 
 ;; There are two basic "family" tests we do here:
 ;;
@@ -689,31 +698,42 @@  (define_insn ""
    MOV%X1.B\t%1, %0"
 )
 
+;; The next three insns emit identical assembly code.
+;; They take a QImode and shift it in SImode.  Only shift counts <= 8
+;; are handled since that is the simple case where the high 16-bits (i.e. the
+;; high register) are always 0.
 (define_insn ""
-  [(set (match_operand:SI 0 "register_operand" "=r")
-	(ashift:SI (zero_extend:SI (match_operand:QI 1 "general_operand" "rm"))
-		   (match_operand:HI 2 "immediate_operand" "M")))]
+  [(set (match_operand:SI			     0 "register_operand" "=r,r,r")
+	(ashift:SI (zero_extend:SI (match_operand:QI 1 "general_operand" "0,rm,rm"))
+		   (match_operand:HI		     2 "const_1_to_8_operand" "M,M,i")))]
   "msp430x"
-  "MOV%X1.B %1, %L0 { RLAM.W %2, %L0 { CLR %H0"
+  "@
+  RLAM.W %2, %L0 { CLR %H0
+  MOV%X1.B %1, %L0 { RLAM.W %2, %L0 { CLR %H0
+  MOV%X1.B %1, %L0 { RPT %2 { RLAX.W %L0 { CLR %H0"
 )
 
-;; We are taking a char and shifting it and putting the result in 2 registers.
-;; the high register will always be for 0 shift counts < 8.
 (define_insn ""
-  [(set (match_operand:SI 0 "register_operand" "=r")
-	(ashift:SI (zero_extend:SI (subreg:HI (match_operand:QI 1 "general_operand" "rm") 0))
-		   (match_operand:HI 2 "immediate_operand" "M")))]
+  [(set (match_operand:SI			     		0 "register_operand" "=r,r,r")
+	(ashift:SI (zero_extend:SI (subreg:HI (match_operand:QI 1 "general_operand" "0,rm,rm") 0))
+		   (match_operand:HI		     		2 "const_1_to_8_operand" "M,M,i")))]
   "msp430x"
-  "MOV%X1.B %1, %L0 { RLAM.W %2, %L0 { CLR %H0"
+  "@
+  RLAM.W %2, %L0 { CLR %H0
+  MOV%X1.B %1, %L0 { RLAM.W %2, %L0 { CLR %H0
+  MOV%X1.B %1, %L0 { RPT %2 { RLAX.W %L0 { CLR %H0"
 )
 
 ;; Same as above but with a NOP sign_extend round the subreg
 (define_insn ""
-  [(set (match_operand:SI 0 "register_operand" "=r")
-	(ashift:SI (zero_extend:SI (sign_extend:PSI (subreg:HI (match_operand:QI 1 "general_operand" "rm") 0)))
-		   (match_operand:HI 2 "immediate_operand" "M")))]
+  [(set (match_operand:SI			     				 0 "register_operand" "=r,r,r")
+	(ashift:SI (zero_extend:SI (sign_extend:PSI (subreg:HI (match_operand:QI 1 "general_operand" "0,rm,rm") 0)))
+		   (match_operand:HI		     				 2 "const_1_to_8_operand" "M,M,i")))]
   "msp430x"
-  "MOV%X1.B %1, %L0 { RLAM.W %2, %L0 { CLR %H0"
+  "@
+  RLAM.W %2, %L0 { CLR %H0
+  MOV%X1.B %1, %L0 { RLAM.W %2, %L0 { CLR %H0
+  MOV%X1.B %1, %L0 { RPT %2 { RLAX.W %L0 { CLR %H0"
 )
 
 (define_insn ""
@@ -724,11 +744,14 @@  (define_insn ""
 )
 
 (define_insn ""
-  [(set (match_operand:PSI 0 "register_operand" "=r")
-	(ashift:PSI (sign_extend:PSI (subreg:HI (match_operand:QI 1 "general_operand" "rm") 0))
-		    (match_operand:HI 2 "immediate_operand" "M")))]
+  [(set (match_operand:PSI					  0 "register_operand" "=r,r,r")
+	(ashift:PSI (sign_extend:PSI (subreg:HI (match_operand:QI 1 "general_operand" "0,rm,rm") 0))
+		    (match_operand:HI				  2 "const_1_to_19_operand" "M,M,i")))]
   "msp430x"
-  "MOV%X1.B %1, %0 { RLAM.W %2, %0"
+  "@
+  RLAM.W %2, %0
+  MOV%X1.B %1, %0 { RLAM.W %2, %0
+  MOV%X1.B %1, %0 { RPT %2 { RLAX.A %0"
 )
 ;; END msp430 pointer manipulation combine insn patterns
 
@@ -840,287 +863,75 @@  (define_insn "truncsipsi2"
 ;; Note - we ignore shift counts of less than one or more than 15.
 ;; This is permitted by the ISO C99 standard as such shifts result
 ;; in "undefined" behavior.  [6.5.7 (3)]
+;;
+;; We avoid emitting insns in msp430_expand_shift, since we would have to handle
+;; many extra cases such as op0 != op1, or, op0 or op1 in memory.  Instead we
+;; let reload coerce op0 and op1 into the same register.
 
-;; signed A << C
-
-(define_expand "ashlhi3"
-  [(set (match_operand:HI	     0 "msp430_general_dst_nonv_operand")
-	(ashift:HI (match_operand:HI 1 "general_operand")
-		   (match_operand:HI 2 "general_operand")))]
+(define_expand "<shift_insn><mode>3"
+  [(set (match_operand:HDI		  0 "msp430_general_dst_nonv_operand")
+	(any_shift:HDI (match_operand:HDI 1 "general_operand")
+		       (match_operand:HDI 2 "general_operand")))]
   ""
   {
-    if ((GET_CODE (operands[1]) == SUBREG
-	 && REG_P (XEXP (operands[1], 0)))
-	|| MEM_P (operands[1]))
-      operands[1] = force_reg (HImode, operands[1]);
-    if (msp430x
-        && REG_P (operands[0])
-        && REG_P (operands[1])
-        && CONST_INT_P (operands[2]))
-      emit_insn (gen_430x_shift_left (operands[0], operands[1], operands[2]));
-    else if (CONST_INT_P (operands[2])
-	     && INTVAL (operands[2]) == 1)
-      emit_insn (gen_slli_1 (operands[0], operands[1]));
-    else		 
-      /* The const variants of mspabi shifts have larger code size than the
-	 generic version, so use the generic version if optimizing for
-	 size.  */
-      msp430_expand_helper (operands, \"__mspabi_slli\", !optimize_size);
-    DONE;
+    if (msp430_expand_shift (<CODE>, <MODE>mode, operands))
+      DONE;
+    /* Otherwise, fallthrough.  */
   }
 )
 
-(define_insn "slli_1"
-  [(set (match_operand:HI	     0 "msp430_general_dst_nonv_operand" "=rm")
-	(ashift:HI (match_operand:HI 1 "general_operand"       "0")
-		   (const_int 1)))]
-  ""
-  "RLA%X0.W\t%0" ;; Note - this is a macro for ADD
-)
-
-(define_insn "430x_shift_left"
-  [(set (match_operand:HI            0 "register_operand" "=r")
-	(ashift:HI (match_operand:HI 1 "register_operand"  "0")
-		   (match_operand    2 "immediate_operand" "n")))]
-  "msp430x"
-  "*
-  if (INTVAL (operands[2]) > 0 && INTVAL (operands[2]) < 5)
-    return \"RLAM.W\t%2, %0\";
-  else if (INTVAL (operands[2]) >= 5 && INTVAL (operands[2]) < 16)
-    return \"RPT\t%2 { RLAX.W\t%0\";
-  return \"# nop left shift\";
-  "
+;; All 430 HImode constant shifts
+(define_insn "<shift_insn>hi3_430"
+  [(set (match_operand:HI		0 "msp430_general_dst_nonv_operand" "=rm")
+	(any_shift:HI (match_operand:HI 1 "general_operand"       "0")
+		      (match_operand:HI 2 "const_int_operand"     "n")))]
+  "!msp430x"
+  "* return msp430_output_asm_shift_insns (<CODE>, HImode, operands);"
 )
 
-(define_insn "slll_1"
-  [(set (match_operand:SI	     0 "msp430_general_dst_nonv_operand" "=rm")
-	(ashift:SI (match_operand:SI 1 "general_operand"       "0")
-		   (const_int 1)))]
+;; All 430 and 430X SImode constant shifts
+(define_insn "<shift_insn>si3_const"
+  [(set (match_operand:SI		0 "msp430_general_dst_nonv_operand" "=rm")
+	(any_shift:SI (match_operand:SI 1 "general_operand"       "0")
+		      (match_operand:SI 2 "const_int_operand"     "n")))]
   ""
-  "RLA%X0.W\t%L0 { RLC%X0.W\t%H0"
-)
-
-(define_insn "slll_2"
-  [(set (match_operand:SI	     0 "msp430_general_dst_nonv_operand" "=rm")
-	(ashift:SI (match_operand:SI 1 "general_operand"       "0")
-		   (const_int 2)))]
-  ""
-  "RLA%X0.W\t%L0 { RLC%X0.W\t%H0 { RLA%X0.W\t%L0 { RLC%X0.W\t%H0"
-)
-
-(define_expand "ashlsi3"
-  [(set (match_operand:SI	     0 "msp430_general_dst_nonv_operand")
-	(ashift:SI (match_operand:SI 1 "general_operand")
-		   (match_operand:SI 2 "general_operand")))]
-  ""
-  "msp430_expand_helper (operands, \"__mspabi_slll\", !optimize_size);
-   DONE;"
-)
-
-(define_expand "ashldi3"
-  [(set (match_operand:DI	     0 "msp430_general_dst_nonv_operand")
-	(ashift:DI (match_operand:DI 1 "general_operand")
-		   (match_operand:DI 2 "general_operand")))]
-  ""
-  {
-    /* No const_variant for 64-bit shifts.  */
-    msp430_expand_helper (operands, \"__mspabi_sllll\", false);
-    DONE;
-  }
-)
-
-;;----------
-
-;; signed A >> C
-
-(define_expand "ashrhi3"
-  [(set (match_operand:HI	       0 "msp430_general_dst_nonv_operand")
-	(ashiftrt:HI (match_operand:HI 1 "general_operand")
-		     (match_operand:HI 2 "general_operand")))]
-  ""
-  {
-    if ((GET_CODE (operands[1]) == SUBREG
-	 && REG_P (XEXP (operands[1], 0)))
-	|| MEM_P (operands[1]))
-      operands[1] = force_reg (HImode, operands[1]);
-    if (msp430x
-        && REG_P (operands[0])
-        && REG_P (operands[1])
-        && CONST_INT_P (operands[2]))
-      emit_insn (gen_430x_arithmetic_shift_right (operands[0], operands[1], operands[2]));
-    else if (CONST_INT_P (operands[2])
-	     && INTVAL (operands[2]) == 1)
-      emit_insn (gen_srai_1 (operands[0], operands[1]));
-    else		 
-       msp430_expand_helper (operands, \"__mspabi_srai\", !optimize_size);
-   DONE;
-   }
-)
-
-(define_insn "srai_1"
-  [(set (match_operand:HI	       0 "msp430_general_dst_operand" "=rm")
-	(ashiftrt:HI (match_operand:HI 1 "msp430_general_operand"      "0")
-		     (const_int 1)))]
-  ""
-  "RRA%X0.W\t%0"
-)
-
-(define_insn "430x_arithmetic_shift_right"
-  [(set (match_operand:HI              0 "register_operand" "=r")
-	(ashiftrt:HI (match_operand:HI 1 "register_operand"  "0")
-		     (match_operand    2 "immediate_operand" "n")))]
-  "msp430x"
-  "*
-  if (INTVAL (operands[2]) > 0 && INTVAL (operands[2]) < 5)
-    return \"RRAM.W\t%2, %0\";
-  else if (INTVAL (operands[2]) >= 5 && INTVAL (operands[2]) < 16)
-    return \"RPT\t%2 { RRAX.W\t%0\";
-  return \"# nop arith right shift\";
-  "
-)
-
-(define_insn "srap_1"
-  [(set (match_operand:PSI              0 "register_operand" "=r")
-	(ashiftrt:PSI (match_operand:PSI 1 "general_operand" "0")
-		      (const_int 1)))]
-  "msp430x"
-  "RRAM.A #1,%0"
+  "* return msp430_output_asm_shift_insns (<CODE>, SImode, operands);"
 )
 
-(define_insn "srap_2"
-  [(set (match_operand:PSI              0 "register_operand" "=r")
-	(ashiftrt:PSI (match_operand:PSI 1 "general_operand" "0")
-		      (const_int 2)))]
+(define_insn "ashl<mode>3_430x"
+  [(set (match_operand:HPSI		 0 "msp430_general_dst_nonv_operand" "=r,r,r,r")
+	(ashift:HPSI (match_operand:HPSI 1 "general_operand" 		     "0 ,0,0,0")
+		     (match_operand:HPSI 2 "const_int_operand" 		     "M ,P,K,i")))]
   "msp430x"
-  "RRAM.A #2,%0"
-)
-
-(define_insn "sral_1"
-  [(set (match_operand:SI	       0 "msp430_general_dst_nonv_operand" "=rm")
-	(ashiftrt:SI (match_operand:SI 1 "general_operand"       "0")
-		     (const_int 1)))]
-  ""
-  "RRA%X0.W\t%H0 { RRC%X0.W\t%L0"
-)
-
-(define_insn "sral_2"
-  [(set (match_operand:SI	       0 "msp430_general_dst_nonv_operand" "=rm")
-	(ashiftrt:SI (match_operand:SI 1 "general_operand"       "0")
-		     (const_int 2)))]
-  ""
-  "RRA%X0.W\t%H0 { RRC%X0.W\t%L0 { RRA%X0.W\t%H0 { RRC%X0.W\t%L0"
-)
-
-(define_expand "ashrsi3"
-  [(set (match_operand:SI	       0 "msp430_general_dst_nonv_operand")
-	(ashiftrt:SI (match_operand:SI 1 "general_operand")
-		     (match_operand:SI 2 "general_operand")))]
-  ""
-  "msp430_expand_helper (operands, \"__mspabi_sral\", !optimize_size);
-   DONE;"
-)
-
-(define_expand "ashrdi3"
-  [(set (match_operand:DI	     0 "msp430_general_dst_nonv_operand")
-	(ashift:DI (match_operand:DI 1 "general_operand")
-		   (match_operand:DI 2 "general_operand")))]
-  ""
-  {
-    /* No const_variant for 64-bit shifts.  */
-    msp430_expand_helper (operands, \"__mspabi_srall\", false);
-    DONE;
-  }
-)
-
-;;----------
-
-;; unsigned A >> C
-
-(define_expand "lshrhi3"
-  [(set (match_operand:HI	       0 "msp430_general_dst_nonv_operand")
-	(lshiftrt:HI (match_operand:HI 1 "general_operand")
-		     (match_operand:HI 2 "general_operand")))]
-  ""
-  {
-    if ((GET_CODE (operands[1]) == SUBREG
-	 && REG_P (XEXP (operands[1], 0)))
-	|| MEM_P (operands[1]))
-      operands[1] = force_reg (HImode, operands[1]);
-    if (msp430x
-        && REG_P (operands[0])
-        && REG_P (operands[1])
-        && CONST_INT_P (operands[2]))
-      emit_insn (gen_430x_logical_shift_right (operands[0], operands[1], operands[2]));
-    else if (CONST_INT_P (operands[2])
-	     && INTVAL (operands[2]) == 1)
-      emit_insn (gen_srli_1 (operands[0], operands[1]));
-    else		 
-      msp430_expand_helper (operands, \"__mspabi_srli\", !optimize_size);
-    DONE;
-  }
-)
-
-(define_insn "srli_1"
-  [(set (match_operand:HI	       0 "msp430_general_dst_nonv_operand" "=rm")
-	(lshiftrt:HI (match_operand:HI 1 "general_operand"       "0")
-		     (const_int 1)))]
-  ""
-  "CLRC { RRC%X0.W\t%0"
+  "@
+  RLAM%b0\t%2, %0
+  RPT\t%2 { RLAX%b0\t%0
+  RPT\t#16 { RLAX%b0\t%0 { RPT\t%W2 { RLAX%b0\t%0
+  # undefined behavior left shift of %1 by %2"
 )
 
-(define_insn "430x_logical_shift_right"
-  [(set (match_operand:HI              0 "register_operand" "=r")
-	(lshiftrt:HI (match_operand:HI 1 "register_operand"  "0")
-		     (match_operand    2 "immediate_operand" "n")))]
+(define_insn "ashr<mode>3_430x"
+  [(set (match_operand:HPSI		   0 "msp430_general_dst_nonv_operand" "=r,r,r,r")
+	(ashiftrt:HPSI (match_operand:HPSI 1 "general_operand"	  	     "0,0,0,0")
+		       (match_operand:HPSI 2 "const_int_operand" 	     "M,P,K,i")))]
   "msp430x"
-  {
-    return msp430x_logical_shift_right (operands[2]);
-  }
-)
-
-(define_insn "srlp_1"
-  [(set (match_operand:PSI              0 "register_operand" "=r")
-	(lshiftrt:PSI (match_operand:PSI 1 "general_operand" "0")
-		      (const_int 1)))]
-  ""
-  "RRUM.A #1,%0"
-)
-
-(define_insn "srll_1"
-  [(set (match_operand:SI	       0 "msp430_general_dst_nonv_operand" "=rm")
-	(lshiftrt:SI (match_operand:SI 1 "general_operand"       "0")
-		     (const_int 1)))]
-  ""
-  "CLRC { RRC%X0.W\t%H0 { RRC%X0.W\t%L0"
+  "@
+  RRAM%b0\t%2, %0
+  RPT\t%2 { RRAX%b0\t%0
+  RPT\t#16 { RRAX%b0\t%0 { RPT\t%W2 { RRAX%b0\t%0
+  # undefined behavior arithmetic right shift of %1 by %2"
 )
 
-(define_insn "srll_2x"
-  [(set (match_operand:SI	       0 "msp430_general_dst_nonv_operand" "=r")
-	(lshiftrt:SI (match_operand:SI 1 "general_operand"       "0")
-		     (const_int 2)))]
+(define_insn "lshr<mode>3_430x"
+  [(set (match_operand:HPSI		   0 "msp430_general_dst_nonv_operand" "=r,r,r,r")
+	(lshiftrt:HPSI (match_operand:HPSI 1 "general_operand"	  	     "0,0,0,0")
+		       (match_operand:HPSI 2 "const_int_operand" 	     "M,P,K,i")))]
   "msp430x"
-  "RRUX.W\t%H0 { RRC.W\t%L0 { RRUX.W\t%H0 { RRC.W\t%L0"
-)
-
-(define_expand "lshrsi3"
-  [(set (match_operand:SI	       0 "msp430_general_dst_nonv_operand")
-	(lshiftrt:SI (match_operand:SI 1 "general_operand")
-		     (match_operand:SI 2 "general_operand")))]
-  ""
-  "msp430_expand_helper (operands, \"__mspabi_srll\", !optimize_size);
-   DONE;"
-)
-
-(define_expand "lshrdi3"
-  [(set (match_operand:DI	     0 "msp430_general_dst_nonv_operand")
-	(ashift:DI (match_operand:DI 1 "general_operand")
-		   (match_operand:DI 2 "general_operand")))]
-  ""
-  {
-    /* No const_variant for 64-bit shifts.  */
-    msp430_expand_helper (operands, \"__mspabi_srlll\", false);
-    DONE;
-  }
+  "@
+  RRUM%b0\t%2, %0
+  RPT\t%2 { RRUX%b0\t%0
+  RPT\t#16 { RRUX%b0\t%0 { RPT\t%W2 { RRUX%b0\t%0
+  # undefined behavior logical right shift of %1 by %2"
 )
 
 ;;------------------------------------------------------------
@@ -1427,7 +1238,7 @@  (define_insn "*bitbranch<mode>4_z"
   [(set (pc) (if_then_else
 	      (ne (zero_extract:HI (match_operand:QHI 0 "msp430_general_dst_operand" "rYs,rm")
 				    (const_int 1)
-				    (match_operand 1 "msp430_bitpos" "i,i"))
+				    (match_operand 1 "const_0_to_15_operand" "i,i"))
 		  (const_int 0))
               (label_ref (match_operand 2 "" ""))
 	      (pc)))
@@ -1443,7 +1254,7 @@  (define_insn "*bitbranch<mode>4_z"
   [(set (pc) (if_then_else
 	      (eq (zero_extract:HI (match_operand:QHI 0 "msp430_general_dst_operand" "rm")
 				   (const_int 1)
-				   (match_operand 1 "msp430_bitpos" "i"))
+				   (match_operand 1 "const_0_to_15_operand" "i"))
 		  (const_int 0))
               (label_ref (match_operand 2 "" ""))
 	      (pc)))
@@ -1457,7 +1268,7 @@  (define_insn "*bitbranch<mode>4_z"
   [(set (pc) (if_then_else
 	      (eq (zero_extract:HI (match_operand:QHI 0 "msp430_general_dst_operand" "rm")
 				   (const_int 1)
-				   (match_operand 1 "msp430_bitpos" "i"))
+				   (match_operand 1 "const_0_to_15_operand" "i"))
 		  (const_int 0))
               (pc)
 	      (label_ref (match_operand 2 "" ""))))
@@ -1471,7 +1282,7 @@  (define_insn "*bitbranch<mode>4_z"
   [(set (pc) (if_then_else
 	      (ne (zero_extract:HI (match_operand:QHI 0 "msp430_general_dst_operand" "rm")
 				   (const_int 1)
-				   (match_operand 1 "msp430_bitpos" "i"))
+				   (match_operand 1 "const_0_to_15_operand" "i"))
 		  (const_int 0))
               (pc)
 	      (label_ref (match_operand 2 "" ""))))
diff --git a/gcc/config/msp430/msp430.opt b/gcc/config/msp430/msp430.opt
index b451174c3d1..8134ca7ac95 100644
--- a/gcc/config/msp430/msp430.opt
+++ b/gcc/config/msp430/msp430.opt
@@ -109,3 +109,9 @@  mdevices-csv-loc=
 Target Joined Var(msp430_devices_csv_loc) RejectNegative Report
 The path to devices.csv.  The GCC driver can normally locate devices.csv itself
 and pass this option to the compiler, so the user shouldn't need to pass this.
+
+mmax-inline-shift=
+Target RejectNegative Joined UInteger IntegerRange(0,65) Var(msp430_max_inline_shift) Init(65) Report
+For shift operations by a constant amount, which require an individual instruction to shift by one
+position, set the maximum number of inline shift instructions (maximum value 64) to emit instead of using the corresponding __mspabi helper function.
+The default value is 4.
diff --git a/gcc/config/msp430/predicates.md b/gcc/config/msp430/predicates.md
index 408d56f610a..4bfa0c0f2d5 100644
--- a/gcc/config/msp430/predicates.md
+++ b/gcc/config/msp430/predicates.md
@@ -113,12 +113,21 @@  (define_predicate "msp430_nonsubregnonpostinc_or_imm_operand"
        (ior (match_code "reg,mem")
 	    (match_operand 0 "immediate_operand"))))
 
-; TRUE for constants which are bit positions for zero_extract
-(define_predicate "msp430_bitpos"
+(define_predicate "const_1_to_8_operand"
+  (and (match_code "const_int")
+       (match_test ("   INTVAL (op) >= 1
+		     && INTVAL (op) <= 8 "))))
+
+(define_predicate "const_0_to_15_operand"
   (and (match_code "const_int")
        (match_test ("   INTVAL (op) >= 0
 		     && INTVAL (op) <= 15 "))))
 
+(define_predicate "const_1_to_19_operand"
+  (and (match_code "const_int")
+       (match_test ("   INTVAL (op) >= 1
+		     && INTVAL (op) <= 19 "))))
+
 (define_predicate "msp430_symbol_operand"
   (match_code "symbol_ref")
 )
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 09bcc5b0f78..885c7aae3a5 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -1066,7 +1066,7 @@  Objective-C and Objective-C++ Dialects}.
 -mwarn-mcu @gol
 -mcode-region=  -mdata-region= @gol
 -msilicon-errata=  -msilicon-errata-warn= @gol
--mhwmult=  -minrt  -mtiny-printf}
+-mhwmult=  -minrt  -mtiny-printf  -mmax-inline-shift=}
 
 @emph{NDS32 Options}
 @gccoptlist{-mbig-endian  -mlittle-endian @gol
@@ -24728,6 +24728,19 @@  buffered before it is sent to write.
 This option requires Newlib Nano IO, so GCC must be configured with
 @samp{--enable-newlib-nano-formatted-io}.
 
+@item -mmax-inline-shift=
+@opindex mmax-inline-shift=
+This option takes an integer between 0 and 64 inclusive, and sets
+the maximum number of inline shift instructions which should be emitted to
+perform a shift operation by a constant amount.  When this value needs to be
+exceeded, an mspabi helper function is used instead.  The default value is 4.
+
+This only affects cases where a shift by multiple positions cannot be
+completed with a single instruction (e.g. all shifts >1 on the 430 ISA).
+
+Shifts of a 32-bit value are at least twice as costly, so the value passed for
+this option is divided by 2 and the resulting value used instead.
+
 @item -mcode-region=
 @itemx -mdata-region=
 @opindex mcode-region
diff --git a/gcc/testsuite/gcc.target/msp430/emulate-srli.c b/gcc/testsuite/gcc.target/msp430/emulate-srli.c
index f870d13f86b..35207b7c458 100644
--- a/gcc/testsuite/gcc.target/msp430/emulate-srli.c
+++ b/gcc/testsuite/gcc.target/msp430/emulate-srli.c
@@ -2,7 +2,7 @@ 
 /* { dg-skip-if "" { *-*-* } { "-mcpu=msp430" } { "" } } */
 /* { dg-options "-Os" } */
 /* { dg-final { scan-assembler-not "mspabi_srli" } } */
-/* { dg-final { scan-assembler "rrum" } } */
+/* { dg-final { scan-assembler "RRUM" } } */
 
 /* Ensure that HImode shifts with source operand in memory are emulated with a
    rotate instructions.  */
diff --git a/gcc/testsuite/gcc.target/msp430/max-inline-shift-430-no-opt.c b/gcc/testsuite/gcc.target/msp430/max-inline-shift-430-no-opt.c
new file mode 100644
index 00000000000..c795f7570d7
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/max-inline-shift-430-no-opt.c
@@ -0,0 +1,52 @@ 
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-mcpu=msp430x" "-mlarge" } { "" } } */
+/* { dg-options "-mcpu=msp430" } */
+/* { dg-final { scan-assembler-not "__mspabi_slli_4" } } */
+/* { dg-final { scan-assembler-not "__mspabi_sral_2" } } */
+/* { dg-final { scan-assembler "__mspabi_slli_5" } } */
+/* { dg-final { scan-assembler "__mspabi_sral_3" } } */
+
+/* Test the default value of 4 for -mmax-inline-shift has been observed.  */
+
+volatile int a1, a2, a3, a4, a5, a6, a7, a8, a9, a10, a11, a12, a13, a14, a15;
+volatile long l1, l2, l3, l4, l5, l6, l7, l8, l9, l10, l11, l12, l13, l14, l15;
+
+void
+ashift (void)
+{
+  a1 <<= 1;
+  a2 <<= 2;
+  a3 <<= 3;
+  a4 <<= 4;
+  a5 <<= 5;
+  a6 <<= 6;
+  a7 <<= 7;
+  a8 <<= 8;
+  a9 <<= 9;
+  a10 <<= 10;
+  a11 <<= 11;
+  a12 <<= 12;
+  a13 <<= 13;
+  a14 <<= 14;
+  a15 <<= 15;
+}
+
+void
+ashiftrt (void)
+{
+  l1  >>= 1;
+  l2  >>= 2;
+  l3  >>= 3;
+  l4  >>= 4;
+  l5  >>= 5;
+  l6  >>= 6;
+  l7  >>= 7;
+  l8  >>= 8;
+  l9  >>= 9;
+  l10 >>= 10;
+  l11 >>= 11;
+  l12 >>= 12;
+  l13 >>= 13;
+  l14 >>= 14;
+  l15 >>= 15;
+}
diff --git a/gcc/testsuite/gcc.target/msp430/max-inline-shift-430.c b/gcc/testsuite/gcc.target/msp430/max-inline-shift-430.c
new file mode 100644
index 00000000000..7a519eec663
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/max-inline-shift-430.c
@@ -0,0 +1,50 @@ 
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-mcpu=msp430x" "-mlarge" } { "" } } */
+/* { dg-options "-mcpu=msp430 -mmax-inline-shift=10" } */
+/* { dg-final { scan-assembler-not "__mspabi_slli_10" } } */
+/* { dg-final { scan-assembler-not "__mspabi_sral_5" } } */
+/* { dg-final { scan-assembler "__mspabi_slli_11" } } */
+/* { dg-final { scan-assembler "__mspabi_sral_6" } } */
+
+volatile int a1, a2, a3, a4, a5, a6, a7, a8, a9, a10, a11, a12, a13, a14, a15;
+volatile long l1, l2, l3, l4, l5, l6, l7, l8, l9, l10, l11, l12, l13, l14, l15;
+
+void
+ashift (void)
+{
+  a1 <<= 1;
+  a2 <<= 2;
+  a3 <<= 3;
+  a4 <<= 4;
+  a5 <<= 5;
+  a6 <<= 6;
+  a7 <<= 7;
+  a8 <<= 8;
+  a9 <<= 9;
+  a10 <<= 10;
+  a11 <<= 11;
+  a12 <<= 12;
+  a13 <<= 13;
+  a14 <<= 14;
+  a15 <<= 15;
+}
+
+void
+ashiftrt (void)
+{
+  l1  >>= 1;
+  l2  >>= 2;
+  l3  >>= 3;
+  l4  >>= 4;
+  l5  >>= 5;
+  l6  >>= 6;
+  l7  >>= 7;
+  l8  >>= 8;
+  l9  >>= 9;
+  l10 >>= 10;
+  l11 >>= 11;
+  l12 >>= 12;
+  l13 >>= 13;
+  l14 >>= 14;
+  l15 >>= 15;
+}
diff --git a/gcc/testsuite/gcc.target/msp430/max-inline-shift-430x.c b/gcc/testsuite/gcc.target/msp430/max-inline-shift-430x.c
new file mode 100644
index 00000000000..074b3af5539
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/max-inline-shift-430x.c
@@ -0,0 +1,48 @@ 
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-mcpu=msp430" } { "" } } */
+/* { dg-options "-mcpu=msp430x -mmax-inline-shift=10" } */
+/* { dg-final { scan-assembler-not "__mspabi_slli" } } */
+/* { dg-final { scan-assembler "__mspabi_sral_6" } } */
+
+volatile int a1, a2, a3, a4, a5, a6, a7, a8, a9, a10, a11, a12, a13, a14, a15;
+volatile long l1, l2, l3, l4, l5, l6, l7, l8, l9, l10, l11, l12, l13, l14, l15;
+
+void
+ashift (void)
+{
+  a1 <<= 1;
+  a2 <<= 2;
+  a3 <<= 3;
+  a4 <<= 4;
+  a5 <<= 5;
+  a6 <<= 6;
+  a7 <<= 7;
+  a8 <<= 8;
+  a9 <<= 9;
+  a10 <<= 10;
+  a11 <<= 11;
+  a12 <<= 12;
+  a13 <<= 13;
+  a14 <<= 14;
+  a15 <<= 15;
+}
+
+void
+ashiftrt (void)
+{
+  l1  >>= 1;
+  l2  >>= 2;
+  l3  >>= 3;
+  l4  >>= 4;
+  l5  >>= 5;
+  l6  >>= 6;
+  l7  >>= 7;
+  l8  >>= 8;
+  l9  >>= 9;
+  l10 >>= 10;
+  l11 >>= 11;
+  l12 >>= 12;
+  l13 >>= 13;
+  l14 >>= 14;
+  l15 >>= 15;
+}
diff --git a/libgcc/config/msp430/slli.S b/libgcc/config/msp430/slli.S
index c31e2d5db9b..b22622e0bf5 100644
--- a/libgcc/config/msp430/slli.S
+++ b/libgcc/config/msp430/slli.S
@@ -65,6 +65,21 @@  __mspabi_slli:
 	RET
 #endif
 
+#ifdef __MSP430X__
+	.section	.text.__gnu_mspabi_sllp
+1:	ADDA	#-1,R13
+	ADDA	R12,R12
+	.global	__gnu_mspabi_sllp
+__gnu_mspabi_sllp:
+	CMP	#0,R13
+	JNZ	1b
+#ifdef __MSP430X_LARGE__
+	RETA
+#else
+	RET
+#endif /* __MSP430X_LARGE__ */
+#endif /* __MSP430X__ */
+
 /* Logical Left Shift - R12:R13 -> R12:R13.  */
 
 	.section	.text.__mspabi_slll_n
diff --git a/libgcc/config/msp430/srai.S b/libgcc/config/msp430/srai.S
index d4a47fa985a..0100a368365 100644
--- a/libgcc/config/msp430/srai.S
+++ b/libgcc/config/msp430/srai.S
@@ -64,6 +64,21 @@  __mspabi_srai:
 	RET
 #endif
 
+#ifdef __MSP430X__
+	.section	.text.__gnu_mspabi_srap
+1:	ADDA	#-1,R13
+	RRAX.A	R12,R12
+	.global	__gnu_mspabi_srap
+__gnu_mspabi_srap:
+	CMP	#0,R13
+	JNZ	1b
+#ifdef __MSP430X_LARGE__
+	RETA
+#else
+	RET
+#endif /* __MSP430X_LARGE__ */
+#endif /* __MSP430X__ */
+
 /* Arithmetic Right Shift - R12:R13 -> R12:R13.  */
 
 	.section	.text.__mspabi_sral_n
diff --git a/libgcc/config/msp430/srli.S b/libgcc/config/msp430/srli.S
index 838c4bc0617..50db47c9938 100644
--- a/libgcc/config/msp430/srli.S
+++ b/libgcc/config/msp430/srli.S
@@ -66,6 +66,22 @@  __mspabi_srli:
 	RET
 #endif
 
+#ifdef __MSP430X__
+	.section	.text.__gnu_mspabi_srlp
+1:	ADDA	#-1,R13
+	CLRC
+	RRCX.A	R12,R12
+	.global	__gnu_mspabi_srlp
+__gnu_mspabi_srlp:
+	CMP	#0,R13
+	JNZ	1b
+#ifdef __MSP430X_LARGE__
+	RETA
+#else
+	RET
+#endif /* __MSP430X_LARGE__ */
+#endif /* __MSP430X__ */
+
 /* Logical Right Shift - R12:R13 -> R12:R13.  */
 
 	.section	.text.__mspabi_srll_n