public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 2/3 v2] xtensa: Add 'adddi3' and 'subdi3' insn patterns
       [not found] <95b8b130-caef-12c8-b247-25ec7dbf0ac3.ref@yahoo.co.jp>
@ 2023-05-30  9:50 ` Takayuki 'January June' Suwa
  2023-05-31  6:02   ` Max Filippov
  0 siblings, 1 reply; 6+ messages in thread
From: Takayuki 'January June' Suwa @ 2023-05-30  9:50 UTC (permalink / raw)
  To: GCC Patches; +Cc: Max Filippov

Resubmitting the correct one due to a mistake in merging order of fixes.
---
More optimized than the default RTL generation.

gcc/ChangeLog:

	* config/xtensa/xtensa.md (adddi3, subdi3):
	New RTL generation patterns implemented according to the instruc-
	tion idioms described in the Xtensa ISA reference manual (p. 600).
---
 gcc/config/xtensa/xtensa.md | 52 +++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/gcc/config/xtensa/xtensa.md b/gcc/config/xtensa/xtensa.md
index eda1353894b..6882baaedfd 100644
--- a/gcc/config/xtensa/xtensa.md
+++ b/gcc/config/xtensa/xtensa.md
@@ -190,6 +190,32 @@
    (set_attr "mode"	"SI")
    (set_attr "length"	"3")])
 
+(define_expand "adddi3"
+  [(set (match_operand:DI 0 "register_operand")
+	(plus:DI (match_operand:DI 1 "register_operand")
+		 (match_operand:DI 2 "register_operand")))]
+  ""
+{
+  rtx lo_dest, hi_dest, lo_op0, hi_op0, lo_op1, hi_op1;
+  rtx_code_label *label;
+  lo_dest = gen_lowpart (SImode, operands[0]);
+  hi_dest = gen_highpart (SImode, operands[0]);
+  lo_op0 = gen_lowpart (SImode, operands[1]);
+  hi_op0 = gen_highpart (SImode, operands[1]);
+  lo_op1 = gen_lowpart (SImode, operands[2]);
+  hi_op1 = gen_highpart (SImode, operands[2]);
+  if (rtx_equal_p (lo_dest, lo_op1))
+    FAIL;
+  emit_clobber (operands[0]);
+  emit_insn (gen_addsi3 (lo_dest, lo_op0, lo_op1));
+  emit_insn (gen_addsi3 (hi_dest, hi_op0, hi_op1));
+  emit_cmp_and_jump_insns (lo_dest, lo_op1, GEU, const0_rtx,
+			   SImode, true, label = gen_label_rtx ());
+  emit_insn (gen_addsi3 (hi_dest, hi_dest, const1_rtx));
+  emit_label (label);
+  DONE;
+})
+
 (define_insn "addsf3"
   [(set (match_operand:SF 0 "register_operand" "=f")
 	(plus:SF (match_operand:SF 1 "register_operand" "%f")
@@ -237,6 +263,32 @@
 		      (const_int 5)
 		      (const_int 6)))])
 
