public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [RFA] Minor improvement to coremark, avoid unconditional jump to return
@ 2022-09-25 16:28 Jeff Law
  2022-09-26  7:53 ` Richard Biener
  2022-10-07 10:51 ` Franz Sirl
  0 siblings, 2 replies; 6+ messages in thread
From: Jeff Law @ 2022-09-25 16:28 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1295 bytes --]

This is a minor improvement for the core_list_find routine in coremark.


Basically for riscv, and likely other targets, we can end up with an 
unconditional jump to a return statement.    This is a result of 
compensation code created by bb-reorder, and no jump optimization pass 
runs after bb-reorder to clean this stuff up.

This patch utilizes preexisting code to identify suitable branch targets 
as well as preexisting code to emit a suitable return, so it's pretty 
simple.  Note that when we arrange to do this optimization, the original 
return block may become unreachable. So we conditionally call 
delete_unreachable_blocks to fix that up.

This triggers ~160 times during an x86_64 bootstrap.  Naturally it 
bootstraps and regression tests on x86_64.

I've also bootstrapped this on riscv64, regression testing with qemu 
shows some regressions, but AFAICT they're actually qemu bugs with 
signal handling/delivery -- qemu user mode emulation is not consistently 
calling user defined signal handlers.  Given the same binary, sometimes 
they'll get called and the test passes, other times the handler isn't 
called and the test (of course) fails. I'll probably spend some time to 
try and chase this down for the sake of making testing easier.


OK for the trunk?


Jeff





