diff mbox series

AArch64: relax cbranch tests to accepted inverted branches [PR113502]

Message ID patch-18218-tamar@arm.com
State New
Headers show
Series AArch64: relax cbranch tests to accepted inverted branches [PR113502] | expand

Commit Message

Tamar Christina Jan. 29, 2024, 3:04 p.m. UTC
Hi All,

Recently something in the midend had started inverting the branches by inverting
the condition and the branches.

While this is fine, it makes it hard to actually test.  In RTL I disable
scheduling and BB reordering to prevent this.  But in GIMPLE there seems to be
nothing I can do.  __builtin_expect seems to have no impact on the change since
I suspect this is happening during expand where conditions can be flipped
regardless of probability during compare_and_branch.

Since the mid-end has plenty of correctness tests, this weakens the backend
tests to just check that a correct looking sequence is emitted.

Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.

Ok for master?

Thanks,
Tamar

gcc/testsuite/ChangeLog:

	PR testsuite/113502
	* gcc.target/aarch64/sve/vect-early-break-cbranch.c: Ignore exact branch.
	* gcc.target/aarch64/vect-early-break-cbranch.c: Likewise.

--- inline copy of patch -- 
diff --git a/gcc/testsuite/gcc.target/aarch64/sve/vect-early-break-cbranch.c b/gcc/testsuite/gcc.target/aarch64/sve/vect-early-break-cbranch.c
index d15053553f94e7dce3540e21f0c1f0d39ea4f289..d7cef1105410be04ed67d1d3b800746267f205a8 100644




--
diff --git a/gcc/testsuite/gcc.target/aarch64/sve/vect-early-break-cbranch.c b/gcc/testsuite/gcc.target/aarch64/sve/vect-early-break-cbranch.c
index d15053553f94e7dce3540e21f0c1f0d39ea4f289..d7cef1105410be04ed67d1d3b800746267f205a8 100644
--- a/gcc/testsuite/gcc.target/aarch64/sve/vect-early-break-cbranch.c
+++ b/gcc/testsuite/gcc.target/aarch64/sve/vect-early-break-cbranch.c
@@ -9,7 +9,7 @@ int b[N] = {0};
 **	...
 **	cmpgt	p[0-9]+.s, p[0-9]+/z, z[0-9]+.s, #0
 **	ptest	p[0-9]+, p[0-9]+.b
-**	b.any	\.L[0-9]+
+**	b.(any|none)	\.L[0-9]+
 **	...
 */
 void f1 ()
@@ -26,7 +26,7 @@ void f1 ()
 **	...
 **	cmpge	p[0-9]+.s, p[0-9]+/z, z[0-9]+.s, #0
 **	ptest	p[0-9]+, p[0-9]+.b
-**	b.any	\.L[0-9]+
+**	b.(any|none)	\.L[0-9]+
 **	...
 */
 void f2 ()
@@ -43,7 +43,7 @@ void f2 ()
 **	...
 **	cmpeq	p[0-9]+.s, p[0-9]+/z, z[0-9]+.s, #0
 **	ptest	p[0-9]+, p[0-9]+.b
-**	b.any	\.L[0-9]+
+**	b.(any|none)	\.L[0-9]+
 **	...
 */
 void f3 ()
@@ -60,7 +60,7 @@ void f3 ()
 **	...
 **	cmpne	p[0-9]+.s, p[0-9]+/z, z[0-9]+.s, #0
 **	ptest	p[0-9]+, p[0-9]+.b
-**	b.any	\.L[0-9]+
+**	b.(any|none)	\.L[0-9]+
 **	...
 */
 void f4 ()
@@ -77,7 +77,7 @@ void f4 ()
 **	...
 **	cmplt	p[0-9]+.s, p7/z, z[0-9]+.s, #0
 **	ptest	p[0-9]+, p[0-9]+.b
-**	b.any	.L[0-9]+
+**	b.(any|none)	.L[0-9]+
 **	...
 */
 void f5 ()
@@ -94,7 +94,7 @@ void f5 ()
 **	...
 **	cmple	p[0-9]+.s, p[0-9]+/z, z[0-9]+.s, #0
 **	ptest	p[0-9]+, p[0-9]+.b
