| Message ID | f321ea49-6adb-7f28-aa98-13168b961e3c@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,rs6000] Enable have_cbranchcc4 on rs6000 | expand |
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
Hi Kewen, Thanks so much for your review comments. I will fix them. 在 2022/12/7 11:06, Kewen.Lin 写道: > 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. Yes, the bootstrapping fails if we enable cbranchcc4 without the generic patch. Actually, the testcase comes from the ICE found in bootstrapping.
on 2022/12/7 13:24, HAO CHEN GUI wrote: > Hi Kewen, > Thanks so much for your review comments. I will fix them. > > 在 2022/12/7 11:06, Kewen.Lin 写道: >> 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. > > Yes, the bootstrapping fails if we enable cbranchcc4 without the generic patch. > Actually, the testcase comes from the ICE found in bootstrapping. Ah, thanks for confirming, so the fix for the generic part should come first. I just noticed Richi has approved it, you can mention this test case gcc.target/powerpc/cbranchcc4-1.c in the commit log for a record when committing it. Thanks! BR, Kewen
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" */ + +/* 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. */ + +int foo (double d) +{ + if (d == 0.0) + return 0; + + d = ((d) >= 0 ? (d) : -(d)); + + if (d < 1.0) + return 1; +} diff --git a/gcc/testsuite/gcc.target/powerpc/cbranchcc4.c b/gcc/testsuite/gcc.target/powerpc/cbranchcc4.c new file mode 100644 index 00000000000..528ba1a878d --- /dev/null +++ b/gcc/testsuite/gcc.target/powerpc/cbranchcc4.c @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-rtl-ce1" } */ +/* { dg-final { scan-rtl-dump "noce_try_store_flag_constants" "ce1" } } */ + +/* The inner branch should be detected by ifcvt then be converted to a setcc + with a plus by noce_try_store_flag_constants. */ + +int test (unsigned int a, unsigned int b) +{ + return (a < b ? 0 : (a > b ? 2 : 1)); +}