[-- Attachment #2: J --]
[-- Type: text/plain, Size: 4394 bytes --]

commit f9a9119fa47f94348305a883fd88c23647fb1b07
Author: Jeff Law <jeffreyalaw@gmail.com>
Date:   Sun Sep 25 12:23:59 2022 -0400

    gcc/
            * cfgcleanup.cc (bb_is_just_return): No longer static.
            * cfgcleanup.h (bb_is_just_return): Add prototype.
            * cfgrtl.cc (fixup_reorder_chain): Do not create an
            unconditional jump to a return block.  Conditionally
            remove unreachable blocks.
    
    gcc/testsuite/gcc.target/riscv/
    
            * ret-1.c: New test.

diff --git a/gcc/cfgcleanup.cc b/gcc/cfgcleanup.cc
index a8b0139bb4d..a363e0b4da3 100644
--- a/gcc/cfgcleanup.cc
+++ b/gcc/cfgcleanup.cc
@@ -2599,7 +2599,7 @@ trivially_empty_bb_p (basic_block bb)
    return value.  Fill in *RET and *USE with the return and use insns
    if any found, otherwise NULL.  All CLOBBERs are ignored.  */
 
-static bool
+bool
 bb_is_just_return (basic_block bb, rtx_insn **ret, rtx_insn **use)
 {
   *ret = *use = NULL;
diff --git a/gcc/cfgcleanup.h b/gcc/cfgcleanup.h
index a6d882f98a4..f1021ca835f 100644
--- a/gcc/cfgcleanup.h
+++ b/gcc/cfgcleanup.h
@@ -30,5 +30,6 @@ extern int flow_find_head_matching_sequence (basic_block, basic_block,
 extern bool delete_unreachable_blocks (void);
 extern void delete_dead_jumptables (void);
 extern bool cleanup_cfg (int);
+extern bool bb_is_just_return (basic_block, rtx_insn **, rtx_insn **);
 
 #endif /* GCC_CFGCLEANUP_H */
diff --git a/gcc/cfgrtl.cc b/gcc/cfgrtl.cc
index a05c338a4c8..90cd6ee56a7 100644
--- a/gcc/cfgrtl.cc
+++ b/gcc/cfgrtl.cc
@@ -3901,6 +3901,7 @@ fixup_reorder_chain (void)
   /* Now add jumps and labels as needed to match the blocks new
      outgoing edges.  */
 
+  bool remove_unreachable_blocks = false;
   for (bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb; bb ; bb = (basic_block)
        bb->aux)
     {
@@ -4043,10 +4044,30 @@ fixup_reorder_chain (void)
 	    continue;
 	}
 
+      /* If E_FALL->dest is just a return block, then we can emit a
+	 return rather than a jump to the return block.  */
+      rtx_insn *ret, *use;
+      basic_block dest;
+      if (bb_is_just_return (e_fall->dest, &ret, &use)
+	  && (PATTERN (ret) == simple_return_rtx || PATTERN (ret) == ret_rtx))
+	{
+	  ret_label = PATTERN (ret);
+	  dest = EXIT_BLOCK_PTR_FOR_FN (cfun);
+
+	  /* E_FALL->dest might become unreachable as a result of
+	     replacing the jump with a return.  So arrange to remove
+	     unreachable blocks.  */
+	  remove_unreachable_blocks = true;
+	}
+      else
+	{
+	  dest = e_fall->dest;
+	}
+
       /* We got here if we need to add a new jump insn. 
 	 Note force_nonfallthru can delete E_FALL and thus we have to
 	 save E_FALL->src prior to the call to force_nonfallthru.  */
-      nb = force_nonfallthru_and_redirect (e_fall, e_fall->dest, ret_label);
+      nb = force_nonfallthru_and_redirect (e_fall, dest, ret_label);
       if (nb)
 	{
 	  nb->aux = bb->aux;
@@ -4134,6 +4155,12 @@ fixup_reorder_chain (void)
 		  ei_next (&ei2);
 	    }
       }
+
+  /* Replacing a jump with a return may have exposed an unreachable
+     block.  Conditionally remove them if such transformations were
+     made.  */
+  if (remove_unreachable_blocks)
+    delete_unreachable_blocks ();
 }
 \f
 /* Perform sanity checks on the insn chain.
diff --git a/gcc/testsuite/gcc.target/riscv/ret-1.c b/gcc/testsuite/gcc.target/riscv/ret-1.c
new file mode 100644
index 00000000000..28133aa4226
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/ret-1.c
@@ -0,0 +1,41 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -dp" } */
+/* This was extracted from coremark.  */
+
+
+typedef signed short ee_s16;
+typedef struct list_data_s
+{
+    ee_s16 data16;
+    ee_s16 idx;
+} list_data;
+
+typedef struct list_head_s
+{
+    struct list_head_s *next;
+    struct list_data_s *info;
+} list_head;
+
+
+list_head *
+core_list_find(list_head *list, list_data *info)
+{
+    if (info->idx >= 0)
+    {
+        while (list && (list->info->idx != info->idx))
+            list = list->next;
+        return list;
+    }
+    else
+    {
+        while (list && ((list->info->data16 & 0xff) != info->data16))
+            list = list->next;
+        return list;
+    }
+}
+
+/* There is only one legitimate unconditional jump, so test for that,
+   which will catch the case where bb-reorder leaves a jump to a ret
+   in the IL.  */
+/* { dg-final { scan-assembler-times "jump" 1 } } */
+

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFA] Minor improvement to coremark, avoid unconditional jump to return
  2022-09-25 16:28 [RFA] Minor improvement to coremark, avoid unconditional jump to return Jeff Law
@ 2022-09-26  7:53 ` Richard Biener
  2022-10-07 10:51 ` Franz Sirl
  1 sibling, 0 replies; 6+ messages in thread
From: Richard Biener @ 2022-09-26  7:53 UTC (permalink / raw)
  To: Jeff Law; +Cc: gcc-patches

On Sun, Sep 25, 2022 at 6:29 PM Jeff Law <jlaw@ventanamicro.com> wrote:
>
> This is a minor improvement for the core_list_find routine in coremark.
>
>
> Basically for riscv, and likely other targets, we can end up with an
> unconditional jump to a return statement.    This is a result of
> compensation code created by bb-reorder, and no jump optimization pass
> runs after bb-reorder to clean this stuff up.
>
> This patch utilizes preexisting code to identify suitable branch targets
> as well as preexisting code to emit a suitable return, so it's pretty
> simple.  Note that when we arrange to do this optimization, the original
> return block may become unreachable. So we conditionally call
> delete_unreachable_blocks to fix that up.
>
> This triggers ~160 times during an x86_64 bootstrap.  Naturally it
> bootstraps and regression tests on x86_64.
>
> I've also bootstrapped this on riscv64, regression testing with qemu
> shows some regressions, but AFAICT they're actually qemu bugs with
> signal handling/delivery -- qemu user mode emulation is not consistently
> calling user defined signal handlers.  Given the same binary, sometimes
> they'll get called and the test passes, other times the handler isn't
> called and the test (of course) fails. I'll probably spend some time to
> try and chase this down for the sake of making testing easier.
>
>
> OK for the trunk?