-**	b.any	\.L[0-9]+
+**	b.(any|none)	\.L[0-9]+
 **	...
 */
 void f6 ()
diff --git a/gcc/testsuite/gcc.target/aarch64/vect-early-break-cbranch.c b/gcc/testsuite/gcc.target/aarch64/vect-early-break-cbranch.c
index a5e7b94827dd70240d754a834f1d11750a9c27a9..673b781eb6d092f6311409797b20a971f4fae247 100644
--- a/gcc/testsuite/gcc.target/aarch64/vect-early-break-cbranch.c
+++ b/gcc/testsuite/gcc.target/aarch64/vect-early-break-cbranch.c
@@ -15,7 +15,7 @@ int b[N] = {0};
 **	cmgt	v[0-9]+.4s, v[0-9]+.4s, #0
 **	umaxp	v[0-9]+.4s, v[0-9]+.4s, v[0-9]+.4s
 **	fmov	x[0-9]+, d[0-9]+
-**	cbnz	x[0-9]+, \.L[0-9]+
+**	cbn?z	x[0-9]+, \.L[0-9]+
 **	...
 */
 void f1 ()
@@ -34,7 +34,7 @@ void f1 ()
 **	cmge	v[0-9]+.4s, v[0-9]+.4s, #0
 **	umaxp	v[0-9]+.4s, v[0-9]+.4s, v[0-9]+.4s
 **	fmov	x[0-9]+, d[0-9]+
-**	cbnz	x[0-9]+, \.L[0-9]+
+**	cbn?z	x[0-9]+, \.L[0-9]+
 **	...
 */
 void f2 ()
@@ -53,7 +53,7 @@ void f2 ()
 **	cmeq	v[0-9]+.4s, v[0-9]+.4s, #0
 **	umaxp	v[0-9]+.4s, v[0-9]+.4s, v[0-9]+.4s
 **	fmov	x[0-9]+, d[0-9]+
-**	cbnz	x[0-9]+, \.L[0-9]+
+**	cbn?z	x[0-9]+, \.L[0-9]+
 **	...
 */
 void f3 ()
@@ -72,7 +72,7 @@ void f3 ()
 **	cmtst	v[0-9]+.4s, v[0-9]+.4s, v[0-9]+.4s
 **	umaxp	v[0-9]+.4s, v[0-9]+.4s, v[0-9]+.4s
 **	fmov	x[0-9]+, d[0-9]+
-**	cbnz	x[0-9]+, \.L[0-9]+
+**	cbn?z	x[0-9]+, \.L[0-9]+
 **	...
 */
 void f4 ()
@@ -91,7 +91,7 @@ void f4 ()
 **	cmlt	v[0-9]+.4s, v[0-9]+.4s, #0
 **	umaxp	v[0-9]+.4s, v[0-9]+.4s, v[0-9]+.4s
 **	fmov	x[0-9]+, d[0-9]+
-**	cbnz	x[0-9]+, \.L[0-9]+
+**	cbn?z	x[0-9]+, \.L[0-9]+
 **	...
 */
 void f5 ()
@@ -110,7 +110,7 @@ void f5 ()
 **	cmle	v[0-9]+.4s, v[0-9]+.4s, #0
 **	umaxp	v[0-9]+.4s, v[0-9]+.4s, v[0-9]+.4s
 **	fmov	x[0-9]+, d[0-9]+
-**	cbnz	x[0-9]+, \.L[0-9]+
+**	cbn?z	x[0-9]+, \.L[0-9]+
 **	...
 */
 void f6 ()

Comments

Richard Sandiford Jan. 29, 2024, 3:59 p.m. UTC | #1
Tamar Christina <tamar.christina@arm.com> writes:
> Hi All,
>
> Recently something in the midend had started inverting the branches by inverting
> the condition and the branches.
>
> While this is fine, it makes it hard to actually test.  In RTL I disable
> scheduling and BB reordering to prevent this.  But in GIMPLE there seems to be
> nothing I can do.  __builtin_expect seems to have no impact on the change since
> I suspect this is happening during expand where conditions can be flipped
> regardless of probability during compare_and_branch.
>
> Since the mid-end has plenty of correctness tests, this weakens the backend
> tests to just check that a correct looking sequence is emitted.
>
> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
>
> Ok for master?
>
> Thanks,
> Tamar
>
> gcc/testsuite/ChangeLog:
>
> 	PR testsuite/113502
> 	* gcc.target/aarch64/sve/vect-early-break-cbranch.c: Ignore exact branch.
> 	* gcc.target/aarch64/vect-early-break-cbranch.c: Likewise.

