Re: [PATCH][ARM] Remove support for MULS

Thu, 19 Sep 2019 02:52:36 -0700 

On 18/09/2019 17:31, Kyrill Tkachov wrote:

Hi Wilco,

On 9/9/19 6:07 PM, Wilco Dijkstra wrote:

ping


Remove various MULS/MLAS patterns which are enabled when optimizing for
 size.  However the codesize gain from these patterns is so minimal that
 there is no point in keeping them.
I disagree. If they still trigger and generate better code than without we should keep them. 

What kind of code is *common* varies greatly from user to user.
Also, the main reason for restricting their use was that in the 'olden days', when we had multi-cycle implementations of the multiply instructions with short-circuit fast termination when the result was completed, the flag setting variants would never short-circuit.
These days we have fixed cycle counts for multiply instructions, so this is no-longer a penalty. In the thumb2 case in particular we can often reduce mul-cmp (6 bytes) to muls (2 bytes), that's a 66% saving on this sequence and definitely worth exploiting when we can, even if it's not all that common.
In fact, give the latest architectural implementations, I think we should look again at re-enabling these when doing normal optimization as well. 

R.



Thanks,

Kyrill



 Bootstrap OK on armhf, regress passes.

 ChangeLog:
 2019-09-03  Wilco Dijkstra  <wdijk...@arm.com>

         * config/arm/arm.md (mulsi3_compare0): Remove pattern.
         (mulsi3_compare0_v6): Likewise.
         (mulsi_compare0_scratch): Likewise.
         (mulsi_compare0_scratch_v6): Likewise.
         (mulsi3addsi_compare0): Likewise.
         (mulsi3addsi_compare0_v6): Likewise.
         (mulsi3addsi_compare0_scratch): Likewise.
         (mulsi3addsi_compare0_scratch_v6): Likewise.
         * config/arm/thumb2.md (thumb2_mulsi_short_compare0): Remove pattern. 
         (thumb2_mulsi_short_compare0_scratch): Likewise.

 --
 diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
 index 738d42fd164f117f1dec1108a824d984ccd70d09..66dafdc47b7cfc37c131764e482d47bcaab90538 100644 
 --- a/gcc/config/arm/arm.md
 +++ b/gcc/config/arm/arm.md
 @@ -1618,60 +1618,6 @@ (define_insn "*arm_mulsi3_v6"
     (set_attr "predicable_short_it" "yes,yes,no")]
  )

 -(define_insn "*mulsi3_compare0"
 -  [(set (reg:CC_NOOV CC_REGNUM)
 -       (compare:CC_NOOV (mult:SI
 -                         (match_operand:SI 2 "s_register_operand" "r,r")  -                         (match_operand:SI 1 "s_register_operand" "%0,r")) 
 -                        (const_int 0)))
 -   (set (match_operand:SI 0 "s_register_operand" "=&r,&r")
 -       (mult:SI (match_dup 2) (match_dup 1)))]
 -  "TARGET_ARM && !arm_arch6"
 -  "muls%?\\t%0, %2, %1"
 -  [(set_attr "conds" "set")
 -   (set_attr "type" "muls")]
 -)
 -
 -(define_insn "*mulsi3_compare0_v6"
 -  [(set (reg:CC_NOOV CC_REGNUM)
 -       (compare:CC_NOOV (mult:SI
 -                         (match_operand:SI 2 "s_register_operand" "r")
 -                         (match_operand:SI 1 "s_register_operand" "r"))
 -                        (const_int 0)))
 -   (set (match_operand:SI 0 "s_register_operand" "=r")
 -       (mult:SI (match_dup 2) (match_dup 1)))]
 -  "TARGET_ARM && arm_arch6 && optimize_size"
 -  "muls%?\\t%0, %2, %1"
 -  [(set_attr "conds" "set")
 -   (set_attr "type" "muls")]
 -)
 -
 -(define_insn "*mulsi_compare0_scratch"
 -  [(set (reg:CC_NOOV CC_REGNUM)
 -       (compare:CC_NOOV (mult:SI
 -                         (match_operand:SI 2 "s_register_operand" "r,r")  -                         (match_operand:SI 1 "s_register_operand" "%0,r")) 
 -                        (const_int 0)))
 -   (clobber (match_scratch:SI 0 "=&r,&r"))]
 -  "TARGET_ARM && !arm_arch6"
 -  "muls%?\\t%0, %2, %1"
 -  [(set_attr "conds" "set")
 -   (set_attr "type" "muls")]
 -)
 -
 -(define_insn "*mulsi_compare0_scratch_v6"
 -  [(set (reg:CC_NOOV CC_REGNUM)
 -       (compare:CC_NOOV (mult:SI
 -                         (match_operand:SI 2 "s_register_operand" "r")
 -                         (match_operand:SI 1 "s_register_operand" "r"))
 -                        (const_int 0)))
 -   (clobber (match_scratch:SI 0 "=r"))]
 -  "TARGET_ARM && arm_arch6 && optimize_size"
 -  "muls%?\\t%0, %2, %1"
 -  [(set_attr "conds" "set")
 -   (set_attr "type" "muls")]
 -)
 -
  ;; Unnamed templates to match MLA instruction.

  (define_insn "*mulsi3addsi"
 @@ -1698,70 +1644,6 @@ (define_insn "*mulsi3addsi_v6"
     (set_attr "predicable" "yes")]
  )

 -(define_insn "*mulsi3addsi_compare0"
 -  [(set (reg:CC_NOOV CC_REGNUM)
 -       (compare:CC_NOOV
 -        (plus:SI (mult:SI
 -                  (match_operand:SI 2 "s_register_operand" "r,r,r,r")
 -                  (match_operand:SI 1 "s_register_operand" "%0,r,0,r"))
 -                 (match_operand:SI 3 "s_register_operand" "r,r,0,0"))
 -        (const_int 0)))
 -   (set (match_operand:SI 0 "s_register_operand" "=&r,&r,&r,&r")
 -       (plus:SI (mult:SI (match_dup 2) (match_dup 1))
 -                (match_dup 3)))]
 -  "TARGET_ARM && arm_arch6"
 -  "mlas%?\\t%0, %2, %1, %3"
 -  [(set_attr "conds" "set")
 -   (set_attr "type" "mlas")]
 -)
 -
 -(define_insn "*mulsi3addsi_compare0_v6"
 -  [(set (reg:CC_NOOV CC_REGNUM)
 -       (compare:CC_NOOV
 -        (plus:SI (mult:SI
 -                  (match_operand:SI 2 "s_register_operand" "r")
 -                  (match_operand:SI 1 "s_register_operand" "r"))
 -                 (match_operand:SI 3 "s_register_operand" "r"))
 -        (const_int 0)))
 -   (set (match_operand:SI 0 "s_register_operand" "=r")
 -       (plus:SI (mult:SI (match_dup 2) (match_dup 1))
 -                (match_dup 3)))]
 -  "TARGET_ARM && arm_arch6 && optimize_size"
 -  "mlas%?\\t%0, %2, %1, %3"
 -  [(set_attr "conds" "set")
 -   (set_attr "type" "mlas")]
 -)
 -
 -(define_insn "*mulsi3addsi_compare0_scratch"
 -  [(set (reg:CC_NOOV CC_REGNUM)
 -       (compare:CC_NOOV
 -        (plus:SI (mult:SI
 -                  (match_operand:SI 2 "s_register_operand" "r,r,r,r")
 -                  (match_operand:SI 1 "s_register_operand" "%0,r,0,r"))
 -                 (match_operand:SI 3 "s_register_operand" "?r,r,0,0"))
 -        (const_int 0)))
 -   (clobber (match_scratch:SI 0 "=&r,&r,&r,&r"))]
 -  "TARGET_ARM && !arm_arch6"
 -  "mlas%?\\t%0, %2, %1, %3"
 -  [(set_attr "conds" "set")
 -   (set_attr "type" "mlas")]
 -)
 -
 -(define_insn "*mulsi3addsi_compare0_scratch_v6"
 -  [(set (reg:CC_NOOV CC_REGNUM)
 -       (compare:CC_NOOV
 -        (plus:SI (mult:SI
 -                  (match_operand:SI 2 "s_register_operand" "r")
 -                  (match_operand:SI 1 "s_register_operand" "r"))
 -                 (match_operand:SI 3 "s_register_operand" "r"))
 -        (const_int 0)))
 -   (clobber (match_scratch:SI 0 "=r"))]
 -  "TARGET_ARM && arm_arch6 && optimize_size"
 -  "mlas%?\\t%0, %2, %1, %3"
 -  [(set_attr "conds" "set")
 -   (set_attr "type" "mlas")]
 -)
 -
  (define_insn "*mulsi3subsi"
    [(set (match_operand:SI 0 "s_register_operand" "=r")
          (minus:SI
 diff --git a/gcc/config/arm/thumb2.md b/gcc/config/arm/thumb2.md
 index 6ccc875e2b4e7b8ce256e52da966dfe220c6f5d6..8e26689b66263e7304a0da6163ceccfb4483d3e7 100644 
 --- a/gcc/config/arm/thumb2.md
 +++ b/gcc/config/arm/thumb2.md
 @@ -1381,31 +1381,6 @@ (define_insn "*thumb2_mulsi_short"
     (set_attr "length" "2")
     (set_attr "type" "muls")])

 -(define_insn "*thumb2_mulsi_short_compare0"
 -  [(set (reg:CC_NOOV CC_REGNUM)
 -        (compare:CC_NOOV
 -         (mult:SI (match_operand:SI 1 "register_operand" "%0")
 -                 (match_operand:SI 2 "register_operand" "l"))
 -         (const_int 0)))
 -   (set (match_operand:SI 0 "register_operand" "=l")
 -       (mult:SI (match_dup 1) (match_dup 2)))]
 -  "TARGET_THUMB2 && optimize_size"
 -  "muls\\t%0, %2, %0"
 -  [(set_attr "length" "2")
 -   (set_attr "type" "muls")])
 -
 -(define_insn "*thumb2_mulsi_short_compare0_scratch"
 -  [(set (reg:CC_NOOV CC_REGNUM)
 -        (compare:CC_NOOV
 -         (mult:SI (match_operand:SI 1 "register_operand" "%0")
 -                 (match_operand:SI 2 "register_operand" "l"))
 -         (const_int 0)))
 -   (clobber (match_scratch:SI 0 "=l"))]
 -  "TARGET_THUMB2 && optimize_size"
 -  "muls\\t%0, %2, %0"
 -  [(set_attr "length" "2")
 -   (set_attr "type" "muls")])
 -
  (define_insn "*thumb2_cbz"
    [(set (pc) (if_then_else
                (eq (match_operand:SI 0 "s_register_operand" "l,?r")

Reply via email to