On Thu, Dec 04, 2025 at 03:27:53PM +0100, Jan Hubicka wrote:
> > Starting with r16-4438-ga93f80feeef744, the edges started to be sorted
> > in ascending order.  Thus the most likely branches were deprioritized
> > instead of optimized to fallthroughs.
> > 
> > Fix by restoring the sorting order prior to r16-4438-ga93f80feeef744.
> > 
> > There are no regressions for C and C++ on x86_64-pc-linux-gnu.
> > 
> > The new tests fail for the respective targets without this patch, and
> > pass with it.
> > 
> >     PR rtl-optimization/122675
> > 
> > gcc/ChangeLog:
> > 
> >     * bb-reorder.cc (edge_order): Fix BB edge ordering to be
> >     descending.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> >     * gcc.target/aarch64/pr122675-1.c: New test.
> >     * gcc.target/i386/pr122675-1.c: New test.
> >     * gcc.target/riscv/pr122675-1.c: New test.
> 
> This is OK.
> Thanks for cathing this and sorry for late reply - I was organizing a 
> workshop.
> 
> Honza

Thank you Honza. Could you please confirm that you prefer this version,
and not the latter [V3] where I addressed Jeff's concerns?

Regards,
Dimitar

[V3] https://gcc.gnu.org/pipermail/gcc-patches/2025-November/702339.html

