Re: [PATCH] [ARC] Recognise add_n and sub_n in combine again

[Claudiu Zissulescu <claziss@gmail.com>]

Hi
It looks alright. No questions to ask, if you need me to upstream it
please ping me.

Cheers,
Claudiu

On Mon, May 15, 2017 at 7:07 PM, Graham Markall
<graham.markall@embecosm.com> wrote:
> Hi Claudiu,
>
> I ran the gcc testsuite with EZsim for NPS-400:
>
> $ ./EZsim_linux_x86_64 --version
> NPS-400 EZsim      - Version 1.9a ( 35b02d7, Nov  3 2015, 20:14:04 )
>
> both with and without the patch, and it did not introduce any new failures.
>
>
> Best regards,
> Graham.
>
> On 15/05/17 17:48, Claudiu Zissulescu wrote:
>> Hi Graham,
>>
>> May I ask if you tested this with gcc's dejagnu?
>>
>> Thanks,
>> Claudiu
>>
>> On Fri, May 12, 2017 at 9:14 PM, Graham Markall
>> <graham.markall@embecosm.com> wrote:
>>> Since the combine pass canonicalises shift-add insns using plus and
>>> ashift (as opposed to plus and mult which it previously used to do), it
>>> no longer creates *add_n or *sub_n insns, as the patterns match plus and
>>> mult only. The outcome of this is that some opportunities to generate
>>> add{1,2,3} and sub{1,2,3} instructions are missed.
>>>
>>> This change adds additional *add_n and *sub_n insns that match the
>>> plus-ashift pattern. The original *add_n and *sub_n insns are still left
>>> in, as they are sometimes generated later on by constant propagation.
>>> The idea of adding these insns is modelled on the changes in:
>>>
>>>   https://gcc.gnu.org/ml/gcc-patches/2015-05/msg01882.html
>>>
>>> which addresses a similar issue for the PA target.
>>>
>>> For the small test cases that are added, even if the combine pass misses
>>> the opportunity to generate addN or subN, constant propagation manages
>>> to do so, so the rtl of the combine pass is checked.
>>>
>>> gcc/ChangeLog:
>>>
>>>         * config/arc/arc.c (arc_print_operand): Handle constant operands.
>>>         (arc_rtx_costs): Add costs for new patterns.
>>>         * config/arc/arc.md: Additional *add_n and *sub_n patterns.
>>>         * config/arc/predicates.md: Add _1_2_3_operand predicate.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>>         * gcc.target/arc/add_n-combine.c: New.
>>>         * gcc.target/arc/sub_n-combine.c: New.
>>> ---
>>>  gcc/ChangeLog                                |  7 ++++
>>>  gcc/config/arc/arc.c                         | 20 +++++++++---
>>>  gcc/config/arc/arc.md                        | 26 +++++++++++++++
>>>  gcc/config/arc/predicates.md                 |  5 +++
>>>  gcc/testsuite/ChangeLog                      |  5 +++
>>>  gcc/testsuite/gcc.target/arc/add_n-combine.c | 48 ++++++++++++++++++++++++++++
>>>  gcc/testsuite/gcc.target/arc/sub_n-combine.c | 21 ++++++++++++
>>>  7 files changed, 128 insertions(+), 4 deletions(-)
>>>  create mode 100644 gcc/testsuite/gcc.target/arc/add_n-combine.c
>>>  create mode 100644 gcc/testsuite/gcc.target/arc/sub_n-combine.c
>>>
>>> diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
>>> index 91c28e7..42730d5 100644
>>> --- a/gcc/config/arc/arc.c
>>> +++ b/gcc/config/arc/arc.c
>>> @@ -3483,6 +3483,14 @@ arc_print_operand (FILE *file, rtx x, int code)
>>>
>>>        return;
>>>
>>> +    case 'c':
>>> +      if (GET_CODE (x) == CONST_INT)
>>> +        fprintf (file, "%d", INTVAL (x) );
>>> +      else
>>> +        output_operand_lossage ("invalid operands to %%c code");
>>> +
>>> +      return;
>>> +
>>>      case 'M':
>>>        if (GET_CODE (x) == CONST_INT)
>>>         fprintf (file, "%d",exact_log2(~INTVAL (x)) );
>>> @@ -4895,8 +4903,10 @@ arc_rtx_costs (rtx x, machine_mode mode, int outer_code,
>>>         *total = COSTS_N_INSNS (2);
>>>        return false;
>>>      case PLUS:
>>> -      if (GET_CODE (XEXP (x, 0)) == MULT
>>> -         && _2_4_8_operand (XEXP (XEXP (x, 0), 1), VOIDmode))
>>> +      if ((GET_CODE (XEXP (x, 0)) == ASHIFT
>>> +          && _1_2_3_operand (XEXP (XEXP (x, 0), 1), VOIDmode))
>>> +          || (GET_CODE (XEXP (x, 0)) == MULT
>>> +              && _2_4_8_operand (XEXP (XEXP (x, 0), 1), VOIDmode)))
>>>         {
>>>           *total += (rtx_cost (XEXP (x, 1), mode, PLUS, 0, speed)
>>>                      + rtx_cost (XEXP (XEXP (x, 0), 0), mode, PLUS, 1, speed));
>>> @@ -4904,8 +4914,10 @@ arc_rtx_costs (rtx x, machine_mode mode, int outer_code,
>>>         }
>>>        return false;
>>>      case MINUS:
>>> -      if (GET_CODE (XEXP (x, 1)) == MULT
>>> -         && _2_4_8_operand (XEXP (XEXP (x, 1), 1), VOIDmode))
>>> +      if ((GET_CODE (XEXP (x, 1)) == ASHIFT
>>> +          && _1_2_3_operand (XEXP (XEXP (x, 1), 1), VOIDmode))
>>> +          || (GET_CODE (XEXP (x, 1)) == MULT
>>> +              && _2_4_8_operand (XEXP (XEXP (x, 1), 1), VOIDmode)))
>>>         {
>>>           *total += (rtx_cost (XEXP (x, 0), mode, PLUS, 0, speed)
>>>                      + rtx_cost (XEXP (XEXP (x, 1), 0), mode, PLUS, 1, speed));
>>> diff --git a/gcc/config/arc/arc.md b/gcc/config/arc/arc.md
>>> index edb983f..ec783a0 100644
>>> --- a/gcc/config/arc/arc.md
>>> +++ b/gcc/config/arc/arc.md
>>> @@ -2995,6 +2995,19 @@
>>>
>>>  (define_insn "*add_n"
>>>    [(set (match_operand:SI 0 "dest_reg_operand" "=Rcqq,Rcw,W,W,w,w")
>>> +       (plus:SI (ashift:SI (match_operand:SI 1 "register_operand" "Rcqq,c,c,c,c,c")
>>> +                           (match_operand:SI 2 "_1_2_3_operand" ""))
>>> +                (match_operand:SI 3 "nonmemory_operand" "0,0,c,?Cal,?c,??Cal")))]
>>> +  ""
>>> +  "add%c2%? %0,%3,%1%&"
>>> +  [(set_attr "type" "shift")
>>> +   (set_attr "length" "*,4,4,8,4,8")
>>> +   (set_attr "predicable" "yes,yes,no,no,no,no")
>>> +   (set_attr "cond" "canuse,canuse,nocond,nocond,nocond,nocond")
>>> +   (set_attr "iscompact" "maybe,false,false,false,false,false")])
>>> +
>>> +(define_insn "*add_n"
>>> +  [(set (match_operand:SI 0 "dest_reg_operand" "=Rcqq,Rcw,W,W,w,w")
>>>         (plus:SI (mult:SI (match_operand:SI 1 "register_operand" "Rcqq,c,c,c,c,c")
>>>                           (match_operand:SI 2 "_2_4_8_operand" ""))
>>>                  (match_operand:SI 3 "nonmemory_operand" "0,0,c,?Cal,?c,??Cal")))]
>>> @@ -3011,6 +3024,19 @@
>>>  (define_insn "*sub_n"
>>>    [(set (match_operand:SI 0 "dest_reg_operand" "=Rcw,w,w")
>>>         (minus:SI (match_operand:SI 1 "nonmemory_operand" "0,c,?Cal")
>>> +                 (ashift:SI (match_operand:SI 2 "register_operand" "c,c,c")
>>> +                            (match_operand:SI 3 "_1_2_3_operand" ""))))]
>>> +  ""
>>> +  "sub%c3%? %0,%1,%2"
>>> +  [(set_attr "type" "shift")
>>> +   (set_attr "length" "4,4,8")
>>> +   (set_attr "predicable" "yes,no,no")
>>> +   (set_attr "cond" "canuse,nocond,nocond")
>>> +   (set_attr "iscompact" "false")])
>>> +
>>> +(define_insn "*sub_n"
>>> +  [(set (match_operand:SI 0 "dest_reg_operand" "=Rcw,w,w")
>>> +       (minus:SI (match_operand:SI 1 "nonmemory_operand" "0,c,?Cal")
>>>                   (mult:SI (match_operand:SI 2 "register_operand" "c,c,c")
>>>                            (match_operand:SI 3 "_2_4_8_operand" ""))))]
>>>    ""
>>> diff --git a/gcc/config/arc/predicates.md b/gcc/config/arc/predicates.md
>>> index 7ddec91..1f66438 100644
>>> --- a/gcc/config/arc/predicates.md
>>> +++ b/gcc/config/arc/predicates.md
>>> @@ -615,6 +615,11 @@
>>>             (match_test "TARGET_ARC700 || TARGET_EA_SET")))
>>>  )
>>>
>>> +(define_predicate "_1_2_3_operand"
>>> +  (and (match_code "const_int")
>>> +       (match_test "INTVAL (op) == 1 || INTVAL (op) == 2 || INTVAL (op) == 3"))
>>> +)
>>> +
>>>  (define_predicate "_2_4_8_operand"
>>>    (and (match_code "const_int")
>>>         (match_test "INTVAL (op) == 2 || INTVAL (op) == 4 || INTVAL (op) == 8"))
>>> diff --git a/gcc/testsuite/gcc.target/arc/add_n-combine.c b/gcc/testsuite/gcc.target/arc/add_n-combine.c
>>> new file mode 100644
>>> index 0000000..db6454f
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/arc/add_n-combine.c
>>> @@ -0,0 +1,48 @@
>>> +/* { dg-do compile }  */
>>> +/* { dg-options "-O2 -fdump-rtl-combine" }  */
>>> +
>>> +struct b1 {
>>> +      char c;
>>> +      char bg;
>>> +};
>>> +
>>> +struct bj1 {
>>> +  char bk;
>>> +  struct b1 bn[];
>>> +};
>>> +
>>> +struct b2 {
>>> +      short c;
>>> +      char bg;
>>> +};
>>> +
>>> +struct bj2 {
>>> +  short bk;
>>> +  struct b2 bn[];
>>> +};
>>> +
>>> +struct b3 {
>>> +      int c;
>>> +      char bg;
>>> +};
>>> +
>>> +struct bj3 {
>>> +  int bk;
>>> +  struct b3 bn[];
>>> +};
>>> +
>>> +
>>> +struct bj1 at1;
>>> +struct bj2 at2;
>>> +struct bj3 at3;
>>> +
>>> +int bu;
>>> +void a();
>>> +
>>> +void f() {
>>> +  a(at1.bn[bu]);
>>> +  a(at2.bn[bu]);
>>> +  a(at3.bn[bu]);
>>> +}
>>> +
>>> +/* { dg-final { scan-rtl-dump-times "\\*add_n" 3 "combine" } } */
>>> diff --git a/gcc/testsuite/gcc.target/arc/sub_n-combine.c b/gcc/testsuite/gcc.target/arc/sub_n-combine.c
>>> new file mode 100644
>>> index 0000000..4e227e4
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/arc/sub_n-combine.c
>>> @@ -0,0 +1,21 @@
>>> +/* { dg-do compile }  */
>>> +/* { dg-options "-O2 -fdump-rtl-combine" }  */
>>> +
>>> +int a;
>>> +
>>> +double b1() {
>>> +  int c = a << 1;
>>> +  return 1 - c;
>>> +}
>>> +
>>> +double b2() {
>>> +  int c = a << 2;
>>> +  return 1 - c;
>>> +}
>>> +
>>> +double b3() {
>>> +  int c = a << 3;
>>> +  return 1 - c;
>>> +}
>>> +
>>> +/* { dg-final { scan-rtl-dump-times "\\*sub_n" 3 "combine" } } */
>>> --
>>> 2.9.3
>>>
>
>