Re: [PATCH 3/3] [V2] [RISC-V] support cm.mva01s cm.mvsa01 in zcmp

2023-09-11 Thread Jeff Law via Gcc-patches 




On 9/7/23 14:33, Palmer Dabbelt wrote:

On Thu, 07 Sep 2023 13:16:36 PDT (-0700), dimi...@dinux.eu wrote:

Hi,

This patch appears to have caused PR 111259.


Thanks.  Looks like wer'e not running our tests with RTL checking, 
Patrick is going to try and see if we've got compute time left for some 
builds -- even just having builds with checking would be a good one, we 
get bit by these bugs from time to time.


I'm spinning up a --enable-checking=yes build.  Maybe we just need 
something like


    diff --git a/gcc/config/riscv/predicates.md 
b/gcc/config/riscv/predicates.md

    index 53e7c1d03aa..aa4f02c67d5 100644
    --- a/gcc/config/riscv/predicates.md
    +++ b/gcc/config/riscv/predicates.md
    @@ -172,11 +172,11 @@ (define_predicate "stack_pop_up_to_s11_operand"
     ;; ZCMP predicates
     (define_predicate "a0a1_reg_operand"
    -  (and (match_operand 0 "register_operand")
    +  (and (match_code "reg")
    (match_test "IN_RANGE (REGNO (op), A0_REGNUM, A1_REGNUM)")))
     (define_predicate "zcmp_mv_sreg_operand"
    -  (and (match_operand 0 "register_operand")
    +  (and (match_code "reg")
    (match_test "TARGET_RVE ? IN_RANGE (REGNO (op), S0_REGNUM, 
S1_REGNUM)

     : IN_RANGE (REGNO (op), S0_REGNUM, S1_REGNUM)
     || IN_RANGE (REGNO (op), S2_REGNUM, S7_REGNUM)")))

Presumably you got a SUBREG if that patch solves the problem.

Note that due to RTL checking's compile-time cost, I think it's a 
separate option to enable-checking.  It's probably not feasible to turn 
it on for anything but cross testing right now.


IIRC Jakub does RTL checking builds & regression testing just once a 
year in the spring due to its cost.  And I think on just one target 
(x86).  We could probably do something similar to weed out these kinds 
of problems.  Spin it in November and wait however long it takes :-)


jeff

Re: [PATCH 3/3] [V2] [RISC-V] support cm.mva01s cm.mvsa01 in zcmp

2023-09-08 Thread Jeff Law via Gcc-patches 




On 9/7/23 20:06, Fei Gao wrote:

On 2023-09-08 04:33  Palmer Dabbelt  wrote:


On Thu, 07 Sep 2023 13:16:36 PDT (-0700), dimi...@dinux.eu wrote:

Hi,

This patch appears to have caused PR 111259.

Hi Patrick

We're reproducing the issue also.

One thing that puzzles me is why a zcmp predicate casused
a regression in rv64gc single lib build. The new define_insn
and define_peephole2 are all gated by TARGET_ZCMP, which
is false when building libgcc.
Operand predicates are checked for validity before the insn condition is 
checked.


Jeff

Re: Re: [PATCH 3/3] [V2] [RISC-V] support cm.mva01s cm.mvsa01 in zcmp

2023-09-07 Thread Fei Gao 
On 2023-09-08 04:33  Palmer Dabbelt  wrote:
>
>On Thu, 07 Sep 2023 13:16:36 PDT (-0700), dimi...@dinux.eu wrote:
>> Hi,
>>
>> This patch appears to have caused PR 111259. 
Hi Patrick 

We're reproducing the issue also. 

One thing that puzzles me is why a zcmp predicate casused
a regression in rv64gc single lib build. The new define_insn
and define_peephole2 are all gated by TARGET_ZCMP, which
is false when building libgcc.

Could you please share more about your findings regading 
"This patch appears to have caused PR 111259"?

BR, 
Fei

>
>Thanks.  Looks like wer'e not running our tests with RTL checking,
>Patrick is going to try and see if we've got compute time left for some
>builds -- even just having builds with checking would be a good one, we
>get bit by these bugs from time to time.
>
>I'm spinning up a --enable-checking=yes build.  Maybe we just need
>something like
>
>    diff --git a/gcc/config/riscv/predicates.md 
>b/gcc/config/riscv/predicates.md
>    index 53e7c1d03aa..aa4f02c67d5 100644
>    --- a/gcc/config/riscv/predicates.md
>    +++ b/gcc/config/riscv/predicates.md
>    @@ -172,11 +172,11 @@ (define_predicate "stack_pop_up_to_s11_operand"
>
> ;; ZCMP predicates
> (define_predicate "a0a1_reg_operand"
>    -  (and (match_operand 0 "register_operand")
>    +  (and (match_code "reg")
>    (match_test "IN_RANGE (REGNO (op), A0_REGNUM, A1_REGNUM)")))
>
> (define_predicate "zcmp_mv_sreg_operand"
>    -  (and (match_operand 0 "register_operand")
>    +  (and (match_code "reg")
>    (match_test "TARGET_RVE ? IN_RANGE (REGNO (op), S0_REGNUM, 
>S1_REGNUM)
> : IN_RANGE (REGNO (op), S0_REGNUM, S1_REGNUM)
> || IN_RANGE (REGNO (op), S2_REGNUM, S7_REGNUM)")))
>
>> Regards,
>> Dimitar
>>
>> On Tue, Aug 29, 2023 at 08:37:46AM +, Fei Gao wrote:
>>> From: Die Li 
>>>
>>> Signed-off-by: Die Li 
>>> Co-Authored-By: Fei Gao 
>>>
>>> gcc/ChangeLog:
>>>
>>> * config/riscv/peephole.md: New pattern.
>>> * config/riscv/predicates.md (a0a1_reg_operand): New predicate.
>>> (zcmp_mv_sreg_operand): New predicate.
>>> * config/riscv/riscv.md: New predicate.
>>> * config/riscv/zc.md (*mva01s): New pattern.
>>> (*mvsa01): New pattern.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> * gcc.target/riscv/cm_mv_rv32.c: New test.
>>> ---
>>>  gcc/config/riscv/peephole.md    | 28 +
>>>  gcc/config/riscv/predicates.md  | 11 
>>>  gcc/config/riscv/riscv.md   |  1 +
>>>  gcc/config/riscv/zc.md  | 22 
>>>  gcc/testsuite/gcc.target/riscv/cm_mv_rv32.c | 23 +
>>>  5 files changed, 85 insertions(+)
>>>  create mode 100644 gcc/testsuite/gcc.target/riscv/cm_mv_rv32.c
>>>
>>> diff --git a/gcc/config/riscv/peephole.md b/gcc/config/riscv/peephole.md
>>> index 0ef0c04410b..92e57f9a447 100644
>>> --- a/gcc/config/riscv/peephole.md
>>> +++ b/gcc/config/riscv/peephole.md
>>> @@ -38,3 +38,31 @@
>>>  {
>>>    operands[5] = GEN_INT (INTVAL (operands[2]) - INTVAL (operands[5]));
>>>  })
>>> +
>>> +;; ZCMP
>>> +(define_peephole2
>>> +  [(set (match_operand:X 0 "a0a1_reg_operand")
>>> +    (match_operand:X 1 "zcmp_mv_sreg_operand"))
>>> +   (set (match_operand:X 2 "a0a1_reg_operand")
>>> +    (match_operand:X 3 "zcmp_mv_sreg_operand"))]
>>> +  "TARGET_ZCMP
>>> +   && (REGNO (operands[2]) != REGNO (operands[0]))"
>>> +  [(parallel [(set (match_dup 0)
>>> +   (match_dup 1))
>>> +  (set (match_dup 2)
>>> +   (match_dup 3))])]
>>> +)
>>> +
>>> +(define_peephole2
>>> +  [(set (match_operand:X 0 "zcmp_mv_sreg_operand")
>>> +    (match_operand:X 1 "a0a1_reg_operand"))
>>> +   (set (match_operand:X 2 "zcmp_mv_sreg_operand")
>>> +    (match_operand:X 3 "a0a1_reg_operand"))]
>>> +  "TARGET_ZCMP
>>> +   && (REGNO (operands[0]) != REGNO (operands[2]))
>>> +   && (REGNO (operands[1]) != REGNO (operands[3]))"
>>> +  [(parallel [(set (match_dup 0)
>>> +   (match_dup 1))
>>> +  (set (match_dup 2)
>>> +   (match_dup 3))])]
>>> +)
>>> diff --git a/gcc/config/riscv/predicates.md b/gcc/config/riscv/predicates.md
>>> index 3ef09996a85..772f45df65c 100644
>>> --- a/gcc/config/riscv/predicates.md
>>> +++ b/gcc/config/riscv/predicates.md
>>> @@ -165,6 +165,17 @@
>>>    (and (match_code "const_int")
>>> (match_test "riscv_zcmp_valid_stack_adj_bytes_p (INTVAL (op), 
>>>13)")))
>>>
>>> +;; ZCMP predicates
>>> +(define_predicate "a0a1_reg_operand"
>>> +  (and (match_operand 0 "register_operand")
>>> +   (match_test "IN_RANGE (REGNO (op), A0_REGNUM, A1_REGNUM)")))
>>> +
>>> +(define_predicate "zcmp_mv_sreg_operand"
>>> +  (and (match_operand 0 "register_operand")
>>> +   (match_test "TARGET_RVE ? IN_RANGE (REGNO (op), S0_REGNUM, 
>>> S1_REGNUM)
>>> +    : IN_RANGE (REGNO (op), S0_REGNUM,

Re: [PATCH 3/3] [V2] [RISC-V] support cm.mva01s cm.mvsa01 in zcmp

2023-09-07 Thread Palmer Dabbelt 

On Thu, 07 Sep 2023 13:16:36 PDT (-0700), dimi...@dinux.eu wrote:

Hi,

This patch appears to have caused PR 111259.


Thanks.  Looks like wer'e not running our tests with RTL checking, 
Patrick is going to try and see if we've got compute time left for some 
builds -- even just having builds with checking would be a good one, we 
get bit by these bugs from time to time.


I'm spinning up a --enable-checking=yes build.  Maybe we just need 
something like


   diff --git a/gcc/config/riscv/predicates.md b/gcc/config/riscv/predicates.md
   index 53e7c1d03aa..aa4f02c67d5 100644
   --- a/gcc/config/riscv/predicates.md
   +++ b/gcc/config/riscv/predicates.md
   @@ -172,11 +172,11 @@ (define_predicate "stack_pop_up_to_s11_operand"

;; ZCMP predicates

(define_predicate "a0a1_reg_operand"
   -  (and (match_operand 0 "register_operand")
   +  (and (match_code "reg")
   (match_test "IN_RANGE (REGNO (op), A0_REGNUM, A1_REGNUM)")))

(define_predicate "zcmp_mv_sreg_operand"

   -  (and (match_operand 0 "register_operand")
   +  (and (match_code "reg")
   (match_test "TARGET_RVE ? IN_RANGE (REGNO (op), S0_REGNUM, S1_REGNUM)
: IN_RANGE (REGNO (op), S0_REGNUM, S1_REGNUM)
|| IN_RANGE (REGNO (op), S2_REGNUM, S7_REGNUM)")))


Regards,
Dimitar

On Tue, Aug 29, 2023 at 08:37:46AM +, Fei Gao wrote:

From: Die Li 

Signed-off-by: Die Li 
Co-Authored-By: Fei Gao 

gcc/ChangeLog:

* config/riscv/peephole.md: New pattern.
* config/riscv/predicates.md (a0a1_reg_operand): New predicate.
(zcmp_mv_sreg_operand): New predicate.
* config/riscv/riscv.md: New predicate.
* config/riscv/zc.md (*mva01s): New pattern.
(*mvsa01): New pattern.

gcc/testsuite/ChangeLog:

* gcc.target/riscv/cm_mv_rv32.c: New test.
---
 gcc/config/riscv/peephole.md| 28 +
 gcc/config/riscv/predicates.md  | 11 
 gcc/config/riscv/riscv.md   |  1 +
 gcc/config/riscv/zc.md  | 22 
 gcc/testsuite/gcc.target/riscv/cm_mv_rv32.c | 23 +
 5 files changed, 85 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/riscv/cm_mv_rv32.c

diff --git a/gcc/config/riscv/peephole.md b/gcc/config/riscv/peephole.md
index 0ef0c04410b..92e57f9a447 100644
--- a/gcc/config/riscv/peephole.md
+++ b/gcc/config/riscv/peephole.md
@@ -38,3 +38,31 @@
 {
   operands[5] = GEN_INT (INTVAL (operands[2]) - INTVAL (operands[5]));
 })
+
+;; ZCMP
+(define_peephole2
+  [(set (match_operand:X 0 "a0a1_reg_operand")
+(match_operand:X 1 "zcmp_mv_sreg_operand"))
+   (set (match_operand:X 2 "a0a1_reg_operand")
+(match_operand:X 3 "zcmp_mv_sreg_operand"))]
+  "TARGET_ZCMP
+   && (REGNO (operands[2]) != REGNO (operands[0]))"
+  [(parallel [(set (match_dup 0)
+   (match_dup 1))
+  (set (match_dup 2)
+   (match_dup 3))])]
+)
+
+(define_peephole2
+  [(set (match_operand:X 0 "zcmp_mv_sreg_operand")
+(match_operand:X 1 "a0a1_reg_operand"))
+   (set (match_operand:X 2 "zcmp_mv_sreg_operand")
+(match_operand:X 3 "a0a1_reg_operand"))]
+  "TARGET_ZCMP
+   && (REGNO (operands[0]) != REGNO (operands[2]))
+   && (REGNO (operands[1]) != REGNO (operands[3]))"
+  [(parallel [(set (match_dup 0)
+   (match_dup 1))
+  (set (match_dup 2)
+   (match_dup 3))])]
+)
diff --git a/gcc/config/riscv/predicates.md b/gcc/config/riscv/predicates.md
index 3ef09996a85..772f45df65c 100644
--- a/gcc/config/riscv/predicates.md
+++ b/gcc/config/riscv/predicates.md
@@ -165,6 +165,17 @@
   (and (match_code "const_int")
(match_test "riscv_zcmp_valid_stack_adj_bytes_p (INTVAL (op), 13)")))

+;; ZCMP predicates
+(define_predicate "a0a1_reg_operand"
+  (and (match_operand 0 "register_operand")
+   (match_test "IN_RANGE (REGNO (op), A0_REGNUM, A1_REGNUM)")))
+
+(define_predicate "zcmp_mv_sreg_operand"
+  (and (match_operand 0 "register_operand")
+   (match_test "TARGET_RVE ? IN_RANGE (REGNO (op), S0_REGNUM, S1_REGNUM)
+: IN_RANGE (REGNO (op), S0_REGNUM, S1_REGNUM)
+|| IN_RANGE (REGNO (op), S2_REGNUM, S7_REGNUM)")))
+
 ;; Only use branch-on-bit sequences when the mask is not an ANDI immediate.
 (define_predicate "branch_on_bit_operand"
   (and (match_code "const_int")
diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index 8e09df6ff63..aa2b5b960dc 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -132,6 +132,7 @@
(S0_REGNUM  8)
(S1_REGNUM  9)
(A0_REGNUM  10)
+   (A1_REGNUM  11)
(S2_REGNUM  18)
(S3_REGNUM  19)
(S4_REGNUM  20)
diff --git a/gcc/config/riscv/zc.md b/gcc/config/riscv/zc.md
index 8d7de97daad..77b28adde95 100644
---

Re: [PATCH 3/3] [V2] [RISC-V] support cm.mva01s cm.mvsa01 in zcmp

2023-09-07 Thread Dimitar Dimitrov 
Hi,

This patch appears to have caused PR 111259.

Regards,
Dimitar

On Tue, Aug 29, 2023 at 08:37:46AM +, Fei Gao wrote:
> From: Die Li 
> 
> Signed-off-by: Die Li 
> Co-Authored-By: Fei Gao 
> 
> gcc/ChangeLog:
> 
> * config/riscv/peephole.md: New pattern.
> * config/riscv/predicates.md (a0a1_reg_operand): New predicate.
> (zcmp_mv_sreg_operand): New predicate.
> * config/riscv/riscv.md: New predicate.
> * config/riscv/zc.md (*mva01s): New pattern.
> (*mvsa01): New pattern.
> 
> gcc/testsuite/ChangeLog:
> 
> * gcc.target/riscv/cm_mv_rv32.c: New test.
> ---
>  gcc/config/riscv/peephole.md| 28 +
>  gcc/config/riscv/predicates.md  | 11 
>  gcc/config/riscv/riscv.md   |  1 +
>  gcc/config/riscv/zc.md  | 22 
>  gcc/testsuite/gcc.target/riscv/cm_mv_rv32.c | 23 +
>  5 files changed, 85 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.target/riscv/cm_mv_rv32.c
> 
> diff --git a/gcc/config/riscv/peephole.md b/gcc/config/riscv/peephole.md
> index 0ef0c04410b..92e57f9a447 100644
> --- a/gcc/config/riscv/peephole.md
> +++ b/gcc/config/riscv/peephole.md
> @@ -38,3 +38,31 @@
>  {
>operands[5] = GEN_INT (INTVAL (operands[2]) - INTVAL (operands[5]));
>  })
> +
> +;; ZCMP
> +(define_peephole2
> +  [(set (match_operand:X 0 "a0a1_reg_operand")
> +(match_operand:X 1 "zcmp_mv_sreg_operand"))
> +   (set (match_operand:X 2 "a0a1_reg_operand")
> +(match_operand:X 3 "zcmp_mv_sreg_operand"))]
> +  "TARGET_ZCMP
> +   && (REGNO (operands[2]) != REGNO (operands[0]))"
> +  [(parallel [(set (match_dup 0)
> +   (match_dup 1))
> +  (set (match_dup 2)
> +   (match_dup 3))])]
> +)
> +
> +(define_peephole2
> +  [(set (match_operand:X 0 "zcmp_mv_sreg_operand")
> +(match_operand:X 1 "a0a1_reg_operand"))
> +   (set (match_operand:X 2 "zcmp_mv_sreg_operand")
> +(match_operand:X 3 "a0a1_reg_operand"))]
> +  "TARGET_ZCMP
> +   && (REGNO (operands[0]) != REGNO (operands[2]))
> +   && (REGNO (operands[1]) != REGNO (operands[3]))"
> +  [(parallel [(set (match_dup 0)
> +   (match_dup 1))
> +  (set (match_dup 2)
> +   (match_dup 3))])]
> +)
> diff --git a/gcc/config/riscv/predicates.md b/gcc/config/riscv/predicates.md
> index 3ef09996a85..772f45df65c 100644
> --- a/gcc/config/riscv/predicates.md
> +++ b/gcc/config/riscv/predicates.md
> @@ -165,6 +165,17 @@
>(and (match_code "const_int")
> (match_test "riscv_zcmp_valid_stack_adj_bytes_p (INTVAL (op), 13)")))
>  
> +;; ZCMP predicates
> +(define_predicate "a0a1_reg_operand"
> +  (and (match_operand 0 "register_operand")
> +   (match_test "IN_RANGE (REGNO (op), A0_REGNUM, A1_REGNUM)")))
> +
> +(define_predicate "zcmp_mv_sreg_operand"
> +  (and (match_operand 0 "register_operand")
> +   (match_test "TARGET_RVE ? IN_RANGE (REGNO (op), S0_REGNUM, S1_REGNUM)
> +: IN_RANGE (REGNO (op), S0_REGNUM, S1_REGNUM)
> +|| IN_RANGE (REGNO (op), S2_REGNUM, S7_REGNUM)")))
> +
>  ;; Only use branch-on-bit sequences when the mask is not an ANDI immediate.
>  (define_predicate "branch_on_bit_operand"
>(and (match_code "const_int")
> diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
> index 8e09df6ff63..aa2b5b960dc 100644
> --- a/gcc/config/riscv/riscv.md
> +++ b/gcc/config/riscv/riscv.md
> @@ -132,6 +132,7 @@
> (S0_REGNUM8)
> (S1_REGNUM9)
> (A0_REGNUM10)
> +   (A1_REGNUM11)
> (S2_REGNUM18)
> (S3_REGNUM19)
> (S4_REGNUM20)
> diff --git a/gcc/config/riscv/zc.md b/gcc/config/riscv/zc.md
> index 8d7de97daad..77b28adde95 100644
> --- a/gcc/config/riscv/zc.md
> +++ b/gcc/config/riscv/zc.md
> @@ -1433,3 +1433,25 @@
>"TARGET_ZCMP"
>"cm.push   {ra, s0-s11}, %0"
>  )
> +
> +;; ZCMP mv
> +(define_insn "*mva01s"
> +  [(set (match_operand:X 0 "a0a1_reg_operand" "=r")
> +(match_operand:X 1 "zcmp_mv_sreg_operand" "r"))
> +   (set (match_operand:X 2 "a0a1_reg_operand" "=r")
> +(match_operand:X 3 "zcmp_mv_sreg_operand" "r"))]
> +  "TARGET_ZCMP
> +   && (REGNO (operands[2]) != REGNO (operands[0]))"
> +  { return (REGNO (operands[0]) == 
> A0_REGNUM)?"cm.mva01s\t%1,%3":"cm.mva01s\t%3,%1"; }
> +  [(set_attr "mode" "")])
> +
> +(define_insn "*mvsa01"
> +  [(set (match_operand:X 0 "zcmp_mv_sreg_operand" "=r")
> +(match_operand:X 1 "a0a1_reg_operand" "r"))
> +   (set (match_operand:X 2 "zcmp_mv_sreg_operand" "=r")
> +(match_operand:X 3 "a0a1_reg_operand" "r"))]
> +  "TARGET_ZCMP
> +   && (REGNO (operands[0]) != REGNO (operands[2]))
> +   && (REGNO (operands[1]) != REGNO (operands[3]))"
> +  { return (REGNO (operands[1]) == 
>