diff mbox series

[v2] Return a NULL rtx when targets don't support cbranchcc4 or predicate check fails in prepare_cmp_insn

Message ID 905c34de-6333-1021-05e6-942922918b18@linux.ibm.com
State Accepted
Headers
Series [v2] Return a NULL rtx when targets don't support cbranchcc4 or predicate check fails in prepare_cmp_insn |

Checks

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

Commit Message

HAO CHEN GUI Dec. 5, 2022, 3:07 a.m. UTC 
  Hi,
  It gets an assertion failure when targers don't support cbranchcc4 or
predicate check fails in prepare_cmp_insn. prepare_cmp_insn is a help
function to generate compare rtx, so it should not assume that cbranchcc4
is existing or all sub-CC modes are supported on one target. I think it
should return the NULL rtx when cbranchcc4 is not supported or predicate check
fails, as its callers already check if the return value is null or not for CC
mode. This patch just does the change.

  Bootstrapped and tested on powerpc64-linux BE/LE and x86 with no regressions.
Is this okay for trunk? Any recommendations? Thanks a lot.


ChangeLog
2022-12-05  Haochen Gui <guihaoc@linux.ibm.com>

gcc/
	* optabs.cc (prepare_cmp_insn): Return a NULL rtx other than assertion
	failure when targets don't have cbranch optab or predicate check
	fails.

patch.diff

Comments

Richard Biener Dec. 5, 2022, 7:31 a.m. UTC | #1 
On Mon, Dec 5, 2022 at 4:08 AM HAO CHEN GUI <guihaoc@linux.ibm.com> wrote:
>
> Hi,
>   It gets an assertion failure when targers don't support cbranchcc4 or
> predicate check fails in prepare_cmp_insn. prepare_cmp_insn is a help
> function to generate compare rtx, so it should not assume that cbranchcc4
> is existing or all sub-CC modes are supported on one target. I think it
> should return the NULL rtx when cbranchcc4 is not supported or predicate check
> fails, as its callers already check if the return value is null or not for CC
> mode. This patch just does the change.
>
>   Bootstrapped and tested on powerpc64-linux BE/LE and x86 with no regressions.
> Is this okay for trunk? Any recommendations? Thanks a lot.

I wonder if you have a testcase you can add showing this change is
worthwhile and
fixes a bug?

OK.

Thanks,
Richard.

>
> ChangeLog
> 2022-12-05  Haochen Gui <guihaoc@linux.ibm.com>
>
> gcc/
>         * optabs.cc (prepare_cmp_insn): Return a NULL rtx other than assertion
>         failure when targets don't have cbranch optab or predicate check
>         fails.
>
> patch.diff
> diff --git a/gcc/optabs.cc b/gcc/optabs.cc
> index 165f8d1fa22..f6d3242479b 100644
> --- a/gcc/optabs.cc
> +++ b/gcc/optabs.cc
> @@ -4484,10 +4484,14 @@ prepare_cmp_insn (rtx x, rtx y, enum rtx_code comparison, rtx size,
>      {
>        enum insn_code icode = optab_handler (cbranch_optab, CCmode);
>        test = gen_rtx_fmt_ee (comparison, VOIDmode, x, y);
> -      gcc_assert (icode != CODE_FOR_nothing
> -                  && insn_operand_matches (icode, 0, test));
> -      *ptest = test;
> -      return;
> +      if (icode != CODE_FOR_nothing
> +         && insn_operand_matches (icode, 0, test))
> +       {
> +         *ptest = test;
> +         return;
> +       }
> +      else
> +       goto fail;
>      }
>
>    mclass = GET_MODE_CLASS (mode);
HAO CHEN GUI Dec. 5, 2022, 8:43 a.m. UTC | #2 
Hi Richard,

在 2022/12/5 15:31, Richard Biener 写道:
> I wonder if you have a testcase you can add showing this change is
> worthwhile and
> fixes a bug?

I want to enable cbranchcc4 on rs6000. But not all sub CCmode is
supported on rs6000. So the predicate check(assert) fails and it hits
ICE. I drafted two patches. This one is for the generic code, and
another is for rs6000. If this one is committed, cbranchcc4 can be
enabled on rs6000. Then I can create a testcase and let the predicate
check fail. Right now I can't write a testcase for it as it never
reaches the failure path.

Thanks a lot
Gui Haochen
Richard Biener Dec. 5, 2022, 8:50 a.m. UTC | #3 
On Mon, Dec 5, 2022 at 9:43 AM HAO CHEN GUI <guihaoc@linux.ibm.com> wrote:
>
> Hi Richard,
>
> 在 2022/12/5 15:31, Richard Biener 写道:
> > I wonder if you have a testcase you can add showing this change is
> > worthwhile and
> > fixes a bug?
>
> I want to enable cbranchcc4 on rs6000. But not all sub CCmode is
> supported on rs6000. So the predicate check(assert) fails and it hits
> ICE. I drafted two patches. This one is for the generic code, and
> another is for rs6000. If this one is committed, cbranchcc4 can be
> enabled on rs6000. Then I can create a testcase and let the predicate
> check fail. Right now I can't write a testcase for it as it never
> reaches the failure path.

Fair enough.


> Thanks a lot
> Gui Haochen
>
diff mbox series

Patch

diff --git a/gcc/optabs.cc b/gcc/optabs.cc
index 165f8d1fa22..f6d3242479b 100644
--- a/gcc/optabs.cc
+++ b/gcc/optabs.cc
@@ -4484,10 +4484,14 @@  prepare_cmp_insn (rtx x, rtx y, enum rtx_code comparison, rtx size,
     {
       enum insn_code icode = optab_handler (cbranch_optab, CCmode);
       test = gen_rtx_fmt_ee (comparison, VOIDmode, x, y);
-      gcc_assert (icode != CODE_FOR_nothing
-                  && insn_operand_matches (icode, 0, test));
-      *ptest = test;
-      return;
+      if (icode != CODE_FOR_nothing
+	  && insn_operand_matches (icode, 0, test))
+	{
+	  *ptest = test;
+	  return;
+	}
+      else
+	goto fail;
     }

   mclass = GET_MODE_CLASS (mode);