[ARC] Use rlc r0, 0 to implement scc_ltu (i.e. carry_flag ? 1 : 0)

Message ID 015501d9f2ed$2e299bb0$8a7cd310$@nextmovesoftware.com
State Accepted
Headers
Series [ARC] Use rlc r0, 0 to implement scc_ltu (i.e. carry_flag ? 1 : 0) |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Roger Sayle Sept. 29, 2023, 3:54 p.m. UTC
  This patch teaches the ARC backend that the contents of the carry flag
can be placed in an integer register conveniently using the "rlc rX,0"
instruction, which is a rotate-left-through-carry using zero as a source.
This is a convenient special case for the LTU form of the scc pattern.

unsigned int foo(unsigned int x, unsigned int y)
{
  return (x+y) < x;
}

With -O2 -mcpu=em this is currently compiled to:

foo:    add.f 0,r0,r1
        mov_s   r0,1    ;3
        j_s.d   [blink]
        mov.hs r0,0

[which after an addition to set the carry flag, sets r0 to 1,
followed by a conditional assignment of r0 to zero if the
carry flag is clear].  With the new define_insn/optimization
in this patch, this becomes:

foo:    add.f 0,r0,r1
        j_s.d   [blink]
        rlc     r0,0

This define_insn is also a useful building block for implementing
shifts and rotates.

Tested on a cross-compiler to arc-linux (hosted on x86_64-pc-linux-gnu),
and a partial tool chain, where the new case passes and there are no
new regressions.  Ok for mainline?


2023-09-29  Roger Sayle  <roger@nextmovesoftware.com>

gcc/ChangeLog
        * config/arc/arc.md (CC_ltu): New mode iterator for CC and CC_C.
        (scc_ltu_<mode>): New define_insn to handle LTU form of scc_insn.
        (*scc_insn): Don't split to a conditional move sequence for LTU.

gcc/testsuite/ChangeLog
        * gcc.target/arc/scc-ltu.c: New test case.


Thanks in advance,
Roger
--
  

Comments

Claudiu Zissulescu Sept. 29, 2023, 6:04 p.m. UTC | #1
Hi Roger,

The patch looks sane. Have you run dejagnu test suite? 

Thanks,
Claudiu

-----Original Message-----
From: Roger Sayle <roger@nextmovesoftware.com> 
Sent: Friday, September 29, 2023 6:54 PM
To: gcc-patches@gcc.gnu.org
Cc: Claudiu Zissulescu <claziss@synopsys.com>
Subject: [ARC PATCH] Use rlc r0,0 to implement scc_ltu (i.e. carry_flag ? 1 : 0)


This patch teaches the ARC backend that the contents of the carry flag can be placed in an integer register conveniently using the "rlc rX,0"
instruction, which is a rotate-left-through-carry using zero as a source.
This is a convenient special case for the LTU form of the scc pattern.

unsigned int foo(unsigned int x, unsigned int y) {
  return (x+y) < x;
}

With -O2 -mcpu=em this is currently compiled to:

foo:    add.f 0,r0,r1
        mov_s   r0,1    ;3
        j_s.d   [blink]
        mov.hs r0,0

[which after an addition to set the carry flag, sets r0 to 1, followed by a conditional assignment of r0 to zero if the carry flag is clear].  With the new define_insn/optimization in this patch, this becomes:

foo:    add.f 0,r0,r1
        j_s.d   [blink]
        rlc     r0,0

This define_insn is also a useful building block for implementing shifts and rotates.

Tested on a cross-compiler to arc-linux (hosted on x86_64-pc-linux-gnu), and a partial tool chain, where the new case passes and there are no new regressions.  Ok for mainline?


2023-09-29  Roger Sayle  <roger@nextmovesoftware.com>

gcc/ChangeLog
        * config/arc/arc.md (CC_ltu): New mode iterator for CC and CC_C.
        (scc_ltu_<mode>): New define_insn to handle LTU form of scc_insn.
        (*scc_insn): Don't split to a conditional move sequence for LTU.

gcc/testsuite/ChangeLog
        * gcc.target/arc/scc-ltu.c: New test case.


Thanks in advance,
Roger
--
  
Roger Sayle Sept. 29, 2023, 9:11 p.m. UTC | #2
Hi Claudiu,
> The patch looks sane. Have you run dejagnu test suite?

I've not yet managed to set up an emulator or compile the entire toolchain,
so my dejagnu results are only useful for catching (serious) problems in the
compile only tests:

                === gcc Summary ===

# of expected passes            91875
# of unexpected failures        23768
# of unexpected successes       23
# of expected failures          1038
# of unresolved testcases       19490
# of unsupported tests          3819
/home/roger/GCC/arc-linux/gcc/xgcc  version 14.0.0 20230828 (experimental)
(GCC)

