public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jeff Law <jeffreyalaw@gmail.com>
To: Matevos Mehrabyan <matevosmehrabyan@gmail.com>, gcc-patches@gcc.gnu.org
Subject: Re: RISC-V: Add divmod instruction support
Date: Fri, 28 Apr 2023 14:09:46 -0600	[thread overview]
Message-ID: <38b3a199-40c6-d7b0-64ba-5ba03d2a7d78@gmail.com> (raw)
In-Reply-To: <CALXbNshUoKPUqJiL2Nit7t4Hahg0wYMPMxcWRrwWZf8=7BADng@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1576 bytes --]



On 2/17/23 07:02, Matevos Mehrabyan via Gcc-patches wrote:
> Hi all,
> If we have division and remainder calculations with the same operands:
> 
>    a = b / c;
>    d = b % c;
> 
> We can replace the calculation of remainder with multiplication +
> subtraction, using the result from the previous division:
> 
>    a = b / c;
>    d = a * c;
>    d = b - d;
> 
> Which will be faster.
> Currently, it isn't done for RISC-V.
> 
> I've added an expander for DIVMOD which replaces 'rem' with 'mul + sub'.
> 
> Best regards,
> Matevos.
> 
> gcc/ChangeLog:
> 
>              * config/riscv/riscv.md: Added divmod expander.
> 
> gcc/testsuite/ChangeLog:
>              * gcc.target/riscv/divmod.c: New testcase.
So here's an update to the patch that I think addresses the key concerns.

Specifically use of the divmod expander is now conditional on the tuning 
info which allows for better control over using div+rem or div+mul+sub.

Given I don't know the right tunings for any implementation other than 
Veyron V1, I left them as-is.  So all the implementations will still use 
the div+rem sequence.  Obviously when we submit the Veyron V1 tunings, 
it will use div+mul+sub.  I expect other implementations will prefer 
div+mul+sub as well.

The testcase is split into two tests.  One to verify the old div+rem 
sequence, the other to test for div+mul+sub.  The latter test is 
disabled for now.  Once the first uarch has flipped tuning, we can add 
the proper tune parameter and enable the test.

Attached is the patch I committed.

Thanks and sorry for the long delay.


jeff

