From: Palmer Dabbelt <palmer@dabbelt.com>
To: gcc-patches@gcc.gnu.org, Nelson Chu <nelson.chu@sifive.com>
Cc: shihua@iscas.ac.cn, jiawei@iscas.ac.cn,
cmuellner@ventanamicro.com, gcc-patches@gcc.gnu.org,
anku.anand@gmail.com, shiyulong@iscas.ac.cn
Subject: Re: [PATCH 1/1 V5] RISC-V: Support Zmmul extension
Date: Thu, 21 Jul 2022 11:43:24 -0700 (PDT) [thread overview]
Message-ID: <mhng-23933a18-1140-48c0-88b8-32eb5000b77a@palmer-mbp2014> (raw)
In-Reply-To: <CA+yXCZDH_-c_A_oFi+hdZwu44bpqPYqiJxi3d9Zb_CLWxso7pg@mail.gmail.com>
On Thu, 21 Jul 2022 02:03:35 PDT (-0700), gcc-patches@gcc.gnu.org wrote:
> LGTM, will merge once binuils part merge.
+Nelson, in case he's already planning on handling those. If not then
they're not in my inbox, so just poke me if you want me to review them.
Also some comments on the patch below.
>
> On Wed, Jul 13, 2022 at 10:14 AM <shihua@iscas.ac.cn> wrote:
>>
>> From: LiaoShihua <shihua@iscas.ac.cn>
>>
>> gcc/ChangeLog:
>>
>> * common/config/riscv/riscv-common.cc: Add Zmmul.
>> * config/riscv/riscv-opts.h (MASK_ZMMUL): New.
>> (TARGET_ZMMUL): Ditto.
>> * config/riscv/riscv.cc (riscv_option_override):Ditto.
>> * config/riscv/riscv.md: Add Zmmul
>> * config/riscv/riscv.opt: Ditto.
>>
>> gcc/testsuite/ChangeLog:
>>
>> * gcc.target/riscv/zmmul-1.c: New test.
>> * gcc.target/riscv/zmmul-2.c: New test.
>>
>> ---
>> gcc/common/config/riscv/riscv-common.cc | 3 +++
>> gcc/config/riscv/riscv-opts.h | 3 +++
>> gcc/config/riscv/riscv.cc | 8 +++++--
>> gcc/config/riscv/riscv.md | 28 ++++++++++++------------
>> gcc/config/riscv/riscv.opt | 3 +++
>> gcc/testsuite/gcc.target/riscv/zmmul-1.c | 20 +++++++++++++++++
>> gcc/testsuite/gcc.target/riscv/zmmul-2.c | 20 +++++++++++++++++
>> 7 files changed, 69 insertions(+), 16 deletions(-)
>> create mode 100644 gcc/testsuite/gcc.target/riscv/zmmul-1.c
>> create mode 100644 gcc/testsuite/gcc.target/riscv/zmmul-2.c
>>
>> diff --git a/gcc/common/config/riscv/riscv-common.cc b/gcc/common/config/riscv/riscv-common.cc
>> index 0e5be2ce105..20acc590b30 100644
>> --- a/gcc/common/config/riscv/riscv-common.cc
>> +++ b/gcc/common/config/riscv/riscv-common.cc
>> @@ -193,6 +193,8 @@ static const struct riscv_ext_version riscv_ext_version_table[] =
>> {"zvl32768b", ISA_SPEC_CLASS_NONE, 1, 0},
>> {"zvl65536b", ISA_SPEC_CLASS_NONE, 1, 0},
>>
>> + {"zmmul", ISA_SPEC_CLASS_NONE, 1, 0},
>> +
>> /* Terminate the list. */
>> {NULL, ISA_SPEC_CLASS_NONE, 0, 0}
>> };
>> @@ -1148,6 +1150,7 @@ static const riscv_ext_flag_table_t riscv_ext_flag_table[] =
>> {"zvl32768b", &gcc_options::x_riscv_zvl_flags, MASK_ZVL32768B},
>> {"zvl65536b", &gcc_options::x_riscv_zvl_flags, MASK_ZVL65536B},
>>
>> + {"zmmul", &gcc_options::x_riscv_zm_subext, MASK_ZMMUL},
>>
>> {NULL, NULL, 0}
>> };
>> diff --git a/gcc/config/riscv/riscv-opts.h b/gcc/config/riscv/riscv-opts.h
>> index 1e153b3a6e7..9c7d69a6ea3 100644
>> --- a/gcc/config/riscv/riscv-opts.h
>> +++ b/gcc/config/riscv/riscv-opts.h
>> @@ -153,6 +153,9 @@ enum stack_protector_guard {
>> #define TARGET_ZICBOM ((riscv_zicmo_subext & MASK_ZICBOM) != 0)
>> #define TARGET_ZICBOP ((riscv_zicmo_subext & MASK_ZICBOP) != 0)
>>
>> +#define MASK_ZMMUL (1 << 0)
>> +#define TARGET_ZMMUL ((riscv_zm_subext & MASK_ZMMUL) != 0)
>> +
>> /* Bit of riscv_zvl_flags will set contintuly, N-1 bit will set if N-bit is
>> set, e.g. MASK_ZVL64B has set then MASK_ZVL32B is set, so we can use
>> popcount to caclulate the minimal VLEN. */
>> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
>> index 2e83ca07394..9ad4181f35f 100644
>> --- a/gcc/config/riscv/riscv.cc
>> +++ b/gcc/config/riscv/riscv.cc
>> @@ -4999,10 +4999,14 @@ riscv_option_override (void)
>> /* The presence of the M extension implies that division instructions
>> are present, so include them unless explicitly disabled. */
>> if (TARGET_MUL && (target_flags_explicit & MASK_DIV) == 0)
>> - target_flags |= MASK_DIV;
>> + if(!TARGET_ZMMUL)
>> + target_flags |= MASK_DIV;
Not sure if I'm missing something here, but that doesn't look right: it
would mean that "-march=rv32im_zmmul" ends up without divide
instructions. I think it's fine to just leave this as it was, we're not
setting TARGET_MUL from "-march...zmmul...", so this should all be OK.
>> else if (!TARGET_MUL && TARGET_DIV)
>> error ("%<-mdiv%> requires %<-march%> to subsume the %<M%> extension");
>> -
>> +
>> + if(TARGET_ZMMUL && !TARGET_MUL && TARGET_DIV)
>> + warning (0, "%<-mdiv%> cannot be used when %<ZMMUL%> extension is present");
That should already be getting caught by the check above, but even so
it's not quite the right error: "-march=rv32im_zmmul -mdiv" is fine,
it's just something like "-march=rv32i_zmmul -mdiv" that's the problem.
>> +
>> /* Likewise floating-point division and square root. */
>> if (TARGET_HARD_FLOAT && (target_flags_explicit & MASK_FDIV) == 0)
>> target_flags |= MASK_FDIV;
>> diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
>> index 308b64dd30d..d4e171464ea 100644
>> --- a/gcc/config/riscv/riscv.md
>> +++ b/gcc/config/riscv/riscv.md
>> @@ -763,7 +763,7 @@
>> [(set (match_operand:SI 0 "register_operand" "=r")
>> (mult:SI (match_operand:SI 1 "register_operand" " r")
>> (match_operand:SI 2 "register_operand" " r")))]
>> - "TARGET_MUL"
>> + "TARGET_ZMMUL || TARGET_MUL"
>> { return TARGET_64BIT ? "mulw\t%0,%1,%2" : "mul\t%0,%1,%2"; }
>> [(set_attr "type" "imul")
>> (set_attr "mode" "SI")])
>> @@ -772,7 +772,7 @@
>> [(set (match_operand:DI 0 "register_operand" "=r")
>> (mult:DI (match_operand:DI 1 "register_operand" " r")
>> (match_operand:DI 2 "register_operand" " r")))]
>> - "TARGET_MUL && TARGET_64BIT"
>> + "TARGET_ZMMUL || TARGET_MUL && TARGET_64BIT"
>> "mul\t%0,%1,%2"
>> [(set_attr "type" "imul")
>> (set_attr "mode" "DI")])
>> @@ -782,7 +782,7 @@
>> (mult:GPR (match_operand:GPR 1 "register_operand" " r")
>> (match_operand:GPR 2 "register_operand" " r")))
>> (label_ref (match_operand 3 "" ""))]
>> - "TARGET_MUL"
>> + "TARGET_ZMMUL || TARGET_MUL"
>> {
>> if (TARGET_64BIT && <MODE>mode == SImode)
>> {
>> @@ -827,7 +827,7 @@
>> (mult:GPR (match_operand:GPR 1 "register_operand" " r")
>> (match_operand:GPR 2 "register_operand" " r")))
>> (label_ref (match_operand 3 "" ""))]
>> - "TARGET_MUL"
>> + "TARGET_ZMMUL || TARGET_MUL"
>> {
>> if (TARGET_64BIT && <MODE>mode == SImode)
>> {
>> @@ -873,7 +873,7 @@
>> (sign_extend:DI
>> (mult:SI (match_operand:SI 1 "register_operand" " r")
>> (match_operand:SI 2 "register_operand" " r"))))]
>> - "TARGET_MUL && TARGET_64BIT"
>> + "(TARGET_ZMMUL || TARGET_MUL) && TARGET_64BIT"
>> "mulw\t%0,%1,%2"
>> [(set_attr "type" "imul")
>> (set_attr "mode" "SI")])
>> @@ -884,7 +884,7 @@
>> (match_operator:SI 3 "subreg_lowpart_operator"
>> [(mult:DI (match_operand:DI 1 "register_operand" " r")
>> (match_operand:DI 2 "register_operand" " r"))])))]
>> - "TARGET_MUL && TARGET_64BIT"
>> + "(TARGET_ZMMUL || TARGET_MUL) && TARGET_64BIT"
>> "mulw\t%0,%1,%2"
>> [(set_attr "type" "imul")
>> (set_attr "mode" "SI")])
>> @@ -902,7 +902,7 @@
>> [(set (match_operand:TI 0 "register_operand")
>> (mult:TI (any_extend:TI (match_operand:DI 1 "register_operand"))
>> (any_extend:TI (match_operand:DI 2 "register_operand"))))]
>> - "TARGET_MUL && TARGET_64BIT"
>> + "(TARGET_ZMMUL || TARGET_MUL) && TARGET_64BIT"
>> {
>> rtx low = gen_reg_rtx (DImode);
>> emit_insn (gen_muldi3 (low, operands[1], operands[2]));
>> @@ -924,7 +924,7 @@
>> (any_extend:TI
>> (match_operand:DI 2 "register_operand" " r")))
>> (const_int 64))))]
>> - "TARGET_MUL && TARGET_64BIT"
>> + "(TARGET_ZMMUL || TARGET_MUL) && TARGET_64BIT"
>> "mulh<u>\t%0,%1,%2"
>> [(set_attr "type" "imul")
>> (set_attr "mode" "DI")])
>> @@ -933,7 +933,7 @@
>> [(set (match_operand:TI 0 "register_operand")
>> (mult:TI (zero_extend:TI (match_operand:DI 1 "register_operand"))
>> (sign_extend:TI (match_operand:DI 2 "register_operand"))))]
>> - "TARGET_MUL && TARGET_64BIT"
>> + "(TARGET_ZMMUL || TARGET_MUL) && TARGET_64BIT"
>> {
>> rtx low = gen_reg_rtx (DImode);
>> emit_insn (gen_muldi3 (low, operands[1], operands[2]));
>> @@ -955,7 +955,7 @@
>> (sign_extend:TI
>> (match_operand:DI 2 "register_operand" " r")))
>> (const_int 64))))]
>> - "TARGET_MUL && TARGET_64BIT"
>> + "(TARGET_ZMMUL || TARGET_MUL) && TARGET_64BIT"
>> "mulhsu\t%0,%2,%1"
>> [(set_attr "type" "imul")
>> (set_attr "mode" "DI")])
>> @@ -966,7 +966,7 @@
>> (match_operand:SI 1 "register_operand" " r"))
>> (any_extend:DI
>> (match_operand:SI 2 "register_operand" " r"))))]
>> - "TARGET_MUL && !TARGET_64BIT"
>> + "(TARGET_ZMMUL || TARGET_MUL) && !TARGET_64BIT"
>> {
>> rtx temp = gen_reg_rtx (SImode);
>> emit_insn (gen_mulsi3 (temp, operands[1], operands[2]));
>> @@ -985,7 +985,7 @@
>> (any_extend:DI
>> (match_operand:SI 2 "register_operand" " r")))
>> (const_int 32))))]
>> - "TARGET_MUL && !TARGET_64BIT"
>> + "(TARGET_ZMMUL || TARGET_MUL) && !TARGET_64BIT"
>> "mulh<u>\t%0,%1,%2"
>> [(set_attr "type" "imul")
>> (set_attr "mode" "SI")])
>> @@ -997,7 +997,7 @@
>> (match_operand:SI 1 "register_operand" " r"))
>> (sign_extend:DI
>> (match_operand:SI 2 "register_operand" " r"))))]
>> - "TARGET_MUL && !TARGET_64BIT"
>> + "(TARGET_ZMMUL || TARGET_MUL) && !TARGET_64BIT"
>> {
>> rtx temp = gen_reg_rtx (SImode);
>> emit_insn (gen_mulsi3 (temp, operands[1], operands[2]));
>> @@ -1016,7 +1016,7 @@
>> (sign_extend:DI
>> (match_operand:SI 2 "register_operand" " r")))
>> (const_int 32))))]
>> - "TARGET_MUL && !TARGET_64BIT"
>> + "(TARGET_ZMMUL || TARGET_MUL) && !TARGET_64BIT"
>> "mulhsu\t%0,%2,%1"
>> [(set_attr "type" "imul")
>> (set_attr "mode" "SI")])
>> diff --git a/gcc/config/riscv/riscv.opt b/gcc/config/riscv/riscv.opt
>> index 9e9fe6d8ccd..f93521c1e70 100644
>> --- a/gcc/config/riscv/riscv.opt
>> +++ b/gcc/config/riscv/riscv.opt
>> @@ -212,6 +212,9 @@ int riscv_zvl_flags
>> TargetVariable
>> int riscv_zicmo_subext
>>
>> +TargetVariable
>> +int riscv_zm_subext
>> +
>> Enum
>> Name(isa_spec_class) Type(enum riscv_isa_spec_class)
>> Supported ISA specs (for use with the -misa-spec= option):
>> diff --git a/gcc/testsuite/gcc.target/riscv/zmmul-1.c b/gcc/testsuite/gcc.target/riscv/zmmul-1.c
>> new file mode 100644
>> index 00000000000..cdae2cb55df
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/riscv/zmmul-1.c
>> @@ -0,0 +1,20 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-march=rv64iafdc_zmmul -mabi=lp64" } */
>> +int foo1(int a, int b)
>> +{
>> + return a*b;
>> +}
>> +
>> +int foo2(int a, int b)
>> +{
>> + return a/b;
>> +}
>> +
>> +int foo3(int a, int b)
>> +{
>> + return a%b;
>> +}
>> +
>> +/* { dg-final { scan-assembler-times "mulw\t" 1 } } */
>> +/* { dg-final { scan-assembler-not "div\t" } } */
>> +/* { dg-final { scan-assembler-not "rem\t" } } */
This should also scan-assembler-not for divw/remw, as that's what ends
up generated for these routines on rv64im.
>> \ No newline at end of file
>> diff --git a/gcc/testsuite/gcc.target/riscv/zmmul-2.c b/gcc/testsuite/gcc.target/riscv/zmmul-2.c
>> new file mode 100644
>> index 00000000000..dc6829da92e
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/riscv/zmmul-2.c
>> @@ -0,0 +1,20 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-march=rv32iafdc_zmmul -mabi=ilp32" } */
>> +int foo1(int a, int b)
>> +{
>> + return a*b;
>> +}
>> +
>> +int foo2(int a, int b)
>> +{
>> + return a/b;
>> +}
>> +
>> +int foo3(int a, int b)
>> +{
>> + return a%b;
>> +}
>> +
>> +/* { dg-final { scan-assembler-times "mul\t" 1 } } */
>> +/* { dg-final { scan-assembler-not "div\t" } } */
>> +/* { dg-final { scan-assembler-not "rem\t" } } */
>> \ No newline at end of file
>> --
>> 2.31.1.windows.1
>>
next prev parent reply other threads:[~2022-07-21 18:43 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-13 2:13 [PATCH 0/1 " shihua
2022-07-13 2:13 ` [PATCH 1/1 " shihua
2022-07-21 9:03 ` Kito Cheng
2022-07-21 18:43 ` Palmer Dabbelt [this message]
2022-07-22 0:35 ` Kito Cheng
2022-09-05 13:47 ` Kito Cheng
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=mhng-23933a18-1140-48c0-88b8-32eb5000b77a@palmer-mbp2014 \
--to=palmer@dabbelt.com \
--cc=anku.anand@gmail.com \
--cc=cmuellner@ventanamicro.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jiawei@iscas.ac.cn \
--cc=nelson.chu@sifive.com \
--cc=shihua@iscas.ac.cn \
--cc=shiyulong@iscas.ac.cn \
/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).