OK I guess, since I agree that the "polarity" of the branch isn't really the
thing that we're trying to test.  But the fact that even __builtin_expect
doesn't work seems like a bug.  Do we have a PR for that?  Might be worth
filing one (for GCC 15+) if we don't.

Thanks,
Richard

>
> --- inline copy of patch -- 
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/vect-early-break-cbranch.c b/gcc/testsuite/gcc.target/aarch64/sve/vect-early-break-cbranch.c
> index d15053553f94e7dce3540e21f0c1f0d39ea4f289..d7cef1105410be04ed67d1d3b800746267f205a8 100644
> --- a/gcc/testsuite/gcc.target/aarch64/sve/vect-early-break-cbranch.c
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/vect-early-break-cbranch.c
> @@ -9,7 +9,7 @@ int b[N] = {0};
>  **	...
>  **	cmpgt	p[0-9]+.s, p[0-9]+/z, z[0-9]+.s, #0
>  **	ptest	p[0-9]+, p[0-9]+.b
> -**	b.any	\.L[0-9]+
> +**	b.(any|none)	\.L[0-9]+
>  **	...
>  */
>  void f1 ()
> @@ -26,7 +26,7 @@ void f1 ()
>  **	...
>  **	cmpge	p[0-9]+.s, p[0-9]+/z, z[0-9]+.s, #0
>  **	ptest	p[0-9]+, p[0-9]+.b
> -**	b.any	\.L[0-9]+
> +**	b.(any|none)	\.L[0-9]+
>  **	...
>  */
>  void f2 ()
> @@ -43,7 +43,7 @@ void f2 ()
>  **	...
>  **	cmpeq	p[0-9]+.s, p[0-9]+/z, z[0-9]+.s, #0
>  **	ptest	p[0-9]+, p[0-9]+.b
> -**	b.any	\.L[0-9]+
> +**	b.(any|none)	\.L[0-9]+
>  **	...
>  */
>  void f3 ()
> @@ -60,7 +60,7 @@ void f3 ()
>  **	...
>  **	cmpne	p[0-9]+.s, p[0-9]+/z, z[0-9]+.s, #0
>  **	ptest	p[0-9]+, p[0-9]+.b
> -**	b.any	\.L[0-9]+
> +**	b.(any|none)	\.L[0-9]+
>  **	...
>  */
>  void f4 ()
> @@ -77,7 +77,7 @@ void f4 ()
>  **	...
>  **	cmplt	p[0-9]+.s, p7/z, z[0-9]+.s, #0
>  **	ptest	p[0-9]+, p[0-9]+.b
> -**	b.any	.L[0-9]+
> +**	b.(any|none)	.L[0-9]+
>  **	...
>  */
>  void f5 ()
> @@ -94,7 +94,7 @@ void f5 ()
>  **	...
>  **	cmple	p[0-9]+.s, p[0-9]+/z, z[0-9]+.s, #0
>  **	ptest	p[0-9]+, p[0-9]+.b
> -**	b.any	\.L[0-9]+
> +**	b.(any|none)	\.L[0-9]+
>  **	...
>  */
>  void f6 ()
> diff --git a/gcc/testsuite/gcc.target/aarch64/vect-early-break-cbranch.c b/gcc/testsuite/gcc.target/aarch64/vect-early-break-cbranch.c
> index a5e7b94827dd70240d754a834f1d11750a9c27a9..673b781eb6d092f6311409797b20a971f4fae247 100644
> --- a/gcc/testsuite/gcc.target/aarch64/vect-early-break-cbranch.c
> +++ b/gcc/testsuite/gcc.target/aarch64/vect-early-break-cbranch.c
> @@ -15,7 +15,7 @@ int b[N] = {0};
>  **	cmgt	v[0-9]+.4s, v[0-9]+.4s, #0
>  **	umaxp	v[0-9]+.4s, v[0-9]+.4s, v[0-9]+.4s
>  **	fmov	x[0-9]+, d[0-9]+
> -**	cbnz	x[0-9]+, \.L[0-9]+
> +**	cbn?z	x[0-9]+, \.L[0-9]+
>  **	...
>  */
>  void f1 ()
> @@ -34,7 +34,7 @@ void f1 ()
>  **	cmge	v[0-9]+.4s, v[0-9]+.4s, #0
>  **	umaxp	v[0-9]+.4s, v[0-9]+.4s, v[0-9]+.4s
>  **	fmov	x[0-9]+, d[0-9]+
> -**	cbnz	x[0-9]+, \.L[0-9]+
> +**	cbn?z	x[0-9]+, \.L[0-9]+
>  **	...
>  */
>  void f2 ()
> @@ -53,7 +53,7 @@ void f2 ()
>  **	cmeq	v[0-9]+.4s, v[0-9]+.4s, #0
>  **	umaxp	v[0-9]+.4s, v[0-9]+.4s, v[0-9]+.4s
>  **	fmov	x[0-9]+, d[0-9]+
> -**	cbnz	x[0-9]+, \.L[0-9]+
> +**	cbn?z	x[0-9]+, \.L[0-9]+
>  **	...
>  */
>  void f3 ()
> @@ -72,7 +72,7 @@ void f3 ()
>  **	cmtst	v[0-9]+.4s, v[0-9]+.4s, v[0-9]+.4s
>  **	umaxp	v[0-9]+.4s, v[0-9]+.4s, v[0-9]+.4s
>  **	fmov	x[0-9]+, d[0-9]+
> -**	cbnz	x[0-9]+, \.L[0-9]+
> +**	cbn?z	x[0-9]+, \.L[0-9]+
>  **	...
>  */
>  void f4 ()
> @@ -91,7 +91,7 @@ void f4 ()
>  **	cmlt	v[0-9]+.4s, v[0-9]+.4s, #0
>  **	umaxp	v[0-9]+.4s, v[0-9]+.4s, v[0-9]+.4s
>  **	fmov	x[0-9]+, d[0-9]+
> -**	cbnz	x[0-9]+, \.L[0-9]+
> +**	cbn?z	x[0-9]+, \.L[0-9]+
>  **	...
>  */
>  void f5 ()
> @@ -110,7 +110,7 @@ void f5 ()
>  **	cmle	v[0-9]+.4s, v[0-9]+.4s, #0
>  **	umaxp	v[0-9]+.4s, v[0-9]+.4s, v[0-9]+.4s
>  **	fmov	x[0-9]+, d[0-9]+
> -**	cbnz	x[0-9]+, \.L[0-9]+
> +**	cbn?z	x[0-9]+, \.L[0-9]+
>  **	...
>  */
>  void f6 ()
Richard Biener Jan. 30, 2024, 7:18 a.m. UTC | #2
On Mon, Jan 29, 2024 at 5:00 PM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> Tamar Christina <tamar.christina@arm.com> writes:
> > Hi All,
> >
> > Recently something in the midend had started inverting the branches by inverting
> > the condition and the branches.
> >
> > While this is fine, it makes it hard to actually test.  In RTL I disable
> > scheduling and BB reordering to prevent this.  But in GIMPLE there seems to be
> > nothing I can do.  __builtin_expect seems to have no impact on the change since
> > I suspect this is happening during expand where conditions can be flipped
> > regardless of probability during compare_and_branch.
> >
> > Since the mid-end has plenty of correctness tests, this weakens the backend
> > tests to just check that a correct looking sequence is emitted.
> >
> > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> >
> > Ok for master?
> >
> > Thanks,
> > Tamar
> >
> > gcc/testsuite/ChangeLog:
> >
> >       PR testsuite/113502
> >       * gcc.target/aarch64/sve/vect-early-break-cbranch.c: Ignore exact branch.
> >       * gcc.target/aarch64/vect-early-break-cbranch.c: Likewise.
>
> OK I guess, since I agree that the "polarity" of the branch isn't really the
> thing that we're trying to test.  But the fact that even __builtin_expect
> doesn't work seems like a bug.  Do we have a PR for that?  Might be worth
> filing one (for GCC 15+) if we don't.