If someone could double check there are no issues on real hardware that
would be great.  I'm not sure if ARC is one of the targets covered by Jeff
Law's
compile farm?


> -----Original Message-----
> From: Roger Sayle <roger@nextmovesoftware.com>
> Sent: Friday, September 29, 2023 6:54 PM
> To: gcc-patches@gcc.gnu.org
> Cc: Claudiu Zissulescu <claziss@synopsys.com>
> Subject: [ARC PATCH] Use rlc r0,0 to implement scc_ltu (i.e. carry_flag ?
1 : 0)
> 
> 
> This patch teaches the ARC backend that the contents of the carry flag can
be
> placed in an integer register conveniently using the "rlc rX,0"
> instruction, which is a rotate-left-through-carry using zero as a source.
> This is a convenient special case for the LTU form of the scc pattern.
> 
> unsigned int foo(unsigned int x, unsigned int y) {
>   return (x+y) < x;
> }
> 
> With -O2 -mcpu=em this is currently compiled to:
> 
> foo:    add.f 0,r0,r1
>         mov_s   r0,1    ;3
>         j_s.d   [blink]
>         mov.hs r0,0
> 
> [which after an addition to set the carry flag, sets r0 to 1, followed by
a
> conditional assignment of r0 to zero if the carry flag is clear].  With
the new
> define_insn/optimization in this patch, this becomes:
> 
> foo:    add.f 0,r0,r1
>         j_s.d   [blink]
>         rlc     r0,0
> 
> This define_insn is also a useful building block for implementing shifts
and rotates.
> 
> Tested on a cross-compiler to arc-linux (hosted on x86_64-pc-linux-gnu),
and a
> partial tool chain, where the new case passes and there are no new
regressions.
> Ok for mainline?
> 
> 
> 2023-09-29  Roger Sayle  <roger@nextmovesoftware.com>
> 
> gcc/ChangeLog
>         * config/arc/arc.md (CC_ltu): New mode iterator for CC and CC_C.
>         (scc_ltu_<mode>): New define_insn to handle LTU form of scc_insn.
>         (*scc_insn): Don't split to a conditional move sequence for LTU.
> 
> gcc/testsuite/ChangeLog
>         * gcc.target/arc/scc-ltu.c: New test case.
> 
> 
> Thanks in advance,
> Roger
> --
  
Jeff Law Sept. 29, 2023, 10:02 p.m. UTC | #3
On 9/29/23 15:11, Roger Sayle wrote:
> 
> Hi Claudiu,
>> The patch looks sane. Have you run dejagnu test suite?
> 
> I've not yet managed to set up an emulator or compile the entire toolchain,
> so my dejagnu results are only useful for catching (serious) problems in the
> compile only tests:
> 
>                  === gcc Summary ===
> 
> # of expected passes            91875
> # of unexpected failures        23768
> # of unexpected successes       23
> # of expected failures          1038
> # of unresolved testcases       19490
> # of unsupported tests          3819
> /home/roger/GCC/arc-linux/gcc/xgcc  version 14.0.0 20230828 (experimental)
> (GCC)
> 
> If someone could double check there are no issues on real hardware that
> would be great.  I'm not sure if ARC is one of the targets covered by Jeff
> Law's compile farm?
It is :-)  Runs daily, about 4:30 am UTC.  So if the bits go in we'd 
have data within 24hrs.


Jeff
  
Claudiu Zissulescu Oct. 1, 2023, 2:32 p.m. UTC | #4
I'll add it to our nightly. Just to be sure 😊 I’ll let you know asap it's status.

Roger, you can always use Synopsys free nsim simulator which you can find it on Synopsys website.

Thanks,
Claudiu


-----Original Message-----
From: Jeff Law <jeffreyalaw@gmail.com> 
Sent: Saturday, September 30, 2023 1:02 AM
To: Roger Sayle <roger@nextmovesoftware.com>; Claudiu Zissulescu <claziss@synopsys.com>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [ARC PATCH] Use rlc r0, 0 to implement scc_ltu (i.e. carry_flag ? 1 : 0)



On 9/29/23 15:11, Roger Sayle wrote:
> 
> Hi Claudiu,
>> The patch looks sane. Have you run dejagnu test suite?
> 
> I've not yet managed to set up an emulator or compile the entire 
> toolchain, so my dejagnu results are only useful for catching 
> (serious) problems in the compile only tests:
> 
>                  === gcc Summary ===
> 
> # of expected passes            91875
> # of unexpected failures        23768
> # of unexpected successes       23
> # of expected failures          1038
> # of unresolved testcases       19490
> # of unsupported tests          3819
> /home/roger/GCC/arc-linux/gcc/xgcc  version 14.0.0 20230828 
> (experimental)
> (GCC)
> 
> If someone could double check there are no issues on real hardware 
> that would be great.  I'm not sure if ARC is one of the targets 
> covered by Jeff Law's compile farm?
It is :-)  Runs daily, about 4:30 am UTC.  So if the bits go in we'd have data within 24hrs.