[-- Attachment #2: P --]
[-- Type: text/plain, Size: 7186 bytes --]

commit 065be0ffbcd676b635d492f4679e635b6ece4fe4
Author: Matevos Mehrabyan <matevosmehrabyan@gmail.com>
Date:   Fri Apr 28 14:01:30 2023 -0600

    RISC-V: Add divmod expansion support
    
    Hi all,
    If we have division and remainder calculations with the same operands:
    
      a = b / c;
      d = b % c;
    
    We can replace the calculation of remainder with multiplication +
    subtraction, using the result from the previous division:
    
      a = b / c;
      d = a * c;
      d = b - d;
    
    Which will be faster.
    Currently, it isn't done for RISC-V.
    
    I've added an expander for DIVMOD which replaces 'rem' with 'mul + sub'.
    
    Best regards,
    Matevos.
    
    gcc/ChangeLog:
    
            * config/riscv/iterators.md (only_div, paired_mod): New iterators.
            (u): Add div/udiv cases.
            * config/riscv/riscv-protos.h (riscv_use_divmod_expander): Prototype.
            * config/riscv/riscv.cc (struct riscv_tune_param): Add field for
            divmod expansion.
            (rocket_tune_info, sifive_7_tune_info): Initialize new field.
            (thead_c906_tune_info): Likewise.
            (optimize_size_tune_info): Likewise.
            (riscv_use_divmod_expander): New function.
            * config/riscv/riscv.md (<u>divmod<mode>4): New expander.
    
    gcc/testsuite/ChangeLog:
            * gcc.target/riscv/divmod-1.c: New testcase.
            * gcc.target/riscv/divmod-2.c: New testcase.

diff --git a/gcc/config/riscv/iterators.md b/gcc/config/riscv/iterators.md
index 9b767038452..1d56324df03 100644
--- a/gcc/config/riscv/iterators.md
+++ b/gcc/config/riscv/iterators.md
@@ -152,6 +152,11 @@ (define_code_iterator any_div [div udiv mod umod])
 ;; from the same template.
 (define_code_iterator any_mod [mod umod])
 
+;; These code iterators allow unsigned and signed divmod to be generated
+;; from the same template.
+(define_code_iterator only_div [div udiv])
+(define_code_attr paired_mod [(div "mod") (udiv "umod")])
+
 ;; These code iterators allow the signed and unsigned scc operations to use
 ;; the same template.
 (define_code_iterator any_gt [gt gtu])
@@ -181,6 +186,7 @@ (define_code_attr u [(sign_extend "") (zero_extend "u")
 		     (lt "") (ltu "u")
 		     (le "") (leu "u")
 		     (fix "") (unsigned_fix "u")
+		     (div "") (udiv "u")
 		     (float "") (unsigned_float "u")])
 
 ;; <su> is like <u>, but the signed form expands to "s" rather than "".
diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-protos.h
index f87661bde2c..5a927bdf1b0 100644
--- a/gcc/config/riscv/riscv-protos.h
+++ b/gcc/config/riscv/riscv-protos.h
@@ -239,4 +239,5 @@ extern const char*
 th_mempair_output_move (rtx[4], bool, machine_mode, RTX_CODE);
 #endif
 
+extern bool riscv_use_divmod_expander (void);
 #endif /* ! GCC_RISCV_PROTOS_H */
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 1529855a2b4..09a30dc260f 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -236,6 +236,7 @@ struct riscv_tune_param
   unsigned short memory_cost;
   unsigned short fmv_cost;
   bool slow_unaligned_access;
+  bool use_divmod_expansion;
 };
 
 /* Information about one micro-arch we know about.  */
@@ -323,6 +324,7 @@ static const struct riscv_tune_param rocket_tune_info = {
   5,						/* memory_cost */
   8,						/* fmv_cost */
   true,						/* slow_unaligned_access */
+  false,					/* use_divmod_expansion */
 };
 
 /* Costs to use when optimizing for Sifive 7 Series.  */
@@ -337,6 +339,7 @@ static const struct riscv_tune_param sifive_7_tune_info = {
   3,						/* memory_cost */
   8,						/* fmv_cost */
   true,						/* slow_unaligned_access */
+  false,					/* use_divmod_expansion */
 };
 
 /* Costs to use when optimizing for T-HEAD c906.  */
@@ -351,6 +354,7 @@ static const struct riscv_tune_param thead_c906_tune_info = {
   5,            /* memory_cost */
   8,		/* fmv_cost */
   false,            /* slow_unaligned_access */
+  false		/* use_divmod_expansion */
 };
 
 /* Costs to use when optimizing for size.  */
@@ -365,6 +369,7 @@ static const struct riscv_tune_param optimize_size_tune_info = {
   2,						/* memory_cost */
   8,						/* fmv_cost */
   false,					/* slow_unaligned_access */
+  false,					/* use_divmod_expansion */
 };
 
 static tree riscv_handle_fndecl_attribute (tree *, tree, tree, int, bool *);
@@ -7210,6 +7215,16 @@ riscv_lshift_subword (machine_mode mode, rtx value, rtx shift,
 						  gen_lowpart (QImode, shift)));
 }
 
+/* Return TRUE if we should use the divmod expander, FALSE otherwise.  This
+   allows the behavior to be tuned for specific implementations as well as
+   when optimizing for size.  */
+
+bool
+riscv_use_divmod_expander (void)
+{
+  return tune_param->use_divmod_expansion;
+}
+
 /* Initialize the GCC target structure.  */
 #undef TARGET_ASM_ALIGNED_HI_OP
 #define TARGET_ASM_ALIGNED_HI_OP "\t.half\t"
diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index 376a8831820..c508ee3ad89 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -1063,6 +1063,22 @@ (define_insn "<optab>di3"
   [(set_attr "type" "idiv")
    (set_attr "mode" "DI")])
 
+(define_expand "<u>divmod<mode>4"
+  [(parallel
+     [(set (match_operand:GPR 0 "register_operand")
+           (only_div:GPR (match_operand:GPR 1 "register_operand")
+                         (match_operand:GPR 2 "register_operand")))
+      (set (match_operand:GPR 3 "register_operand")
+           (<paired_mod>:GPR (match_dup 1) (match_dup 2)))])]
+  "TARGET_DIV && riscv_use_divmod_expander ()"
+  {
+      rtx tmp = gen_reg_rtx (<MODE>mode);
+      emit_insn (gen_<u>div<GPR:mode>3 (operands[0], operands[1], operands[2]));
+      emit_insn (gen_mul<GPR:mode>3 (tmp, operands[0], operands[2]));
+      emit_insn (gen_sub<GPR:mode>3 (operands[3], operands[1], tmp));
+      DONE;
+  })
+
 (define_insn "*<optab>si3_extended"
   [(set (match_operand:DI                 0 "register_operand" "=r")
 	(sign_extend:DI
diff --git a/gcc/testsuite/gcc.target/riscv/divmod-1.c b/gcc/testsuite/gcc.target/riscv/divmod-1.c
new file mode 100644
index 00000000000..9706611b500
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/divmod-1.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+
+void
+foo(int a, int b, int *c, int *d)
+{
+   *c = a / b;
+   *d = a % b;
+}
+
+/* { dg-final { scan-assembler-times "\tdiv" 1 } } */
+/* { dg-final { scan-assembler-times "\trem" 1} } */
diff --git a/gcc/testsuite/gcc.target/riscv/divmod-2.c b/gcc/testsuite/gcc.target/riscv/divmod-2.c
new file mode 100644
index 00000000000..dfd319e52c0
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/divmod-2.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* Skip this everywhere for now.  Once we have a target with
+   divmod enabled, only skip for -O0, -O1, -Og, -Oz, -Os.  */
+/* { dg-skip-if "" { *-*-* } { } } */
+
+void
+foo(int a, int b, int *c, int *d)
+{
+   *c = a / b;
+   *d = a % b;
+}
+
+/* { dg-final { scan-assembler-not "\trem" } } */
+/* { dg-final { scan-assembler-times "\tdiv" 1 } } */
+/* { dg-final { scan-assembler-times "\tmul" 1 } } */
+/* { dg-final { scan-assembler-times "\tsub" 1 } } */

      parent reply	other threads:[~2023-04-28 20:09 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-17 14:02 Matevos Mehrabyan
2023-02-18 18:26 ` Palmer Dabbelt
2023-02-18 18:42   ` Andrew Pinski
2023-02-18 19:26     ` Palmer Dabbelt
2023-02-18 19:31     ` Maciej W. Rozycki
2023-02-18 20:57       ` Prathamesh Kulkarni
2023-02-18 21:07       ` Jeff Law
2023-02-19  1:14         ` Maciej W. Rozycki
2023-02-20  8:11           ` Richard Biener
2023-02-20 13:32             ` Alexander Monakov
2023-02-28 12:54               ` Maciej W. Rozycki
2023-02-18 21:06   ` Jeff Law
2023-02-18 21:30     ` Palmer Dabbelt
2023-02-18 21:57       ` Jeff Law
2023-02-20  1:27       ` Andrew Waterman
2023-04-28 20:09 ` Jeff Law [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=38b3a199-40c6-d7b0-64ba-5ba03d2a7d78@gmail.com \
    --to=jeffreyalaw@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=matevosmehrabyan@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).