But that only should affect which edge is fallthru, not the "polarity"
of the branch, no?

> Thanks,
> Richard
>
> >
> > --- inline copy of patch --
> > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/vect-early-break-cbranch.c b/gcc/testsuite/gcc.target/aarch64/sve/vect-early-break-cbranch.c
> > index d15053553f94e7dce3540e21f0c1f0d39ea4f289..d7cef1105410be04ed67d1d3b800746267f205a8 100644
> > --- a/gcc/testsuite/gcc.target/aarch64/sve/vect-early-break-cbranch.c
> > +++ b/gcc/testsuite/gcc.target/aarch64/sve/vect-early-break-cbranch.c
> > @@ -9,7 +9,7 @@ int b[N] = {0};
> >  **   ...
> >  **   cmpgt   p[0-9]+.s, p[0-9]+/z, z[0-9]+.s, #0
> >  **   ptest   p[0-9]+, p[0-9]+.b
> > -**   b.any   \.L[0-9]+
> > +**   b.(any|none)    \.L[0-9]+
> >  **   ...
> >  */
> >  void f1 ()
> > @@ -26,7 +26,7 @@ void f1 ()
> >  **   ...
> >  **   cmpge   p[0-9]+.s, p[0-9]+/z, z[0-9]+.s, #0
> >  **   ptest   p[0-9]+, p[0-9]+.b
> > -**   b.any   \.L[0-9]+
> > +**   b.(any|none)    \.L[0-9]+
> >  **   ...
> >  */
> >  void f2 ()
> > @@ -43,7 +43,7 @@ void f2 ()
> >  **   ...
> >  **   cmpeq   p[0-9]+.s, p[0-9]+/z, z[0-9]+.s, #0
> >  **   ptest   p[0-9]+, p[0-9]+.b
> > -**   b.any   \.L[0-9]+
> > +**   b.(any|none)    \.L[0-9]+
> >  **   ...
> >  */
> >  void f3 ()
> > @@ -60,7 +60,7 @@ void f3 ()
> >  **   ...
> >  **   cmpne   p[0-9]+.s, p[0-9]+/z, z[0-9]+.s, #0
> >  **   ptest   p[0-9]+, p[0-9]+.b
> > -**   b.any   \.L[0-9]+
> > +**   b.(any|none)    \.L[0-9]+
> >  **   ...
> >  */
> >  void f4 ()
> > @@ -77,7 +77,7 @@ void f4 ()
> >  **   ...
> >  **   cmplt   p[0-9]+.s, p7/z, z[0-9]+.s, #0
> >  **   ptest   p[0-9]+, p[0-9]+.b
> > -**   b.any   .L[0-9]+
> > +**   b.(any|none)    .L[0-9]+
> >  **   ...
> >  */
> >  void f5 ()
> > @@ -94,7 +94,7 @@ void f5 ()
> >  **   ...
> >  **   cmple   p[0-9]+.s, p[0-9]+/z, z[0-9]+.s, #0
> >  **   ptest   p[0-9]+, p[0-9]+.b
> > -**   b.any   \.L[0-9]+
> > +**   b.(any|none)    \.L[0-9]+
> >  **   ...
> >  */
> >  void f6 ()
> > diff --git a/gcc/testsuite/gcc.target/aarch64/vect-early-break-cbranch.c b/gcc/testsuite/gcc.target/aarch64/vect-early-break-cbranch.c
> > index a5e7b94827dd70240d754a834f1d11750a9c27a9..673b781eb6d092f6311409797b20a971f4fae247 100644
> > --- a/gcc/testsuite/gcc.target/aarch64/vect-early-break-cbranch.c
> > +++ b/gcc/testsuite/gcc.target/aarch64/vect-early-break-cbranch.c
> > @@ -15,7 +15,7 @@ int b[N] = {0};
> >  **   cmgt    v[0-9]+.4s, v[0-9]+.4s, #0
> >  **   umaxp   v[0-9]+.4s, v[0-9]+.4s, v[0-9]+.4s
> >  **   fmov    x[0-9]+, d[0-9]+
> > -**   cbnz    x[0-9]+, \.L[0-9]+
> > +**   cbn?z   x[0-9]+, \.L[0-9]+
> >  **   ...
> >  */
> >  void f1 ()
> > @@ -34,7 +34,7 @@ void f1 ()
> >  **   cmge    v[0-9]+.4s, v[0-9]+.4s, #0
> >  **   umaxp   v[0-9]+.4s, v[0-9]+.4s, v[0-9]+.4s
> >  **   fmov    x[0-9]+, d[0-9]+
> > -**   cbnz    x[0-9]+, \.L[0-9]+
> > +**   cbn?z   x[0-9]+, \.L[0-9]+
> >  **   ...
> >  */
> >  void f2 ()
> > @@ -53,7 +53,7 @@ void f2 ()
> >  **   cmeq    v[0-9]+.4s, v[0-9]+.4s, #0
> >  **   umaxp   v[0-9]+.4s, v[0-9]+.4s, v[0-9]+.4s
> >  **   fmov    x[0-9]+, d[0-9]+
> > -**   cbnz    x[0-9]+, \.L[0-9]+
> > +**   cbn?z   x[0-9]+, \.L[0-9]+
> >  **   ...
> >  */
> >  void f3 ()
> > @@ -72,7 +72,7 @@ void f3 ()
> >  **   cmtst   v[0-9]+.4s, v[0-9]+.4s, v[0-9]+.4s
> >  **   umaxp   v[0-9]+.4s, v[0-9]+.4s, v[0-9]+.4s
> >  **   fmov    x[0-9]+, d[0-9]+
> > -**   cbnz    x[0-9]+, \.L[0-9]+
> > +**   cbn?z   x[0-9]+, \.L[0-9]+
> >  **   ...
> >  */
> >  void f4 ()
> > @@ -91,7 +91,7 @@ void f4 ()
> >  **   cmlt    v[0-9]+.4s, v[0-9]+.4s, #0
> >  **   umaxp   v[0-9]+.4s, v[0-9]+.4s, v[0-9]+.4s
> >  **   fmov    x[0-9]+, d[0-9]+
> > -**   cbnz    x[0-9]+, \.L[0-9]+
> > +**   cbn?z   x[0-9]+, \.L[0-9]+
> >  **   ...
> >  */
> >  void f5 ()
> > @@ -110,7 +110,7 @@ void f5 ()
> >  **   cmle    v[0-9]+.4s, v[0-9]+.4s, #0
> >  **   umaxp   v[0-9]+.4s, v[0-9]+.4s, v[0-9]+.4s
> >  **   fmov    x[0-9]+, d[0-9]+
> > -**   cbnz    x[0-9]+, \.L[0-9]+
> > +**   cbn?z   x[0-9]+, \.L[0-9]+
> >  **   ...
> >  */
> >  void f6 ()
Richard Sandiford Jan. 30, 2024, 10:40 a.m. UTC | #3
Richard Biener <richard.guenther@gmail.com> writes:
> On Mon, Jan 29, 2024 at 5:00 PM Richard Sandiford
> <richard.sandiford@arm.com> wrote:
>>
>> Tamar Christina <tamar.christina@arm.com> writes:
>> > Hi All,
>> >
>> > Recently something in the midend had started inverting the branches by inverting
>> > the condition and the branches.
>> >
>> > While this is fine, it makes it hard to actually test.  In RTL I disable
>> > scheduling and BB reordering to prevent this.  But in GIMPLE there seems to be
>> > nothing I can do.  __builtin_expect seems to have no impact on the change since
>> > I suspect this is happening during expand where conditions can be flipped
>> > regardless of probability during compare_and_branch.
>> >
>> > Since the mid-end has plenty of correctness tests, this weakens the backend
>> > tests to just check that a correct looking sequence is emitted.
>> >
>> > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
>> >
>> > Ok for master?
>> >
>> > Thanks,
>> > Tamar
>> >
>> > gcc/testsuite/ChangeLog:
>> >
>> >       PR testsuite/113502
>> >       * gcc.target/aarch64/sve/vect-early-break-cbranch.c: Ignore exact branch.
>> >       * gcc.target/aarch64/vect-early-break-cbranch.c: Likewise.
>>
>> OK I guess, since I agree that the "polarity" of the branch isn't really the
>> thing that we're trying to test.  But the fact that even __builtin_expect
>> doesn't work seems like a bug.  Do we have a PR for that?  Might be worth
>> filing one (for GCC 15+) if we don't.
>
> But that only should affect which edge is fallthru, not the "polarity"
> of the branch, no?

