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

2020-08-25 Thread Jeff Law via Gcc-patches
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



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

2020-08-07 Thread Jozef Lawrynowicz
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 
> 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.
>   (3): New define_expand.
>   (hi3_430): New define_insn.
>   (si3_const): Likewise.
>   (ashl3_430x): Likewise.
>   (ashr3_430x): Likewise.
>   (lshr3_430x): Likewise.
>   (*bitbranch4_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/ms

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

2020-07-21 Thread Jozef Lawrynowicz
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 
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.
(3): New define_expand.
(hi3_430): New define_insn.
(si3_const): Likewise.
(ashl3_430x): Likewise.
(ashr3_430x): Likewise.
(lshr3_430x): Likewise.
(*bitbranch4_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