[RFC,v1,01/10] docs: Document a canonical RTL for a conditional-zero insns

Message ID 20230210224150.2801962-2-philipp.tomsich@vrull.eu
State Under Review
Delegated to: Jeff Law
Headers
Series RISC-V: Support the Zicond (conditional-operations) extension |

Commit Message

Philipp Tomsich Feb. 10, 2023, 10:41 p.m. UTC
  On RISC-V, conditional-zero (i.e., move a register value or zero to a
destination register) instructions are part if the Zicond extension.
To support architectures that have similar constructs, we define a
canonical RTL representation that can be used in if-conversion.

Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
---

 gcc/doc/md.texi | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
  

Comments

Andrew Pinski Feb. 10, 2023, 11:18 p.m. UTC | #1
On Fri, Feb 10, 2023 at 2:43 PM Philipp Tomsich
<philipp.tomsich@vrull.eu> wrote:
>
> On RISC-V, conditional-zero (i.e., move a register value or zero to a
> destination register) instructions are part if the Zicond extension.
> To support architectures that have similar constructs, we define a
> canonical RTL representation that can be used in if-conversion.

This is seems like worse canonical form than say:
(define_insn ""
  [(set (match_operand:m 0 ...)
      (if_then_else (eq_or_ne (match_operand:m 1 ...) (const_int 0))
         (match_operand:m 2 ...)
         (const_int 0)
       ))]
  "..."
  "...")
(define_insn ""
  [(set (match_operand:m 0 ...)
      (if_then_else (eq_or_ne (match_operand:m 1 ...) (const_int 0))
         (const_int 0)
         (match_operand:m 2 ...)
       ))]
  "..."
  "...")

Why can't you use the above form instead? This is the standard way of
expressing condition moves of 0.
This matches even what aarch64 form is already:
(define_insn "*cmov<mode>_insn"
  [(set (match_operand:ALLI 0 "register_operand" "=r,r,r,r,r,r,r")
        (if_then_else:ALLI
         (match_operator 1 "aarch64_comparison_operator"
          [(match_operand 2 "cc_register" "") (const_int 0)])
         (match_operand:ALLI 3 "aarch64_reg_zero_or_m1_or_1"
"rZ,rZ,UsM,rZ,Ui1,UsM,Ui1")
         (match_operand:ALLI 4 "aarch64_reg_zero_or_m1_or_1"
"rZ,UsM,rZ,Ui1,rZ,UsM,Ui1")))]
  "!((operands[3] == const1_rtx && operands[4] == constm1_rtx)
     || (operands[3] == constm1_rtx && operands[4] == const1_rtx))"
  ;; Final two alternatives should be unreachable, but included for completeness
  "..."
  [(set_attr "type" "csel, csel, csel, csel, csel, mov_imm, mov_imm")]
)

(Which is more complex as it can handle even more than just the simple
case you provide).

Thanks,
Andrew Pinski

> +(define_insn ""
> +  [(set (match_operand:@var{m} 0 @dots{})
> +        (and:@var{m}
> +          (neg:@var{m} (@var{eq_or_ne} (match_operand:@var{m} 1 @dots{})
> +                                       (const_int 0)))
> +          (match_operand:@var{m} 2 @dots{})))]
> +  "@dots{}"
> +  "@dots{}")

>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
> ---
>
>  gcc/doc/md.texi | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
> index 7235d34c4b3..579462ea67f 100644
> --- a/gcc/doc/md.texi
> +++ b/gcc/doc/md.texi
> @@ -8347,6 +8347,23 @@ operand of @code{mult} is also a shift, then that is extended also.
>  This transformation is only applied when it can be proven that the
>  original operation had sufficient precision to prevent overflow.
>
> +@cindex @code{conditional-zero}, canonicalization of
> +@item
> +A machine that has an instruction that performs a conditional-zero
> +operation (i.e., an instruction that moves a register value or puts 0
> +into the destination register) should specify the pattern for that
> +instruction as:
> +@smallexample
> +(define_insn ""
> +  [(set (match_operand:@var{m} 0 @dots{})
> +        (and:@var{m}
> +          (neg:@var{m} (@var{eq_or_ne} (match_operand:@var{m} 1 @dots{})
> +                                       (const_int 0)))
> +          (match_operand:@var{m} 2 @dots{})))]
> +  "@dots{}"
> +  "@dots{}")
> +@end smallexample
> +
>  @end itemize
>
>  Further canonicalization rules are defined in the function
> --
> 2.34.1
>
  

Patch

diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
index 7235d34c4b3..579462ea67f 100644
--- a/gcc/doc/md.texi
+++ b/gcc/doc/md.texi
@@ -8347,6 +8347,23 @@  operand of @code{mult} is also a shift, then that is extended also.
 This transformation is only applied when it can be proven that the
 original operation had sufficient precision to prevent overflow.
 
+@cindex @code{conditional-zero}, canonicalization of
+@item
+A machine that has an instruction that performs a conditional-zero
+operation (i.e., an instruction that moves a register value or puts 0
+into the destination register) should specify the pattern for that
+instruction as:
+@smallexample
+(define_insn ""
+  [(set (match_operand:@var{m} 0 @dots{})
+        (and:@var{m}
+          (neg:@var{m} (@var{eq_or_ne} (match_operand:@var{m} 1 @dots{})
+                                       (const_int 0)))
+          (match_operand:@var{m} 2 @dots{})))]
+  "@dots{}"
+  "@dots{}")
+@end smallexample
+
 @end itemize
 
 Further canonicalization rules are defined in the function