> > 
> > Signed-off-by: Dimitar Dimitrov <[email protected]>
> > ---
> > Changes since v1:
> >   - Added tests utilizing check-function-bodies, per suggestion from
> >     Andrew Pinski.
> > 
> >  gcc/bb-reorder.cc                             |  7 +++-
> >  gcc/testsuite/gcc.target/aarch64/pr122675-1.c | 31 ++++++++++++++
> >  gcc/testsuite/gcc.target/i386/pr122675-1.c    | 33 +++++++++++++++
> >  gcc/testsuite/gcc.target/riscv/pr122675-1.c   | 42 +++++++++++++++++++
> >  4 files changed, 112 insertions(+), 1 deletion(-)
> >  create mode 100644 gcc/testsuite/gcc.target/aarch64/pr122675-1.c
> >  create mode 100644 gcc/testsuite/gcc.target/i386/pr122675-1.c
> >  create mode 100644 gcc/testsuite/gcc.target/riscv/pr122675-1.c
> > 
> > diff --git a/gcc/bb-reorder.cc b/gcc/bb-reorder.cc
> > index e4efdee0b16..43fa019984a 100644
> > --- a/gcc/bb-reorder.cc
> > +++ b/gcc/bb-reorder.cc
> > @@ -2392,7 +2392,12 @@ edge_order (const void *ve1, const void *ve2)
> >    gcov_type gc1 = c1.initialized_p () ? c1.to_gcov_type () : 0;
> >    gcov_type gc2 = c2.initialized_p () ? c2.to_gcov_type () : 0;
> >    gcov_type m = MAX (gc1, gc2);
> > -  return (m == gc1) - (m == gc2);
> > +  /* While gcc_stablesort sorts in ascending order, the edges should
> > +     be sorted in descending order of their execution frequency.
> > +     So return a reversed comparison.  Expressed with a spaceship operator:
> > +   return gc2 <=> gc1;
> > +   */
> > +  return (m == gc2) - (m == gc1);
> >  }
> >  
> >  /* Reorder basic blocks using the "simple" algorithm.  This tries to
> > diff --git a/gcc/testsuite/gcc.target/aarch64/pr122675-1.c 
> > b/gcc/testsuite/gcc.target/aarch64/pr122675-1.c
> > new file mode 100644
> > index 00000000000..8d2982ac21e
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/aarch64/pr122675-1.c
> > @@ -0,0 +1,31 @@
> > +/* Verify that the most likely BB edges are optimized as fallthroughs.  */
> > +/* { dg-do compile } */
> > +/* { dg-options "-O1 -fno-pic -mtune=generic -march=armv8-a" } */
> > +/* Keep labels and directives ('.cfi_startproc', '.cfi_endproc').  */
> > +/* { dg-final { check-function-bodies "**" "" "" { target *-*-* } {^\t?\.} 
> > } } */
> > +
> > +/*
> > +**test:
> > +**.LFB[0-9]+:
> > +** .cfi_startproc
> > +**...
> > +** cbz     w0, .L[0-9]*
> > +**...
> > +** bl      f1
> > +**...
> > +** ret
> > +**.L[0-9]+:
> > +**...
> > +** ret
> > +**...
> > +*/
> > +
> > +int f1(void);
> > +
> > +int test(int a)
> > +{
> > +  if (__builtin_expect(!!a, 1)) {
> > +    return f1();
> > +  }
> > +  return a;
> > +}
> > diff --git a/gcc/testsuite/gcc.target/i386/pr122675-1.c 
> > b/gcc/testsuite/gcc.target/i386/pr122675-1.c
> > new file mode 100644
> > index 00000000000..fb41f5b2788
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/i386/pr122675-1.c
> > @@ -0,0 +1,33 @@
> > +/* Verify that the most likely BB edges are optimized as fallthroughs.  */
> > +/* { dg-do compile } */
> > +/* { dg-options "-O1 -fno-pic -march=x86-64 -mtune=generic 
> > -mgeneral-regs-only" } */
> > +/* Keep labels and directives ('.cfi_startproc', '.cfi_endproc').  */
> > +/* { dg-final { check-function-bodies "**" "" "" { target lp64 } {^\t?\.} 
> > } } */
> > +
> > +/*
> > +**test:
> > +**.LFB[0-9]+:
> > +** .cfi_startproc
> > +** testl   %edi, %edi
> > +** je      .L[0-9]*
> > +** subq    \$[0-9]*, %rsp
> > +** .cfi_def_cfa_offset [0-9]*
> > +** call    f1
> > +** addq    \$[0-9]*, %rsp
> > +** .cfi_def_cfa_offset [0-9]*
> > +** ret
> > +**.L[0-9]+:
> > +** movl    %edi, %eax
> > +** ret
> > +**...
> > +*/
> > +
> > +int f1(void);
> > +
> > +int test(int a)
> > +{
> > +  if (__builtin_expect(!!a, 1)) {
> > +    return f1();
> > +  }
> > +  return a;
> > +}
> > diff --git a/gcc/testsuite/gcc.target/riscv/pr122675-1.c 
> > b/gcc/testsuite/gcc.target/riscv/pr122675-1.c
> > new file mode 100644
> > index 00000000000..6f49ef3b5d8
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/riscv/pr122675-1.c
> > @@ -0,0 +1,42 @@
> > +/* Verify that the most likely BB edges are optimized as fallthroughs.  */
> > +/* { dg-do compile } */
> > +/* { dg-options "-O1 -fno-pic -march=rv64gc -mabi=lp64d" } */
> > +/* Keep labels and directives ('.cfi_startproc', '.cfi_endproc').  */
> > +/* { dg-final { check-function-bodies "**" "" "" { target *-*-* } {^\t?\.} 
> > } } */
> > +
> > +/*
> > +**test:
> > +**...
> > +**.LFB[0-9]+:
> > +**...
> > +** .cfi_startproc
> > +**...
> > +** beq     a0,zero,.L[0-9]*
> > +**...
> > +** call    f1
> > +**...
> > +** (
> > +** jr      ra
> > +** |
> > +** ret
> > +** )
> > +**...
> > +**.L[0-9]+:
> > +**...
> > +** (
> > +** jr      ra
> > +** |
> > +** ret
> > +** )
> > +**...
> > +*/
> > +
> > +int f1(void);
> > +
> > +int test(int a)
> > +{
> > +  if (__builtin_expect(!!a, 1)) {
> > +    return f1();
> > +  }
> > +  return a;
> > +}
> > -- 
> > 2.51.1
> > 

Reply via email to