Jeff
  
Claudiu Zissulescu Oct. 2, 2023, 4:39 p.m. UTC | #5
Hi Roger,

Everything is good. Ok for mainline.

Thank you for your contribution,
Claudiu

-----Original Message-----
From: Claudiu Zissulescu 
Sent: Sunday, October 1, 2023 5:33 PM
To: Jeff Law <jeffreyalaw@gmail.com>; Roger Sayle <roger@nextmovesoftware.com>
Cc: gcc-patches@gcc.gnu.org
Subject: RE: [ARC PATCH] Use rlc r0, 0 to implement scc_ltu (i.e. carry_flag ? 1 : 0)

I'll add it to our nightly. Just to be sure 😊 I’ll let you know asap it's status.

Roger, you can always use Synopsys free nsim simulator which you can find it on Synopsys website.

Thanks,
Claudiu


-----Original Message-----
From: Jeff Law <jeffreyalaw@gmail.com> 
Sent: Saturday, September 30, 2023 1:02 AM
To: Roger Sayle <roger@nextmovesoftware.com>; Claudiu Zissulescu <claziss@synopsys.com>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [ARC PATCH] Use rlc r0, 0 to implement scc_ltu (i.e. carry_flag ? 1 : 0)



On 9/29/23 15:11, Roger Sayle wrote:
> 
> Hi Claudiu,
>> The patch looks sane. Have you run dejagnu test suite?
> 
> I've not yet managed to set up an emulator or compile the entire 
> toolchain, so my dejagnu results are only useful for catching 
> (serious) problems in the compile only tests:
> 
>                  === gcc Summary ===
> 
> # of expected passes            91875
> # of unexpected failures        23768
> # of unexpected successes       23
> # of expected failures          1038
> # of unresolved testcases       19490
> # of unsupported tests          3819
> /home/roger/GCC/arc-linux/gcc/xgcc  version 14.0.0 20230828 
> (experimental)
> (GCC)
> 
> If someone could double check there are no issues on real hardware 
> that would be great.  I'm not sure if ARC is one of the targets 
> covered by Jeff Law's compile farm?
It is :-)  Runs daily, about 4:30 am UTC.  So if the bits go in we'd have data within 24hrs.


Jeff
  

Patch

diff --git a/gcc/config/arc/arc.md b/gcc/config/arc/arc.md
index d37ecbf..fe2e7fb 100644
--- a/gcc/config/arc/arc.md
+++ b/gcc/config/arc/arc.md
@@ -3658,12 +3658,24 @@  archs4x, archs4xd"
 (define_expand "scc_insn"
   [(set (match_operand:SI 0 "dest_reg_operand" "=w") (match_operand:SI 1 ""))])
 
+(define_mode_iterator CC_ltu [CC_C CC])
+
+(define_insn "scc_ltu_<mode>"
+  [(set (match_operand:SI 0 "dest_reg_operand" "=w")
+        (ltu:SI (reg:CC_ltu CC_REG) (const_int 0)))]
+  ""
+  "rlc\\t%0,0"
+  [(set_attr "type" "shift")
+   (set_attr "predicable" "no")
+   (set_attr "length" "4")])
+
 (define_insn_and_split "*scc_insn"
   [(set (match_operand:SI 0 "dest_reg_operand" "=w")
 	(match_operator:SI 1 "proper_comparison_operator" [(reg CC_REG) (const_int 0)]))]
   ""
   "#"
-  "reload_completed"
+  "reload_completed
+   && GET_CODE (operands[1]) != LTU"
   [(set (match_dup 0) (const_int 1))
    (cond_exec
      (match_dup 1)
diff --git a/gcc/testsuite/gcc.target/arc/scc-ltu.c b/gcc/testsuite/gcc.target/arc/scc-ltu.c
new file mode 100644
index 0000000..653c55d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arc/scc-ltu.c
@@ -0,0 +1,12 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -mcpu=em" } */
+
+unsigned int foo(unsigned int x, unsigned int y)
+{
+  return (x+y) < x;
+}
+
+/* { dg-final { scan-assembler "rlc\\s+r0,0" } } */
+/* { dg-final { scan-assembler "add.f\\s+0,r0,r1" } } */
+/* { dg-final { scan-assembler-not "mov_s\\s+r0,1" } } */
+/* { dg-final { scan-assembler-not "mov\.hs\\s+r0,0" } } */