OK.

Thanks,
Richard.

>
>
> Jeff
>
>
>
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFA] Minor improvement to coremark, avoid unconditional jump to return
  2022-09-25 16:28 [RFA] Minor improvement to coremark, avoid unconditional jump to return Jeff Law
  2022-09-26  7:53 ` Richard Biener
@ 2022-10-07 10:51 ` Franz Sirl
  2022-10-07 11:33   ` Richard Biener
  2022-10-07 14:13   ` Jeff Law
  1 sibling, 2 replies; 6+ messages in thread
From: Franz Sirl @ 2022-10-07 10:51 UTC (permalink / raw)
  To: Jeff Law, gcc-patches

Am 2022-09-25 um 18:28 schrieb Jeff Law:
> This is a minor improvement for the core_list_find routine in coremark.
> 
> 
> Basically for riscv, and likely other targets, we can end up with an 
> unconditional jump to a return statement.    This is a result of 
> compensation code created by bb-reorder, and no jump optimization pass 
> runs after bb-reorder to clean this stuff up.
> 
> This patch utilizes preexisting code to identify suitable branch targets 
> as well as preexisting code to emit a suitable return, so it's pretty 
> simple.  Note that when we arrange to do this optimization, the original 
> return block may become unreachable. So we conditionally call 
> delete_unreachable_blocks to fix that up.
> 
> This triggers ~160 times during an x86_64 bootstrap.  Naturally it 
> bootstraps and regression tests on x86_64.
> 
> I've also bootstrapped this on riscv64, regression testing with qemu 
> shows some regressions, but AFAICT they're actually qemu bugs with 
> signal handling/delivery -- qemu user mode emulation is not consistently 
> calling user defined signal handlers.  Given the same binary, sometimes 
> they'll get called and the test passes, other times the handler isn't 
> called and the test (of course) fails. I'll probably spend some time to 
> try and chase this down for the sake of making testing easier.
> 
> 
> OK for the trunk?

Hello Jeff,

I've bisected this change to break a "profiledbootstrap" on x86_64 like 
that:

make[3]: Entering directory 
'/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/gcc'
/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/./prev-gcc/xg++ 
-B/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/./prev-gcc/ 
-B/usr/x86_64-suse-linux/
bin/ -nostdinc++ 
-B/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/prev-x86_64-suse-linux/libstdc++-v3/src/.libs 
-B/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64
-suse-linux/prev-x86_64-suse-linux/libstdc++-v3/libsupc++/.libs 
-I/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/prev-x86_64-suse-linux/libstdc++-v3/include/x86_64-suse-linu
x 
-I/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/prev-x86_64-suse-linux/libstdc++-v3/include 
 
-I/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/libstdc++-v3/libsupc++ 
-L
/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/prev-x86_64-suse-linux/libstdc++-v3/src/.libs 
-L/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/prev-x8
6_64-suse-linux/libstdc++-v3/libsupc++/.libs  -fno-PIE -c   -O2 -g 
-fmessage-length=0 -D_FORTIFY_SOURCE=2 -funwind-tables 
-fasynchronous-unwind-tables -U_FORTIFY_SOURCE -fprofile-use 
-fprofile-reprod
ucible=parallel-runs -DIN_GCC     -fno-exceptions -fno-rtti 
-fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings 
-Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic
-Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common 
-DHAVE_CONFIG_H -I. -I. -I../../gcc -I../../gcc/. -I../../gcc/../include 
-I../../gcc/../libcpp/include -I../../gcc/../libcody
  -I../../gcc/../libdecnumber -I../../gcc/../libdecnumber/bid 