Right, that's what I meant by "polarity".  I should have used a proper
term, sorry.  The conditions in the patch are inverses of each other,
but it seems like the choice between them should be predictable if
the branch is very likely or very unlikely.

Thanks,
Richard
Richard Biener Jan. 30, 2024, 10:48 a.m. UTC | #4
On Tue, Jan 30, 2024 at 11:40 AM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> Richard Biener <richard.guenther@gmail.com> writes:
> > On Mon, Jan 29, 2024 at 5:00 PM Richard Sandiford
> > <richard.sandiford@arm.com> wrote:
> >>
> >> Tamar Christina <tamar.christina@arm.com> writes:
> >> > Hi All,
> >> >
> >> > Recently something in the midend had started inverting the branches by inverting
> >> > the condition and the branches.
> >> >
> >> > While this is fine, it makes it hard to actually test.  In RTL I disable
> >> > scheduling and BB reordering to prevent this.  But in GIMPLE there seems to be
> >> > nothing I can do.  __builtin_expect seems to have no impact on the change since
> >> > I suspect this is happening during expand where conditions can be flipped
> >> > regardless of probability during compare_and_branch.
> >> >
> >> > Since the mid-end has plenty of correctness tests, this weakens the backend
> >> > tests to just check that a correct looking sequence is emitted.
> >> >
> >> > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> >> >
> >> > Ok for master?
> >> >
> >> > Thanks,
> >> > Tamar
> >> >
> >> > gcc/testsuite/ChangeLog:
> >> >
> >> >       PR testsuite/113502
> >> >       * gcc.target/aarch64/sve/vect-early-break-cbranch.c: Ignore exact branch.
> >> >       * gcc.target/aarch64/vect-early-break-cbranch.c: Likewise.
> >>
> >> OK I guess, since I agree that the "polarity" of the branch isn't really the
> >> thing that we're trying to test.  But the fact that even __builtin_expect
> >> doesn't work seems like a bug.  Do we have a PR for that?  Might be worth
> >> filing one (for GCC 15+) if we don't.
> >
> > But that only should affect which edge is fallthru, not the "polarity"
> > of the branch, no?
>
> Right, that's what I meant by "polarity".  I should have used a proper
> term, sorry.  The conditions in the patch are inverses of each other,
> but it seems like the choice between them should be predictable if
> the branch is very likely or very unlikely.

