Re: [PATCH v3, rs6000] Enable have_cbranchcc4 on rs6000

["Kewen.Lin" <linkw@linux.ibm.com>]

Hi Haochen,

on 2022/12/6 13:44, HAO CHEN GUI wrote:
> Hi,
>   This patch enables "have_cbranchcc4" on rs6000 by defining
> a "cbranchcc4" expander. "have_cbrnachcc4" is a flag in ifcvt.cc
> to indicate if branch by CC bits is invalid or not. With this
> flag enabled, some branches can be optimized to conditional
> moves.
> 
>   The patch relies on the former patch which is under review.
> https://gcc.gnu.org/pipermail/gcc-patches/2022-December/607810.html
> 
>   Bootstrapped and tested on powerpc64-linux BE and LE with no
> regressions. Is this okay for trunk? Any recommendations? Thanks
> a lot.
> 
> Thanks
> Gui Haochen
> 
> ChangeLog
> 2022-12-06  Haochen Gui <guihaoc@linux.ibm.com>
> 
> gcc/
> 	* config/rs6000/rs6000.md (cbranchcc4): New expander.
> 
> gcc/testsuite
> 	* gcc.target/powerpc/cbranchcc4.c: New.
> 	* gcc.target/powerpc/cbranchcc4-1.c: New.

Nit: "cbranchcc4.c" -> "cbranchcc4-2.c" since we already number the cases.

> 
> 
> patch.diff
> diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
> index e9e5cd1e54d..d7ddd96cc70 100644
> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -11932,6 +11932,16 @@ (define_expand "cbranch<mode>4"
>    DONE;
>  })
> 
> +(define_expand "cbranchcc4"
> +  [(set (pc)
> +	(if_then_else (match_operator 0 "branch_comparison_operator"
> +			[(match_operand 1 "cc_reg_operand")
> +			 (match_operand 2 "zero_constant")])
> +		      (label_ref (match_operand 3))
> +		      (pc)))]
> +  ""
> +  "")
> +
>  (define_expand "cstore<mode>4_signed"
>    [(use (match_operator 1 "signed_comparison_operator"
>           [(match_operand:P 2 "gpc_reg_operand")
> diff --git a/gcc/testsuite/gcc.target/powerpc/cbranchcc4-1.c b/gcc/testsuite/gcc.target/powerpc/cbranchcc4-1.c
> new file mode 100644
> index 00000000000..3c8286bf091
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/cbranchcc4-1.c
> @@ -0,0 +1,17 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2" */
> +

Missing one " }", typo from copy/paste?

> +/* This case should be successfully compiled after cbranchcc4 is enabled.  It
> +   generates a "*cbranch_2insn" insn which makes predicate check of cbranchcc4
> +   failed and returns a NULL rtx from prepare_cmp_insn.  */

Nit: May be shorter like "Verify there is no ICE with cbranchcc4 enabled." and put
the details into commit logs.

Does this issue which relies on the fix for generic part make bootstrapping fail?
If no, how many failures it can cause?  I'm thinking if we can commit this firstly,
then in the commit log of the fix for generic part you can mention it can fix the
ICE exposed by this test case.

BR,
Kewen