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

[Andrew Pinski <pinskia@gmail.com>]

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
>