I'm not sure, esp. if you disable BB reordering which might be the one
flipping the polarity to keep fallthru/backedge in-order.

Richard.

> Thanks,
> Richard
>
diff mbox series

Patch

--- a/gcc/testsuite/gcc.target/aarch64/sve/vect-early-break-cbranch.c
+++ b/gcc/testsuite/gcc.target/aarch64/sve/vect-early-break-cbranch.c
@@ -9,7 +9,7 @@  int b[N] = {0};
 **	...
 **	cmpgt	p[0-9]+.s, p[0-9]+/z, z[0-9]+.s, #0
 **	ptest	p[0-9]+, p[0-9]+.b
-**	b.any	\.L[0-9]+
+**	b.(any|none)	\.L[0-9]+
 **	...
 */
 void f1 ()
@@ -26,7 +26,7 @@  void f1 ()
 **	...
 **	cmpge	p[0-9]+.s, p[0-9]+/z, z[0-9]+.s, #0
 **	ptest	p[0-9]+, p[0-9]+.b
-**	b.any	\.L[0-9]+
+**	b.(any|none)	\.L[0-9]+
 **	...
 */
 void f2 ()
@@ -43,7 +43,7 @@  void f2 ()
 **	...
 **	cmpeq	p[0-9]+.s, p[0-9]+/z, z[0-9]+.s, #0
 **	ptest	p[0-9]+, p[0-9]+.b