+(define_expand "subdi3"
+  [(set (match_operand:DI 0 "register_operand")
+	(minus:DI (match_operand:DI 1 "register_operand")
+		  (match_operand:DI 2 "register_operand")))]
+  ""
+{
+  rtx lo_dest, hi_dest, lo_op0, hi_op0, lo_op1, hi_op1;
+  rtx_code_label *label;
+  lo_dest = gen_lowpart (SImode, operands[0]);
+  hi_dest = gen_highpart (SImode, operands[0]);
+  lo_op0 = gen_lowpart (SImode, operands[1]);
+  hi_op0 = gen_highpart (SImode, operands[1]);
+  lo_op1 = gen_lowpart (SImode, operands[2]);
+  hi_op1 = gen_highpart (SImode, operands[2]);
+  if (rtx_equal_p (lo_op0, lo_op1))
+    FAIL;
+  emit_clobber (operands[0]);
+  emit_insn (gen_subsi3 (lo_dest, lo_op0, lo_op1));
+  emit_insn (gen_subsi3 (hi_dest, hi_op0, hi_op1));
+  emit_cmp_and_jump_insns (lo_op0, lo_op1, GEU, const0_rtx,
+			   SImode, true, label = gen_label_rtx ());
+  emit_insn (gen_addsi3 (hi_dest, hi_dest, constm1_rtx));
+  emit_label (label);
+  DONE;
+})
+
 (define_insn "subsf3"
   [(set (match_operand:SF 0 "register_operand" "=f")
 	(minus:SF (match_operand:SF 1 "register_operand" "f")
-- 
2.30.2

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/3 v2] xtensa: Add 'adddi3' and 'subdi3' insn patterns
  2023-05-30  9:50 ` [PATCH 2/3 v2] xtensa: Add 'adddi3' and 'subdi3' insn patterns Takayuki 'January June' Suwa
@ 2023-05-31  6:02   ` Max Filippov
  2023-06-01  6:01     ` [PATCH 2/3 v3] " Takayuki 'January June' Suwa
  0 siblings, 1 reply; 6+ messages in thread
From: Max Filippov @ 2023-05-31  6:02 UTC (permalink / raw)
  To: Takayuki 'January June' Suwa; +Cc: GCC Patches

On Tue, May 30, 2023 at 2:50 AM Takayuki 'January June' Suwa
<jjsuwa_sys3175@yahoo.co.jp> wrote:
>
> Resubmitting the correct one due to a mistake in merging order of fixes.
> ---
> More optimized than the default RTL generation.
>
> gcc/ChangeLog:
>
>         * config/xtensa/xtensa.md (adddi3, subdi3):
>         New RTL generation patterns implemented according to the instruc-
>         tion idioms described in the Xtensa ISA reference manual (p. 600).
> ---
>  gcc/config/xtensa/xtensa.md | 52 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
>
> diff --git a/gcc/config/xtensa/xtensa.md b/gcc/config/xtensa/xtensa.md
> index eda1353894b..6882baaedfd 100644
> --- a/gcc/config/xtensa/xtensa.md
> +++ b/gcc/config/xtensa/xtensa.md
> @@ -190,6 +190,32 @@
>     (set_attr "mode"    "SI")
>     (set_attr "length"  "3")])
>
> +(define_expand "adddi3"
> +  [(set (match_operand:DI 0 "register_operand")
> +       (plus:DI (match_operand:DI 1 "register_operand")
> +                (match_operand:DI 2 "register_operand")))]
> +  ""
> +{
> +  rtx lo_dest, hi_dest, lo_op0, hi_op0, lo_op1, hi_op1;
> +  rtx_code_label *label;
> +  lo_dest = gen_lowpart (SImode, operands[0]);
> +  hi_dest = gen_highpart (SImode, operands[0]);
> +  lo_op0 = gen_lowpart (SImode, operands[1]);
> +  hi_op0 = gen_highpart (SImode, operands[1]);
> +  lo_op1 = gen_lowpart (SImode, operands[2]);
> +  hi_op1 = gen_highpart (SImode, operands[2]);
> +  if (rtx_equal_p (lo_dest, lo_op1))
> +    FAIL;

With this condition I see the following source

unsigned long long foo(unsigned long long a, unsigned long long b)
{
       return a + b;
}

turns to (expected)

       .global foo
       .type   foo, @function
foo:
       add.n   a2, a2, a4
       add.n   a3, a3, a5
       bgeu    a2, a4, .L2
       addi.n  a3, a3, 1
.L2:
       ret.n

but

unsigned long long foo(unsigned long long a, unsigned long long b)
{
       return b + a;
}

has an extra instruction:

       .global foo
       .type   foo, @function
foo:
       mov.n   a9, a2
       add.n   a2, a4, a2
       add.n   a3, a5, a3
       bgeu    a2, a9, .L2
       addi.n  a3, a3, 1
.L2:
       ret.n

I though that maybe the following would help (plus using
lo_cmp in the emit_cmp_and_jump_insns below):

  if (!rtx_equal_p (lo_dest, lo_op0))
   lo_cmp = lo_op0;
 else if (!rtx_equal_p (lo_dest, lo_op1))
   lo_cmp = lo_op1;
 else
   FAIL;

but to my surprise it doesn't.

> +  emit_clobber (operands[0]);

Why is this clobber needed?

> +  emit_insn (gen_addsi3 (lo_dest, lo_op0, lo_op1));
> +  emit_insn (gen_addsi3 (hi_dest, hi_op0, hi_op1));
> +  emit_cmp_and_jump_insns (lo_dest, lo_op1, GEU, const0_rtx,
> +                          SImode, true, label = gen_label_rtx ());
> +  emit_insn (gen_addsi3 (hi_dest, hi_dest, const1_rtx));
> +  emit_label (label);
> +  DONE;
> +})
> +
>  (define_insn "addsf3"
>    [(set (match_operand:SF 0 "register_operand" "=f")
>         (plus:SF (match_operand:SF 1 "register_operand" "%f")
> @@ -237,6 +263,32 @@
>                       (const_int 5)
>                       (const_int 6)))])
>
> +(define_expand "subdi3"
> +  [(set (match_operand:DI 0 "register_operand")
> +       (minus:DI (match_operand:DI 1 "register_operand")
> +                 (match_operand:DI 2 "register_operand")))]
> +  ""
> +{
> +  rtx lo_dest, hi_dest, lo_op0, hi_op0, lo_op1, hi_op1;
> +  rtx_code_label *label;
> +  lo_dest = gen_lowpart (SImode, operands[0]);
> +  hi_dest = gen_highpart (SImode, operands[0]);
> +  lo_op0 = gen_lowpart (SImode, operands[1]);
> +  hi_op0 = gen_highpart (SImode, operands[1]);
> +  lo_op1 = gen_lowpart (SImode, operands[2]);
> +  hi_op1 = gen_highpart (SImode, operands[2]);
> +  if (rtx_equal_p (lo_op0, lo_op1))
> +    FAIL;

I believe that for the emit_cmp_and_jump_insns below
the check here should look like this:

if (rtx_equal_p (lo_dest, lo_op0) || rtx_equal_p (lo_dest, lo_op1))

But maybe drop this check and use the following instead?

 emit_insn (gen_subsi3 (hi_dest, hi_op0, hi_op1));
 emit_cmp_and_jump_insns (lo_op0, lo_op1, GEU, const0_rtx,
                          SImode, true, label = gen_label_rtx ());
 emit_insn (gen_addsi3 (hi_dest, hi_dest, constm1_rtx));
 emit_label (label);
 emit_insn (gen_subsi3 (lo_dest, lo_op0, lo_op1));

> +  emit_clobber (operands[0]);

Why is this clobber needed?

> +  emit_insn (gen_subsi3 (lo_dest, lo_op0, lo_op1));
> +  emit_insn (gen_subsi3 (hi_dest, hi_op0, hi_op1));
> +  emit_cmp_and_jump_insns (lo_op0, lo_op1, GEU, const0_rtx,
> +                          SImode, true, label = gen_label_rtx ());
> +  emit_insn (gen_addsi3 (hi_dest, hi_dest, constm1_rtx));
> +  emit_label (label);
> +  DONE;
> +})
> +
>  (define_insn "subsf3"
>    [(set (match_operand:SF 0 "register_operand" "=f")
>         (minus:SF (match_operand:SF 1 "register_operand" "f")
> --
> 2.30.2

-- 
Thanks.
-- Max

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/3 v3] xtensa: Add 'adddi3' and 'subdi3' insn patterns
  2023-05-31  6:02   ` Max Filippov
@ 2023-06-01  6:01     ` Takayuki 'January June' Suwa
  2023-06-01 14:16       ` Max Filippov
  2023-06-01 14:20       ` Max Filippov
  0 siblings, 2 replies; 6+ messages in thread
From: Takayuki 'January June' Suwa @ 2023-06-01  6:01 UTC (permalink / raw)
  To: GCC Patches; +Cc: Max Filippov

On 2023/05/31 15:02, Max Filippov wrote:
Hi!

> On Tue, May 30, 2023 at 2:50 AM Takayuki 'January June' Suwa
> <jjsuwa_sys3175@yahoo.co.jp> wrote:
>>
>> Resubmitting the correct one due to a mistake in merging order of fixes.
>> ---
>> More optimized than the default RTL generation.
>>
>> gcc/ChangeLog:
>>
>>         * config/xtensa/xtensa.md (adddi3, subdi3):
>>         New RTL generation patterns implemented according to the instruc-
>>         tion idioms described in the Xtensa ISA reference manual (p. 600).
>> ---
>>  gcc/config/xtensa/xtensa.md | 52 +++++++++++++++++++++++++++++++++++++
>>  1 file changed, 52 insertions(+)
>>
>> diff --git a/gcc/config/xtensa/xtensa.md b/gcc/config/xtensa/xtensa.md
>> index eda1353894b..6882baaedfd 100644
>> --- a/gcc/config/xtensa/xtensa.md
>> +++ b/gcc/config/xtensa/xtensa.md
>> @@ -190,6 +190,32 @@
>>     (set_attr "mode"    "SI")
>>     (set_attr "length"  "3")])
>>
>> +(define_expand "adddi3"
>> +  [(set (match_operand:DI 0 "register_operand")
>> +       (plus:DI (match_operand:DI 1 "register_operand")
>> +                (match_operand:DI 2 "register_operand")))]
>> +  ""
>> +{
>> +  rtx lo_dest, hi_dest, lo_op0, hi_op0, lo_op1, hi_op1;
>> +  rtx_code_label *label;
>> +  lo_dest = gen_lowpart (SImode, operands[0]);
>> +  hi_dest = gen_highpart (SImode, operands[0]);
>> +  lo_op0 = gen_lowpart (SImode, operands[1]);
>> +  hi_op0 = gen_highpart (SImode, operands[1]);
>> +  lo_op1 = gen_lowpart (SImode, operands[2]);
>> +  hi_op1 = gen_highpart (SImode, operands[2]);
>> +  if (rtx_equal_p (lo_dest, lo_op1))
>> +    FAIL;
> 
> With this condition I see the following source
> 
> unsigned long long foo(unsigned long long a, unsigned long long b)
> {
>        return a + b;
> }
> 
> turns to (expected)
> 
>        .global foo
>        .type   foo, @function
> foo:
>        add.n   a2, a2, a4
>        add.n   a3, a3, a5
>        bgeu    a2, a4, .L2
>        addi.n  a3, a3, 1
> .L2:
>        ret.n
> 
> but
> 
> unsigned long long foo(unsigned long long a, unsigned long long b)
> {
>        return b + a;
> }
> 
> has an extra instruction:
> 
>        .global foo
>        .type   foo, @function
> foo:
>        mov.n   a9, a2
>        add.n   a2, a4, a2
>        add.n   a3, a5, a3
>        bgeu    a2, a9, .L2
>        addi.n  a3, a3, 1
> .L2:
>        ret.n
> 
> I though that maybe the following would help (plus using
> lo_cmp in the emit_cmp_and_jump_insns below):
> 
>   if (!rtx_equal_p (lo_dest, lo_op0))
>    lo_cmp = lo_op0;
>  else if (!rtx_equal_p (lo_dest, lo_op1))
>    lo_cmp = lo_op1;
>  else
>    FAIL;
> 
> but to my surprise it doesn't.

As you may have noticed, at the time of RTL generation both of the above-
mentioned are almost the same (only a and b have been swapped).
Whether or not there are extra registers is determined at a later stage,
so there is very little that can be done about it at (define_expand).

I thought as above, but when I looked at the generated RTL again, I noticed
that I could somehow make a decision based on the order of the generated
pseudo-register numbers.

> 
>> +  emit_clobber (operands[0]);
> 
> Why is this clobber needed?

Apparently there is no need to clobber explicitly (because even if omitted,
it will appear in the generated result).

> 
>> +  emit_insn (gen_addsi3 (lo_dest, lo_op0, lo_op1));
>> +  emit_insn (gen_addsi3 (hi_dest, hi_op0, hi_op1));
>> +  emit_cmp_and_jump_insns (lo_dest, lo_op1, GEU, const0_rtx,
>> +                          SImode, true, label = gen_label_rtx ());
>> +  emit_insn (gen_addsi3 (hi_dest, hi_dest, const1_rtx));
>> +  emit_label (label);
>> +  DONE;
>> +})
>> +
>>  (define_insn "addsf3"
>>    [(set (match_operand:SF 0 "register_operand" "=f")
>>         (plus:SF (match_operand:SF 1 "register_operand" "%f")
>> @@ -237,6 +263,32 @@
>>                       (const_int 5)
>>                       (const_int 6)))])
>>
>> +(define_expand "subdi3"
>> +  [(set (match_operand:DI 0 "register_operand")
>> +       (minus:DI (match_operand:DI 1 "register_operand")
>> +                 (match_operand:DI 2 "register_operand")))]
>> +  ""
>> +{
>> +  rtx lo_dest, hi_dest, lo_op0, hi_op0, lo_op1, hi_op1;
>> +  rtx_code_label *label;
>> +  lo_dest = gen_lowpart (SImode, operands[0]);
>> +  hi_dest = gen_highpart (SImode, operands[0]);
>> +  lo_op0 = gen_lowpart (SImode, operands[1]);
>> +  hi_op0 = gen_highpart (SImode, operands[1]);
>> +  lo_op1 = gen_lowpart (SImode, operands[2]);
>> +  hi_op1 = gen_highpart (SImode, operands[2]);
>> +  if (rtx_equal_p (lo_op0, lo_op1))
>> +    FAIL;
> 
> I believe that for the emit_cmp_and_jump_insns below
> the check here should look like this:
> 
> if (rtx_equal_p (lo_dest, lo_op0) || rtx_equal_p (lo_dest, lo_op1))
> 
> But maybe drop this check and use the following instead?
> 
>  emit_insn (gen_subsi3 (hi_dest, hi_op0, hi_op1));
>  emit_cmp_and_jump_insns (lo_op0, lo_op1, GEU, const0_rtx,
>                           SImode, true, label = gen_label_rtx ());
>  emit_insn (gen_addsi3 (hi_dest, hi_dest, constm1_rtx));
>  emit_label (label);
>  emit_insn (gen_subsi3 (lo_dest, lo_op0, lo_op1));

This seems like a better improvement.

> 
>> +  emit_clobber (operands[0]);
> 
> Why is this clobber needed?
> 
>> +  emit_insn (gen_subsi3 (lo_dest, lo_op0, lo_op1));
>> +  emit_insn (gen_subsi3 (hi_dest, hi_op0, hi_op1));
>> +  emit_cmp_and_jump_insns (lo_op0, lo_op1, GEU, const0_rtx,
>> +                          SImode, true, label = gen_label_rtx ());
>> +  emit_insn (gen_addsi3 (hi_dest, hi_dest, constm1_rtx));
>> +  emit_label (label);
>> +  DONE;
>> +})
>> +
>>  (define_insn "subsf3"
>>    [(set (match_operand:SF 0 "register_operand" "=f")
>>         (minus:SF (match_operand:SF 1 "register_operand" "f")
>> --
>> 2.30.2
> 

===
More optimized than the default RTL generation.

gcc/ChangeLog:

	* config/xtensa/xtensa.md (adddi3, subdi3):
	New RTL generation patterns implemented according to the instruc-
	tion idioms described in the Xtensa ISA reference manual (p. 600).
---
 gcc/config/xtensa/xtensa.md | 52 +++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/gcc/config/xtensa/xtensa.md b/gcc/config/xtensa/xtensa.md
index eda1353894b..21afa747e89 100644
--- a/gcc/config/xtensa/xtensa.md
+++ b/gcc/config/xtensa/xtensa.md
@@ -190,6 +190,35 @@
    (set_attr "mode"	"SI")
    (set_attr "length"	"3")])
 
+(define_expand "adddi3"
+  [(set (match_operand:DI 0 "register_operand")
+	(plus:DI (match_operand:DI 1 "register_operand")
+		 (match_operand:DI 2 "register_operand")))]
+  ""
+{
+  rtx lo_dest, hi_dest, lo_op0, hi_op0, lo_op1, hi_op1;
+  rtx_code_label *label;
+  if (rtx_equal_p (operands[0], operands[1])
+      || rtx_equal_p (operands[0], operands[2])
+      || ! REG_P (operands[1]) || ! REG_P (operands[2]))
+    FAIL;
+  lo_dest = gen_lowpart (SImode, operands[0]);
+  hi_dest = gen_highpart (SImode, operands[0]);
+  lo_op0 = gen_lowpart (SImode, operands[1]);
+  hi_op0 = gen_highpart (SImode, operands[1]);
+  lo_op1 = gen_lowpart (SImode, operands[2]);
+  hi_op1 = gen_highpart (SImode, operands[2]);
+  emit_insn (gen_addsi3 (hi_dest, hi_op0, hi_op1));
+  emit_insn (gen_addsi3 (lo_dest, lo_op0, lo_op1));
+  emit_cmp_and_jump_insns (lo_dest,
+			   (REGNO (operands[1]) < REGNO (operands[2])
+			    ? lo_op1 : lo_op0), GEU, const0_rtx,
+			   SImode, true, label = gen_label_rtx ());
+  emit_insn (gen_addsi3 (hi_dest, hi_dest, const1_rtx));
+  emit_label (label);
+  DONE;
+})
+
 (define_insn "addsf3"
   [(set (match_operand:SF 0 "register_operand" "=f")
 	(plus:SF (match_operand:SF 1 "register_operand" "%f")
@@ -237,6 +266,29 @@
 		      (const_int 5)
 		      (const_int 6)))])
 
+(define_expand "subdi3"
+  [(set (match_operand:DI 0 "register_operand")
+	(minus:DI (match_operand:DI 1 "register_operand")
+		  (match_operand:DI 2 "register_operand")))]
+  ""
+{
+  rtx lo_dest, hi_dest, lo_op0, hi_op0, lo_op1, hi_op1;
+  rtx_code_label *label;
+  lo_dest = gen_lowpart (SImode, operands[0]);
+  hi_dest = gen_highpart (SImode, operands[0]);
+  lo_op0 = gen_lowpart (SImode, operands[1]);
+  hi_op0 = gen_highpart (SImode, operands[1]);
+  lo_op1 = gen_lowpart (SImode, operands[2]);
+  hi_op1 = gen_highpart (SImode, operands[2]);
+  emit_insn (gen_subsi3 (hi_dest, hi_op0, hi_op1));
+  emit_cmp_and_jump_insns (lo_op0, lo_op1, GEU, const0_rtx,
+			   SImode, true, label = gen_label_rtx ());
+  emit_insn (gen_addsi3 (hi_dest, hi_dest, constm1_rtx));
+  emit_label (label);
+  emit_insn (gen_subsi3 (lo_dest, lo_op0, lo_op1));
+  DONE;
+})
+
 (define_insn "subsf3"
   [(set (match_operand:SF 0 "register_operand" "=f")
 	(minus:SF (match_operand:SF 1 "register_operand" "f")
-- 
2.30.2

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/3 v3] xtensa: Add 'adddi3' and 'subdi3' insn patterns
  2023-06-01  6:01     ` [PATCH 2/3 v3] " Takayuki 'January June' Suwa
@ 2023-06-01 14:16       ` Max Filippov
  2023-06-01 14:20       ` Max Filippov
  1 sibling, 0 replies; 6+ messages in thread
From: Max Filippov @ 2023-06-01 14:16 UTC (permalink / raw)
  To: Takayuki 'January June' Suwa; +Cc: GCC Patches

On Wed, May 31, 2023 at 11:01 PM Takayuki 'January June' Suwa
<jjsuwa_sys3175@yahoo.co.jp> wrote:
> More optimized than the default RTL generation.
>
> gcc/ChangeLog:
>
>         * config/xtensa/xtensa.md (adddi3, subdi3):
>         New RTL generation patterns implemented according to the instruc-
>         tion idioms described in the Xtensa ISA reference manual (p. 600).
> ---
>  gcc/config/xtensa/xtensa.md | 52 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 52 insertions(+)

Regtested for target=xtensa-linux-uclibc, no new regressions.
Committed to master.

-- 
Thanks.
-- Max

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/3 v3] xtensa: Add 'adddi3' and 'subdi3' insn patterns
  2023-06-01  6:01     ` [PATCH 2/3 v3] " Takayuki 'January June' Suwa
  2023-06-01 14:16       ` Max Filippov
@ 2023-06-01 14:20       ` Max Filippov
  2023-06-01 15:36         ` Takayuki 'January June' Suwa
  1 sibling, 1 reply; 6+ messages in thread
From: Max Filippov @ 2023-06-01 14:20 UTC (permalink / raw)
  To: Takayuki 'January June' Suwa; +Cc: GCC Patches

On Wed, May 31, 2023 at 11:01 PM Takayuki 'January June' Suwa
<jjsuwa_sys3175@yahoo.co.jp> wrote:
> More optimized than the default RTL generation.
>
> gcc/ChangeLog:
>
>         * config/xtensa/xtensa.md (adddi3, subdi3):
>         New RTL generation patterns implemented according to the instruc-
>         tion idioms described in the Xtensa ISA reference manual (p. 600).
> ---
>  gcc/config/xtensa/xtensa.md | 52 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
>
> diff --git a/gcc/config/xtensa/xtensa.md b/gcc/config/xtensa/xtensa.md
> index eda1353894b..21afa747e89 100644
> --- a/gcc/config/xtensa/xtensa.md
> +++ b/gcc/config/xtensa/xtensa.md
> @@ -190,6 +190,35 @@
>     (set_attr "mode"    "SI")
>     (set_attr "length"  "3")])
>
> +(define_expand "adddi3"
> +  [(set (match_operand:DI 0 "register_operand")
> +       (plus:DI (match_operand:DI 1 "register_operand")
> +                (match_operand:DI 2 "register_operand")))]
> +  ""
> +{
> +  rtx lo_dest, hi_dest, lo_op0, hi_op0, lo_op1, hi_op1;
> +  rtx_code_label *label;
> +  if (rtx_equal_p (operands[0], operands[1])
> +      || rtx_equal_p (operands[0], operands[2])

> +      || ! REG_P (operands[1]) || ! REG_P (operands[2]))

I wonder if these additional conditions are necessary, given that
the operands have the "register_operand" predicates?

-- 
Thanks.
-- Max

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/3 v3] xtensa: Add 'adddi3' and 'subdi3' insn patterns
  2023-06-01 14:20       ` Max Filippov
@ 2023-06-01 15:36         ` Takayuki 'January June' Suwa
  0 siblings, 0 replies; 6+ messages in thread
From: Takayuki 'January June' Suwa @ 2023-06-01 15:36 UTC (permalink / raw)
  To: Max Filippov; +Cc: GCC Patches

On 2023/06/01 23:20, Max Filippov wrote:
> On Wed, May 31, 2023 at 11:01 PM Takayuki 'January June' Suwa
> <jjsuwa_sys3175@yahoo.co.jp> wrote:
>> More optimized than the default RTL generation.
>>
>> gcc/ChangeLog:
>>
>>         * config/xtensa/xtensa.md (adddi3, subdi3):
>>         New RTL generation patterns implemented according to the instruc-
>>         tion idioms described in the Xtensa ISA reference manual (p. 600).
>> ---
>>  gcc/config/xtensa/xtensa.md | 52 +++++++++++++++++++++++++++++++++++++
>>  1 file changed, 52 insertions(+)
>>
>> diff --git a/gcc/config/xtensa/xtensa.md b/gcc/config/xtensa/xtensa.md
>> index eda1353894b..21afa747e89 100644
>> --- a/gcc/config/xtensa/xtensa.md
>> +++ b/gcc/config/xtensa/xtensa.md
>> @@ -190,6 +190,35 @@
>>     (set_attr "mode"    "SI")
>>     (set_attr "length"  "3")])
>>
>> +(define_expand "adddi3"
>> +  [(set (match_operand:DI 0 "register_operand")
>> +       (plus:DI (match_operand:DI 1 "register_operand")
>> +                (match_operand:DI 2 "register_operand")))]
>> +  ""
>> +{
>> +  rtx lo_dest, hi_dest, lo_op0, hi_op0, lo_op1, hi_op1;
>> +  rtx_code_label *label;
>> +  if (rtx_equal_p (operands[0], operands[1])
>> +      || rtx_equal_p (operands[0], operands[2])
> 
>> +      || ! REG_P (operands[1]) || ! REG_P (operands[2]))
> 
> I wonder if these additional conditions are necessary, given that
> the operands have the "register_operand" predicates?
> 

See register_operand() in gcc/recog.cc.

In fact, I've encountered several operands that satisfy the
register_operand predicate but result in REG_P() being false.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-06-01 15:36 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <95b8b130-caef-12c8-b247-25ec7dbf0ac3.ref@yahoo.co.jp>
2023-05-30  9:50 ` [PATCH 2/3 v2] xtensa: Add 'adddi3' and 'subdi3' insn patterns Takayuki 'January June' Suwa
2023-05-31  6:02   ` Max Filippov
2023-06-01  6:01     ` [PATCH 2/3 v3] " Takayuki 'January June' Suwa
2023-06-01 14:16       ` Max Filippov
2023-06-01 14:20       ` Max Filippov
2023-06-01 15:36         ` Takayuki 'January June' Suwa

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).