-I../libdecnumber -I../../gcc/../libbacktrace   -o cgraph.o -MT cgraph.o 
-MMD -MP -MF ./.deps/cgraph.TPo ../../gcc/cgraph.cc
../../gcc/cgraph.cc: In member function 'cgraph_edge* 
cgraph_edge::first_speculative_call_target()':
../../gcc/cgraph.cc:1166:1: error: EDGE_CROSSING incorrectly set across 
same section
  1166 | }
       | ^
../../gcc/cgraph.cc:1166:1: error: No region crossing jump at section 
boundary in bb 19
during RTL pass: bbro
../../gcc/cgraph.cc:1166:1: internal compiler error: verify_flow_info failed
0xa7116e verify_flow_info()
         ../../gcc/cfghooks.cc:284
0x1c64958 execute
         ../../gcc/bb-reorder.cc:2663

In such a case, what do you need to reproduce it? I'm a mere user of the 
Suse RPM builds here, no idea if profiling needs any extra data to 
reproduce to bug.

Franz.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFA] Minor improvement to coremark, avoid unconditional jump to return
  2022-10-07 10:51 ` Franz Sirl
@ 2022-10-07 11:33   ` Richard Biener
  2022-10-07 14:13   ` Jeff Law
  1 sibling, 0 replies; 6+ messages in thread
From: Richard Biener @ 2022-10-07 11:33 UTC (permalink / raw)
  To: Franz Sirl; +Cc: Jeff Law, gcc-patches

On Fri, Oct 7, 2022 at 12:51 PM Franz Sirl
<Franz.Sirl-kernel@lauterbach.com> wrote:
>
> Am 2022-09-25 um 18:28 schrieb Jeff Law:
> > This is a minor improvement for the core_list_find routine in coremark.
> >
> >
> > Basically for riscv, and likely other targets, we can end up with an
> > unconditional jump to a return statement.    This is a result of
> > compensation code created by bb-reorder, and no jump optimization pass
> > runs after bb-reorder to clean this stuff up.
> >
> > This patch utilizes preexisting code to identify suitable branch targets
> > as well as preexisting code to emit a suitable return, so it's pretty
> > simple.  Note that when we arrange to do this optimization, the original
> > return block may become unreachable. So we conditionally call
> > delete_unreachable_blocks to fix that up.
> >
> > This triggers ~160 times during an x86_64 bootstrap.  Naturally it
> > bootstraps and regression tests on x86_64.
> >
> > I've also bootstrapped this on riscv64, regression testing with qemu
> > shows some regressions, but AFAICT they're actually qemu bugs with
> > signal handling/delivery -- qemu user mode emulation is not consistently
> > calling user defined signal handlers.  Given the same binary, sometimes
> > they'll get called and the test passes, other times the handler isn't
> > called and the test (of course) fails. I'll probably spend some time to
> > try and chase this down for the sake of making testing easier.
> >
> >
> > OK for the trunk?
>
> Hello Jeff,
>
> I've bisected this change to break a "profiledbootstrap" on x86_64 like
> that:

Can you open a bugreport please?

> make[3]: Entering directory
> '/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/gcc'
> /home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/./prev-gcc/xg++
> -B/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/./prev-gcc/
> -B/usr/x86_64-suse-linux/
> bin/ -nostdinc++
> -B/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/prev-x86_64-suse-linux/libstdc++-v3/src/.libs
> -B/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64
> -suse-linux/prev-x86_64-suse-linux/libstdc++-v3/libsupc++/.libs
> -I/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/prev-x86_64-suse-linux/libstdc++-v3/include/x86_64-suse-linu
> x
> -I/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/prev-x86_64-suse-linux/libstdc++-v3/include
>
> -I/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/libstdc++-v3/libsupc++
> -L
> /home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/prev-x86_64-suse-linux/libstdc++-v3/src/.libs
> -L/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/prev-x8
> 6_64-suse-linux/libstdc++-v3/libsupc++/.libs  -fno-PIE -c   -O2 -g
> -fmessage-length=0 -D_FORTIFY_SOURCE=2 -funwind-tables
> -fasynchronous-unwind-tables -U_FORTIFY_SOURCE -fprofile-use
> -fprofile-reprod
> ucible=parallel-runs -DIN_GCC     -fno-exceptions -fno-rtti
> -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings
> -Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic
> -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common
> -DHAVE_CONFIG_H -I. -I. -I../../gcc -I../../gcc/. -I../../gcc/../include
> -I../../gcc/../libcpp/include -I../../gcc/../libcody
>   -I../../gcc/../libdecnumber -I../../gcc/../libdecnumber/bid
> -I../libdecnumber -I../../gcc/../libbacktrace   -o cgraph.o -MT cgraph.o
> -MMD -MP -MF ./.deps/cgraph.TPo ../../gcc/cgraph.cc
> ../../gcc/cgraph.cc: In member function 'cgraph_edge*
> cgraph_edge::first_speculative_call_target()':
> ../../gcc/cgraph.cc:1166:1: error: EDGE_CROSSING incorrectly set across
> same section
>   1166 | }
>        | ^
> ../../gcc/cgraph.cc:1166:1: error: No region crossing jump at section
> boundary in bb 19
> during RTL pass: bbro
> ../../gcc/cgraph.cc:1166:1: internal compiler error: verify_flow_info failed
> 0xa7116e verify_flow_info()
>          ../../gcc/cfghooks.cc:284
> 0x1c64958 execute
>          ../../gcc/bb-reorder.cc:2663
>
> In such a case, what do you need to reproduce it? I'm a mere user of the
> Suse RPM builds here, no idea if profiling needs any extra data to
> reproduce to bug.
>
> Franz.
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFA] Minor improvement to coremark, avoid unconditional jump to return
  2022-10-07 10:51 ` Franz Sirl
  2022-10-07 11:33   ` Richard Biener
@ 2022-10-07 14:13   ` Jeff Law
  2022-10-07 15:11     ` Franz Sirl
  1 sibling, 1 reply; 6+ messages in thread
From: Jeff Law @ 2022-10-07 14:13 UTC (permalink / raw)
  To: Franz Sirl, gcc-patches


On 10/7/22 04:51, Franz Sirl wrote:
> Am 2022-09-25 um 18:28 schrieb Jeff Law:
>> This is a minor improvement for the core_list_find routine in coremark.
>>
>>
>> Basically for riscv, and likely other targets, we can end up with an 
>> unconditional jump to a return statement.    This is a result of 
>> compensation code created by bb-reorder, and no jump optimization 
>> pass runs after bb-reorder to clean this stuff up.
>>
>> This patch utilizes preexisting code to identify suitable branch 
>> targets as well as preexisting code to emit a suitable return, so 
>> it's pretty simple.  Note that when we arrange to do this 
>> optimization, the original return block may become unreachable. So we 
>> conditionally call delete_unreachable_blocks to fix that up.
>>
>> This triggers ~160 times during an x86_64 bootstrap.  Naturally it 
>> bootstraps and regression tests on x86_64.
>>
>> I've also bootstrapped this on riscv64, regression testing with qemu 
>> shows some regressions, but AFAICT they're actually qemu bugs with 
>> signal handling/delivery -- qemu user mode emulation is not 
>> consistently calling user defined signal handlers.  Given the same 
>> binary, sometimes they'll get called and the test passes, other times 
>> the handler isn't called and the test (of course) fails. I'll 
>> probably spend some time to try and chase this down for the sake of 
>> making testing easier.
>>
>>
>> OK for the trunk?
>
> Hello Jeff,
>
> I've bisected this change to break a "profiledbootstrap" on x86_64 
> like that:
>
> make[3]: Entering directory 
> '/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/gcc'
> /home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/./prev-gcc/xg++ 
> -B/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/./prev-gcc/ 
> -B/usr/x86_64-suse-linux/
> bin/ -nostdinc++ 
> -B/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/prev-x86_64-suse-linux/libstdc++-v3/src/.libs 
> -B/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64
> -suse-linux/prev-x86_64-suse-linux/libstdc++-v3/libsupc++/.libs 
> -I/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/prev-x86_64-suse-linux/libstdc++-v3/include/x86_64-suse-linu
> x 
> -I/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/prev-x86_64-suse-linux/libstdc++-v3/include 
>
> -I/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/libstdc++-v3/libsupc++ 
> -L
> /home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/prev-x86_64-suse-linux/libstdc++-v3/src/.libs 
> -L/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/prev-x8 
>
> 6_64-suse-linux/libstdc++-v3/libsupc++/.libs  -fno-PIE -c   -O2 -g 
> -fmessage-length=0 -D_FORTIFY_SOURCE=2 -funwind-tables 
> -fasynchronous-unwind-tables -U_FORTIFY_SOURCE -fprofile-use 
> -fprofile-reprod
> ucible=parallel-runs -DIN_GCC     -fno-exceptions -fno-rtti 
> -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings 
> -Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic
> -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings 
> -fno-common -DHAVE_CONFIG_H -I. -I. -I../../gcc -I../../gcc/. 
> -I../../gcc/../include -I../../gcc/../libcpp/include 
> -I../../gcc/../libcody
>  -I../../gcc/../libdecnumber -I../../gcc/../libdecnumber/bid 
> -I../libdecnumber -I../../gcc/../libbacktrace   -o cgraph.o -MT 
> cgraph.o -MMD -MP -MF ./.deps/cgraph.TPo ../../gcc/cgraph.cc
> ../../gcc/cgraph.cc: In member function 'cgraph_edge* 
> cgraph_edge::first_speculative_call_target()':
> ../../gcc/cgraph.cc:1166:1: error: EDGE_CROSSING incorrectly set 
> across same section
>  1166 | }
>       | ^
> ../../gcc/cgraph.cc:1166:1: error: No region crossing jump at section 
> boundary in bb 19
> during RTL pass: bbro
> ../../gcc/cgraph.cc:1166:1: internal compiler error: verify_flow_info 
> failed
> 0xa7116e verify_flow_info()
>         ../../gcc/cfghooks.cc:284
> 0x1c64958 execute
>         ../../gcc/bb-reorder.cc:2663
>
> In such a case, what do you need to reproduce it? I'm a mere user of 
> the Suse RPM builds here, no idea if profiling needs any extra data to 
> reproduce to bug.

Franz, it's been a long time.  Good to hear from you.


I'll take a look.  I'll start with a profiledbootstrap and if that 
doesn't reproduce I'll get in contact


jeff


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFA] Minor improvement to coremark, avoid unconditional jump to return
  2022-10-07 14:13   ` Jeff Law
