* [PATCH]AArch64: relax cbranch tests to accepted inverted branches [PR113502]
@ 2024-01-29 15:04 Tamar Christina
2024-01-29 15:59 ` Richard Sandiford
0 siblings, 1 reply; 5+ messages in thread
From: Tamar Christina @ 2024-01-29 15:04 UTC (permalink / raw)
To: gcc-patches
Cc: nd, Richard.Earnshaw, Marcus.Shawcroft, Kyrylo.Tkachov,
richard.sandiford
[-- Attachment #1: Type: text/plain, Size: 4094 bytes --]
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
--- 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 ()
--
[-- Attachment #2: rb18218.patch --]
[-- Type: text/plain, Size: 3157 bytes --]
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 ()
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH]AArch64: relax cbranch tests to accepted inverted branches [PR113502]
2024-01-29 15:04 [PATCH]AArch64: relax cbranch tests to accepted inverted branches [PR113502] Tamar Christina
@ 2024-01-29 15:59 ` Richard Sandiford
2024-01-30 7:18 ` Richard Biener
0 siblings, 1 reply; 5+ messages in thread
From: Richard Sandiford @ 2024-01-29 15:59 UTC (permalink / raw)
To: Tamar Christina
Cc: gcc-patches, nd, Richard.Earnshaw, Marcus.Shawcroft, Kyrylo.Tkachov
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 ()
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH]AArch64: relax cbranch tests to accepted inverted branches [PR113502]
2024-01-29 15:59 ` Richard Sandiford
@ 2024-01-30 7:18 ` Richard Biener
2024-01-30 10:40 ` Richard Sandiford
0 siblings, 1 reply; 5+ messages in thread
From: Richard Biener @ 2024-01-30 7:18 UTC (permalink / raw)
To: Tamar Christina, gcc-patches, nd, Richard.Earnshaw,
Marcus.Shawcroft, Kyrylo.Tkachov, richard.sandiford
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 ()
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH]AArch64: relax cbranch tests to accepted inverted branches [PR113502]
2024-01-30 7:18 ` Richard Biener
@ 2024-01-30 10:40 ` Richard Sandiford
2024-01-30 10:48 ` Richard Biener
0 siblings, 1 reply; 5+ messages in thread
From: Richard Sandiford @ 2024-01-30 10:40 UTC (permalink / raw)
To: Richard Biener
Cc: Tamar Christina, gcc-patches, nd, Richard.Earnshaw,
Marcus.Shawcroft, Kyrylo.Tkachov
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH]AArch64: relax cbranch tests to accepted inverted branches [PR113502]
2024-01-30 10:40 ` Richard Sandiford
@ 2024-01-30 10:48 ` Richard Biener
0 siblings, 0 replies; 5+ messages in thread
From: Richard Biener @ 2024-01-30 10:48 UTC (permalink / raw)
To: Richard Biener, Tamar Christina, gcc-patches, nd,
Richard.Earnshaw, Marcus.Shawcroft, Kyrylo.Tkachov,
richard.sandiford
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
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-01-30 10:54 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-29 15:04 [PATCH]AArch64: relax cbranch tests to accepted inverted branches [PR113502] Tamar Christina
2024-01-29 15:59 ` Richard Sandiford
2024-01-30 7:18 ` Richard Biener
2024-01-30 10:40 ` Richard Sandiford
2024-01-30 10:48 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).