-**	b.any	\.L[0-9]+
+**	b.(any|none)	\.L[0-9]+
 **	...
 */
 void f3 ()
@@ -60,7 +60,7 @@  void f3 ()
 **	...
 **	cmpne	p[0-9]+.s, p[0-9]+/z, z[0-9]+.s, #0
 **	ptest	p[0-9]+, p[0-9]+.b
-**	b.any	\.L[0-9]+
+**	b.(any|none)	\.L[0-9]+
 **	...
 */
 void f4 ()
@@ -77,7 +77,7 @@  void f4 ()
 **	...
 **	cmplt	p[0-9]+.s, p7/z, z[0-9]+.s, #0
 **	ptest	p[0-9]+, p[0-9]+.b
-**	b.any	.L[0-9]+
+**	b.(any|none)	.L[0-9]+
 **	...
 */
 void f5 ()
@@ -94,7 +94,7 @@  void f5 ()
 **	...
 **	cmple	p[0-9]+.s, p[0-9]+/z, z[0-9]+.s, #0
 **	ptest	p[0-9]+, p[0-9]+.b
-**	b.any	\.L[0-9]+
+**	b.(any|none)	\.L[0-9]+
 **	...
 */
 void f6 ()
diff --git a/gcc/testsuite/gcc.target/aarch64/vect-early-break-cbranch.c b/gcc/testsuite/gcc.target/aarch64/vect-early-break-cbranch.c
index a5e7b94827dd70240d754a834f1d11750a9c27a9..673b781eb6d092f6311409797b20a971f4fae247 100644
--- a/gcc/testsuite/gcc.target/aarch64/vect-early-break-cbranch.c
+++ b/gcc/testsuite/gcc.target/aarch64/vect-early-break-cbranch.c
@@ -15,7 +15,7 @@  int b[N] = {0};
 **	cmgt	v[0-9]+.4s, v[0-9]+.4s, #0
 **	umaxp	v[0-9]+.4s, v[0-9]+.4s, v[0-9]+.4s
 **	fmov	x[0-9]+, d[0-9]+
