[RFA] Restore combine.c split point for multiply-accumulate instructions

[Richard Biener]

On Thu, May 21, 2015 at 7:38 AM, Jeff Law <law@redhat.com> wrote:
>
> find_split_point will tend to favor splitting complex insns in such a way as
> to encourage multiply-add insns.  It does this by splitting an
> unrecognizable insn at the (plus (mult)).
>
> Now that many MULTs are canonicalized as ASHIFT, that code to prefer the
> multiply-add is no longer triggering when it could/should.  This ultimately
> results in splitting at the ASHIFT rather than the containing PLUS and thus
> we generate distinct shift and add insns rather than a single shadd insn on
> the PA (and probably other architectures).
>
> This patch will treat (plus (ashift)) just like (plus (mult)) which
> encourages creation of shift-add insns.
>
> This has been bootstrapped and regression tested on x86-unknown-linux-gnu
> and with an hppa2.0w-hp-hpux11.00 cross compiler on the hppa.exp testsuite
> (full disclosure -- hppa.exp only has two tests, so it's far from
> extensive).
>
> I've also verified this is one of the changes ultimately necessary to
> resolve the code generation regressions caused by Venkat's combine.c change
> on the PA across my 300+ testfiles for a PA cross compiler.
>
> OK for the trunk?

Sounds reasonable.

Thanks,
Richard.

>
>
> Jeff
>
>
>
>
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index 490386e..250fa0a 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,5 +1,8 @@
>  2015-05-20  Jeff Law  <law@redhat.com>
>
> +       * combine.c (find_split_point): Handle ASHIFT like MULT to encourage
> +       multiply-accumulate/shift-add insn generation.
> +
>         * config/pa/pa.c (pa_print_operand): New 'o' output modifier.
>         (pa_mem_shadd_constant_p): Renamed from pa_shadd_constant_p.
>         (pa_shadd_constant_p): Allow constants for shadd insns rather
> diff --git a/gcc/combine.c b/gcc/combine.c
> index a90849e..ab6de3a 100644
> --- a/gcc/combine.c
> +++ b/gcc/combine.c
> @@ -5145,7 +5163,9 @@ find_split_point (rtx *loc, rtx_insn *insn, bool
> set_src)
>        /* Split at a multiply-accumulate instruction.  However if this is
>           the SET_SRC, we likely do not have such an instruction and it's
>           worthless to try this split.  */
> -      if (!set_src && GET_CODE (XEXP (x, 0)) == MULT)
> +      if (!set_src
> +         && (GET_CODE (XEXP (x, 0)) == MULT
> +             || GET_CODE (XEXP (x, 0)) == ASHIFT))
>          return loc;
>
>      default:
> diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
> index f20a131..bac0973 100644
> --- a/gcc/testsuite/ChangeLog
> +++ b/gcc/testsuite/ChangeLog
> @@ -1,5 +1,7 @@
>  2015-05-20  Jeff Law  <law@redhat.com>
>
> +       * gcc.target/hppa/shadd-2.c: New test.
> +
>         * gcc.target/hppa/hppa.exp: New target test driver.
>         * gcc.target/hppa/shadd-1.c: New test.
>
> diff --git a/gcc/testsuite/gcc.target/hppa/shadd-2.c
> b/gcc/testsuite/gcc.target/hppa/shadd-2.c
> new file mode 100644
> index 0000000..34708e5
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/hppa/shadd-2.c
> @@ -0,0 +1,49 @@
> +/* { dg-do compile }  */
> +/* { dg-options "-O2" }  */
> +/* { dg-final { scan-assembler-times "sh.add" 2 } }  */
> +
> +typedef struct rtx_def *rtx;
> +typedef const struct rtx_def *const_rtx;
> +enum machine_mode
> +{
> +  VOIDmode, BLKmode, CCmode, CCGCmode, CCGOCmode, CCNOmode, CCAmode,
> CCCmode,
> +    CCOmode, CCSmode, CCZmode, CCFPmode, CCFPUmode, BImode, QImode, HImode,
> +    SImode, DImode, TImode, OImode, QQmode, HQmode, SQmode, DQmode, TQmode,
> +    UQQmode, UHQmode, USQmode, UDQmode, UTQmode, HAmode, SAmode, DAmode,
> +    TAmode, UHAmode, USAmode, UDAmode, UTAmode, SFmode, DFmode, XFmode,
> +    TFmode, SDmode, DDmode, TDmode, CQImode, CHImode, CSImode, CDImode,
> +    CTImode, COImode, SCmode, DCmode, XCmode, TCmode, V2QImode, V4QImode,
> +    V2HImode, V1SImode, V8QImode, V4HImode, V2SImode, V1DImode, V16QImode,
> +    V8HImode, V4SImode, V2DImode, V1TImode, V32QImode, V16HImode, V8SImode,
> +    V4DImode, V2TImode, V64QImode, V32HImode, V16SImode, V8DImode,
> V4TImode,
> +    V2SFmode, V4SFmode, V2DFmode, V8SFmode, V4DFmode, V2TFmode, V16SFmode,
> +    V8DFmode, V4TFmode, MAX_MACHINE_MODE, NUM_MACHINE_MODES =
> MAX_MACHINE_MODE
> +};
> +struct rtx_def
> +{
> +  __extension__ enum machine_mode mode:8;
> +};
> +struct target_regs
> +{
> +  unsigned char x_hard_regno_nregs[53][MAX_MACHINE_MODE];
> +};
> +extern void oof (void);
> +extern int rhs_regno (rtx);
> +
> +extern struct target_regs default_target_regs;
> +__inline__ unsigned int
> +end_hard_regno (enum machine_mode mode, unsigned int regno)
> +{
> +  return regno +
> +    ((&default_target_regs)->x_hard_regno_nregs)[regno][(int) mode];
> +}
> +
> +void
> +note_btr_set (rtx dest, const_rtx set
> +             __attribute__ ((__unused__)), void *data)
> +{
> +  int regno, end_regno;
> +  end_regno = end_hard_regno (((dest)->mode), (rhs_regno (dest)));
> +  for (; regno < end_regno; regno++)
> +    oof ();
> +}
>