@ 2022-10-07 15:11     ` Franz Sirl
  0 siblings, 0 replies; 6+ messages in thread
From: Franz Sirl @ 2022-10-07 15:11 UTC (permalink / raw)
  To: Jeff Law, gcc-patches

Am 2022-10-07 um 16:13 schrieb Jeff Law:
> 
> On 10/7/22 04:51, Franz Sirl wrote:
>> Am 2022-09-25 um 18:28 schrieb Jeff Law:
>>> This is a minor improvement for the core_list_find routine in coremark.
>>>
>>>
>>> Basically for riscv, and likely other targets, we can end up with an 
>>> unconditional jump to a return statement.    This is a result of 
>>> compensation code created by bb-reorder, and no jump optimization 
>>> pass runs after bb-reorder to clean this stuff up.
>>>
>>> This patch utilizes preexisting code to identify suitable branch 
>>> targets as well as preexisting code to emit a suitable return, so 
>>> it's pretty simple.  Note that when we arrange to do this 
>>> optimization, the original return block may become unreachable. So we 
>>> conditionally call delete_unreachable_blocks to fix that up.
>>>
>>> This triggers ~160 times during an x86_64 bootstrap.  Naturally it 
>>> bootstraps and regression tests on x86_64.
>>>
>>> I've also bootstrapped this on riscv64, regression testing with qemu 
>>> shows some regressions, but AFAICT they're actually qemu bugs with 
>>> signal handling/delivery -- qemu user mode emulation is not 
>>> consistently calling user defined signal handlers.  Given the same 
>>> binary, sometimes they'll get called and the test passes, other times 
>>> the handler isn't called and the test (of course) fails. I'll 
>>> probably spend some time to try and chase this down for the sake of 
>>> making testing easier.
>>>
>>>
>>> OK for the trunk?
>>
>> Hello Jeff,
>>
>> I've bisected this change to break a "profiledbootstrap" on x86_64 
>> like that:
>>
>> make[3]: Entering directory 
>> '/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/gcc'
>> /home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/./prev-gcc/xg++ -B/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/./prev-gcc/ -B/usr/x86_64-suse-linux/
>> bin/ -nostdinc++ 
>> -B/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/prev-x86_64-suse-linux/libstdc++-v3/src/.libs -B/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64
>> -suse-linux/prev-x86_64-suse-linux/libstdc++-v3/libsupc++/.libs 
>> -I/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/prev-x86_64-suse-linux/libstdc++-v3/include/x86_64-suse-linu
>> x 
>> -I/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/prev-x86_64-suse-linux/libstdc++-v3/include
>> -I/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/libstdc++-v3/libsupc++ -L
>> /home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/prev-x86_64-suse-linux/libstdc++-v3/src/.libs -L/home/fsirl/rpmbuild/BUILD/gcc-13.0.0+gitr13+2871/obj-x86_64-suse-linux/prev-x8
>> 6_64-suse-linux/libstdc++-v3/libsupc++/.libs  -fno-PIE -c   -O2 -g 
>> -fmessage-length=0 -D_FORTIFY_SOURCE=2 -funwind-tables 
>> -fasynchronous-unwind-tables -U_FORTIFY_SOURCE -fprofile-use 
>> -fprofile-reprod
>> ucible=parallel-runs -DIN_GCC     -fno-exceptions -fno-rtti 
>> -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings 
>> -Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic
>> -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings 
>> -fno-common -DHAVE_CONFIG_H -I. -I. -I../../gcc -I../../gcc/. 
>> -I../../gcc/../include -I../../gcc/../libcpp/include 
>> -I../../gcc/../libcody
>>  -I../../gcc/../libdecnumber -I../../gcc/../libdecnumber/bid 
>> -I../libdecnumber -I../../gcc/../libbacktrace   -o cgraph.o -MT 
>> cgraph.o -MMD -MP -MF ./.deps/cgraph.TPo ../../gcc/cgraph.cc
>> ../../gcc/cgraph.cc: In member function 'cgraph_edge* 
>> cgraph_edge::first_speculative_call_target()':
>> ../../gcc/cgraph.cc:1166:1: error: EDGE_CROSSING incorrectly set 
>> across same section
>>  1166 | }
>>       | ^
>> ../../gcc/cgraph.cc:1166:1: error: No region crossing jump at section 
>> boundary in bb 19
>> during RTL pass: bbro
>> ../../gcc/cgraph.cc:1166:1: internal compiler error: verify_flow_info 
>> failed
>> 0xa7116e verify_flow_info()
>>         ../../gcc/cfghooks.cc:284
>> 0x1c64958 execute
>>         ../../gcc/bb-reorder.cc:2663
>>
>> In such a case, what do you need to reproduce it? I'm a mere user of 
>> the Suse RPM builds here, no idea if profiling needs any extra data to 
>> reproduce to bug.
> 
> Franz, it's been a long time.  Good to hear from you.
> 
> 
> I'll take a look.  I'll start with a profiledbootstrap and if that 
> doesn't reproduce I'll get in contact

Yes, long time... Things happened :-) But I'm still following GCC and 
report bugs as time permits.

I've opened 107182 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107182 
with the -freport-bug file and cgraph.gcda. I hope this is enough to 
reproduce.

Franz.


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-10-07 15:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-25 16:28 [RFA] Minor improvement to coremark, avoid unconditional jump to return Jeff Law
2022-09-26  7:53 ` Richard Biener
2022-10-07 10:51 ` Franz Sirl
2022-10-07 11:33   ` Richard Biener
2022-10-07 14:13   ` Jeff Law
2022-10-07 15:11     ` Franz Sirl

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).