-**	cbnz	x[0-9]+, \.L[0-9]+
+**	cbn?z	x[0-9]+, \.L[0-9]+
 **	...
 */
 void f1 ()
@@ -34,7 +34,7 @@  void f1 ()
 **	cmge	v[0-9]+.4s, v[0-9]+.4s, #0
 **	umaxp	v[0-9]+.4s, v[0-9]+.4s, v[0-9]+.4s
 **	fmov	x[0-9]+, d[0-9]+
-**	cbnz	x[0-9]+, \.L[0-9]+
+**	cbn?z	x[0-9]+, \.L[0-9]+
 **	...
 */
 void f2 ()
@@ -53,7 +53,7 @@  void f2 ()
 **	cmeq	v[0-9]+.4s, v[0-9]+.4s, #0
 **	umaxp	v[0-9]+.4s, v[0-9]+.4s, v[0-9]+.4s
 **	fmov	x[0-9]+, d[0-9]+
-**	cbnz	x[0-9]+, \.L[0-9]+
+**	cbn?z	x[0-9]+, \.L[0-9]+
 **	...
 */
 void f3 ()
@@ -72,7 +72,7 @@  void f3 ()
 **	cmtst	v[0-9]+.4s, v[0-9]+.4s, v[0-9]+.4s
 **	umaxp	v[0-9]+.4s, v[0-9]+.4s, v[0-9]+.4s
 **	fmov	x[0-9]+, d[0-9]+
-**	cbnz	x[0-9]+, \.L[0-9]+
+**	cbn?z	x[0-9]+, \.L[0-9]+
 **	...
 */
 void f4 ()
@@ -91,7 +91,7 @@  void f4 ()
 **	cmlt	v[0-9]+.4s, v[0-9]+.4s, #0
 **	umaxp	v[0-9]+.4s, v[0-9]+.4s, v[0-9]+.4s
 **	fmov	x[0-9]+, d[0-9]+
-**	cbnz	x[0-9]+, \.L[0-9]+
+**	cbn?z	x[0-9]+, \.L[0-9]+
 **	...
 */
 void f5 ()
@@ -110,7 +110,7 @@  void f5 ()
 **	cmle	v[0-9]+.4s, v[0-9]+.4s, #0
 **	umaxp	v[0-9]+.4s, v[0-9]+.4s, v[0-9]+.4s
 **	fmov	x[0-9]+, d[0-9]+
-**	cbnz	x[0-9]+, \.L[0-9]+
+**	cbn?z	x[0-9]+, \.L[0-9]+
 **	...
 */
 void f6 ()