public inbox for gcc-regression@sourceware.org
help / color / mirror / Atom feed
* [TCWG CI] Failure after basepoints/gcc-13-2871-g1b74b5cb4e9: [RFA] Minor improvement to coremark, avoid unconditional jump to return
@ 2022-09-27  1:09 ci_notify
  0 siblings, 0 replies; 3+ messages in thread
From: ci_notify @ 2022-09-27  1:09 UTC (permalink / raw)
  To: Jeff Law; +Cc: gcc-regression

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

Failure after basepoints/gcc-13-2871-g1b74b5cb4e9: [RFA] Minor improvement to coremark, avoid unconditional jump to return:

Results changed to
-10
# true:
0
# build_abe binutils:
1
# build_abe stage1:
# FAILED
# First few build errors in logs:
# 00:09:33 cc1: error: no include path in which to search for stdc-predef.h
# 00:10:01 /home/tcwg-buildslave/workspace/tcwg_gnu_1/abe/snapshots/gcc.git~master/libgcc/fixed-bit.c:246:1: internal compiler error: in target_gen_simple_return, at /home/tcwg-buildslave/workspace/tcwg_gnu_1/abe/snapshots/gcc.git~master/gcc/config/arm/arm.md:8998
# 00:10:01 /home/tcwg-buildslave/workspace/tcwg_gnu_1/abe/snapshots/gcc.git~master/libgcc/fixed-bit.c:246:1: internal compiler error: in target_gen_simple_return, at /home/tcwg-buildslave/workspace/tcwg_gnu_1/abe/snapshots/gcc.git~master/gcc/config/arm/arm.md:8998
# 00:10:01 make[2]: *** [/home/tcwg-buildslave/workspace/tcwg_gnu_1/abe/snapshots/gcc.git~master/libgcc/fixed-obj.mk:27: _saturate2DA.o] Error 1
# 00:10:01 make[2]: *** [/home/tcwg-buildslave/workspace/tcwg_gnu_1/abe/snapshots/gcc.git~master/libgcc/fixed-obj.mk:27: _saturate2DQ.o] Error 1
# 00:10:02 make[1]: *** [Makefile:12767: all-target-libgcc] Error 2
# 00:10:03 make: *** [Makefile:1004: all] Error 2

from
-10
# true:
0
# build_abe binutils:
1
# build_abe stage1:
2
# build_abe linux:
3
# build_abe glibc:
4
# build_abe stage2:
5
# build_abe gdb:
6
# build_abe qemu:
7

THIS IS THE END OF INTERESTING STUFF.  BELOW ARE LINKS TO BUILDS, REPRODUCTION INSTRUCTIONS, AND THE RAW COMMIT.

For latest status see comments in https://linaro.atlassian.net/browse/GNU-692 .
Status of basepoints/gcc-13-2871-g1b74b5cb4e9 commit for tcwg_gnu_cross_build:
commit 1b74b5cb4e9d7191f298245063a8f9c3a1bbeff4
Author: Jeff Law <jeffreyalaw@gmail.com>
Date:   Sun Sep 25 12:23:59 2022 -0400

    [RFA] Minor improvement to coremark, avoid unconditional jump to return
    
    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.
* master-arm
** Failure after basepoints/gcc-13-2871-g1b74b5cb4e9: [RFA] Minor improvement to coremark, avoid unconditional jump to return:
** https://ci.linaro.org/job/tcwg_gnu_cross_build-build-master-arm/1653/

Bad  build: https://ci.linaro.org/job/tcwg_gnu_cross_build-build-master-arm/1653/artifact/artifacts
Good build: https://ci.linaro.org/job/tcwg_gnu_cross_build-build-master-arm/1652/artifact/artifacts

Reproduce current build:
<cut>
mkdir -p investigate-gcc-1b74b5cb4e9d7191f298245063a8f9c3a1bbeff4
cd investigate-gcc-1b74b5cb4e9d7191f298245063a8f9c3a1bbeff4

# Fetch scripts
git clone https://git.linaro.org/toolchain/jenkins-scripts

# Fetch manifests for bad and good builds
mkdir -p bad/artifacts good/artifacts
curl -o bad/artifacts/manifest.sh https://ci.linaro.org/job/tcwg_gnu_cross_build-build-master-arm/1653/artifact/artifacts/manifest.sh --fail
curl -o good/artifacts/manifest.sh https://ci.linaro.org/job/tcwg_gnu_cross_build-build-master-arm/1652/artifact/artifacts/manifest.sh --fail

# Reproduce bad build
(cd bad; ../jenkins-scripts/tcwg_gnu-build.sh ^^ true %%rr[top_artifacts] artifacts)
# Reproduce good build
(cd good; ../jenkins-scripts/tcwg_gnu-build.sh ^^ true %%rr[top_artifacts] artifacts)
</cut>

Full commit (up to 1000 lines):
<cut>
commit 1b74b5cb4e9d7191f298245063a8f9c3a1bbeff4
Author: Jeff Law <jeffreyalaw@gmail.com>
Date:   Sun Sep 25 12:23:59 2022 -0400

    [RFA] Minor improvement to coremark, avoid unconditional jump to return
    
    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.
---
 gcc/cfgcleanup.cc                      |  2 +-
 gcc/cfgcleanup.h                       |  1 +
 gcc/cfgrtl.cc                          | 29 +++++++++++++++++++++++-
 gcc/testsuite/gcc.target/riscv/ret-1.c | 41 ++++++++++++++++++++++++++++++++++
 4 files changed, 71 insertions(+), 2 deletions(-)

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 } } */
+
</cut>

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

* [TCWG CI] Failure after basepoints/gcc-13-2871-g1b74b5cb4e9: [RFA] Minor improvement to coremark, avoid unconditional jump to return
@ 2022-09-26 20:53 ci_notify
  0 siblings, 0 replies; 3+ messages in thread
From: ci_notify @ 2022-09-26 20:53 UTC (permalink / raw)
  To: Jeff Law; +Cc: gcc-regression

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

Failure after basepoints/gcc-13-2871-g1b74b5cb4e9: [RFA] Minor improvement to coremark, avoid unconditional jump to return:

Results changed to
-10
# true:
0
# build_abe binutils:
1
# build_abe bootstrap:
# FAILED
# First few build errors in logs:
# 00:04:19 /home/tcwg-buildslave/workspace/tcwg_gnu_0/abe/snapshots/gcc.git~master/libgomp/env.c:1964:1: internal compiler error: in target_gen_simple_return, at /home/tcwg-buildslave/workspace/tcwg_gnu_0/abe/snapshots/gcc.git~master/gcc/config/arm/arm.md:8998
# 00:04:19 make[5]: *** [Makefile:801: env.lo] Error 1
# 00:04:41 make[4]: *** [Makefile:1024: all-recursive] Error 1
# 00:04:41 make[3]: *** [Makefile:624: all] Error 2
# 00:04:41 make[2]: *** [Makefile:23499: all-stage1-target-libgomp] Error 2
# 00:05:29 make[3]: [Makefile:1788: armv8l-unknown-linux-gnueabihf/bits/largefile-config.h] Error 1 (ignored)
# 00:05:29 make[1]: *** [Makefile:25433: stage1-bubble] Error 2
# 00:05:29 make: *** [Makefile:1064: all] Error 2

from
-10
# true:
0
# build_abe binutils:
1
# build_abe bootstrap:
2

THIS IS THE END OF INTERESTING STUFF.  BELOW ARE LINKS TO BUILDS, REPRODUCTION INSTRUCTIONS, AND THE RAW COMMIT.

For latest status see comments in https://linaro.atlassian.net/browse/GNU-692 .
Status of basepoints/gcc-13-2871-g1b74b5cb4e9 commit for tcwg_gcc_bootstrap:
commit 1b74b5cb4e9d7191f298245063a8f9c3a1bbeff4
Author: Jeff Law <jeffreyalaw@gmail.com>
Date:   Sun Sep 25 12:23:59 2022 -0400

    [RFA] Minor improvement to coremark, avoid unconditional jump to return
    
    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.
* master-arm-bootstrap
** Failure after basepoints/gcc-13-2871-g1b74b5cb4e9: [RFA] Minor improvement to coremark, avoid unconditional jump to return:
** https://ci.linaro.org/job/tcwg_gcc_bootstrap-build-master-arm-bootstrap/1541/

Bad  build: https://ci.linaro.org/job/tcwg_gcc_bootstrap-build-master-arm-bootstrap/1541/artifact/artifacts
Good build: https://ci.linaro.org/job/tcwg_gcc_bootstrap-build-master-arm-bootstrap/1540/artifact/artifacts

Reproduce current build:
<cut>
mkdir -p investigate-gcc-1b74b5cb4e9d7191f298245063a8f9c3a1bbeff4
cd investigate-gcc-1b74b5cb4e9d7191f298245063a8f9c3a1bbeff4

# Fetch scripts
git clone https://git.linaro.org/toolchain/jenkins-scripts

# Fetch manifests for bad and good builds
mkdir -p bad/artifacts good/artifacts
curl -o bad/artifacts/manifest.sh https://ci.linaro.org/job/tcwg_gcc_bootstrap-build-master-arm-bootstrap/1541/artifact/artifacts/manifest.sh --fail
curl -o good/artifacts/manifest.sh https://ci.linaro.org/job/tcwg_gcc_bootstrap-build-master-arm-bootstrap/1540/artifact/artifacts/manifest.sh --fail

# Reproduce bad build
(cd bad; ../jenkins-scripts/tcwg_gnu-build.sh ^^ true %%rr[top_artifacts] artifacts)
# Reproduce good build
(cd good; ../jenkins-scripts/tcwg_gnu-build.sh ^^ true %%rr[top_artifacts] artifacts)
</cut>

Full commit (up to 1000 lines):
<cut>
commit 1b74b5cb4e9d7191f298245063a8f9c3a1bbeff4
Author: Jeff Law <jeffreyalaw@gmail.com>
Date:   Sun Sep 25 12:23:59 2022 -0400

    [RFA] Minor improvement to coremark, avoid unconditional jump to return
    
    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.
---
 gcc/cfgcleanup.cc                      |  2 +-
 gcc/cfgcleanup.h                       |  1 +
 gcc/cfgrtl.cc                          | 29 +++++++++++++++++++++++-
 gcc/testsuite/gcc.target/riscv/ret-1.c | 41 ++++++++++++++++++++++++++++++++++
 4 files changed, 71 insertions(+), 2 deletions(-)

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 } } */
+
</cut>

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

* [TCWG CI] Failure after basepoints/gcc-13-2871-g1b74b5cb4e9: [RFA] Minor improvement to coremark, avoid unconditional jump to return
@ 2022-09-26 20:33 ci_notify
  0 siblings, 0 replies; 3+ messages in thread
From: ci_notify @ 2022-09-26 20:33 UTC (permalink / raw)
  To: Jeff Law; +Cc: gcc-regression

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

Failure after basepoints/gcc-13-2871-g1b74b5cb4e9: [RFA] Minor improvement to coremark, avoid unconditional jump to return:

Results changed to
-10
# true:
0
# build_abe binutils:
1
# build_abe gcc:
# FAILED
# First few build errors in logs:
# 00:02:25 /home/tcwg-buildslave/workspace/tcwg_gnu_11/abe/snapshots/gcc.git~master/libbacktrace/dwarf.c:1605:1: internal compiler error: in target_gen_simple_return, at /home/tcwg-buildslave/workspace/tcwg_gnu_11/abe/snapshots/gcc.git~master/gcc/config/arm/arm.md:8998
# 00:02:25 make[3]: *** [Makefile:1406: dwarf.lo] Error 1
# 00:02:30 make[2]: *** [Makefile:1161: all] Error 2
# 00:02:30 make[1]: *** [Makefile:14755: all-target-libbacktrace] Error 2
# 00:02:31 /home/tcwg-buildslave/workspace/tcwg_gnu_11/abe/snapshots/gcc.git~master/libgomp/env.c:1964:1: internal compiler error: in target_gen_simple_return, at /home/tcwg-buildslave/workspace/tcwg_gnu_11/abe/snapshots/gcc.git~master/gcc/config/arm/arm.md:8998
# 00:02:31 make[4]: *** [Makefile:801: env.lo] Error 1
# 00:02:37 make[3]: *** [Makefile:1024: all-recursive] Error 1
# 00:02:37 make[2]: *** [Makefile:624: all] Error 2
# 00:02:37 make[1]: *** [Makefile:17041: all-target-libgomp] Error 2
# 00:03:14 make[2]: [Makefile:1788: armv8l-unknown-linux-gnueabihf/bits/largefile-config.h] Error 1 (ignored)

from
-10
# true:
0
# build_abe binutils:
1
# build_abe gcc:
2
# build_abe linux:
4
# build_abe glibc:
5
# build_abe gdb:
6

THIS IS THE END OF INTERESTING STUFF.  BELOW ARE LINKS TO BUILDS, REPRODUCTION INSTRUCTIONS, AND THE RAW COMMIT.

For latest status see comments in https://linaro.atlassian.net/browse/GNU-692 .
Status of basepoints/gcc-13-2871-g1b74b5cb4e9 commit for tcwg_gnu_native_build:
commit 1b74b5cb4e9d7191f298245063a8f9c3a1bbeff4
Author: Jeff Law <jeffreyalaw@gmail.com>
Date:   Sun Sep 25 12:23:59 2022 -0400

    [RFA] Minor improvement to coremark, avoid unconditional jump to return
    
    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.
* master-arm
** Failure after basepoints/gcc-13-2871-g1b74b5cb4e9: [RFA] Minor improvement to coremark, avoid unconditional jump to return:
** https://ci.linaro.org/job/tcwg_gnu_native_build-build-master-arm/462/

Bad  build: https://ci.linaro.org/job/tcwg_gnu_native_build-build-master-arm/462/artifact/artifacts
Good build: https://ci.linaro.org/job/tcwg_gnu_native_build-build-master-arm/461/artifact/artifacts

Reproduce current build:
<cut>
mkdir -p investigate-gcc-1b74b5cb4e9d7191f298245063a8f9c3a1bbeff4
cd investigate-gcc-1b74b5cb4e9d7191f298245063a8f9c3a1bbeff4

# Fetch scripts
git clone https://git.linaro.org/toolchain/jenkins-scripts

# Fetch manifests for bad and good builds
mkdir -p bad/artifacts good/artifacts
curl -o bad/artifacts/manifest.sh https://ci.linaro.org/job/tcwg_gnu_native_build-build-master-arm/462/artifact/artifacts/manifest.sh --fail
curl -o good/artifacts/manifest.sh https://ci.linaro.org/job/tcwg_gnu_native_build-build-master-arm/461/artifact/artifacts/manifest.sh --fail

# Reproduce bad build
(cd bad; ../jenkins-scripts/tcwg_gnu-build.sh ^^ true %%rr[top_artifacts] artifacts)
# Reproduce good build
(cd good; ../jenkins-scripts/tcwg_gnu-build.sh ^^ true %%rr[top_artifacts] artifacts)
</cut>

Full commit (up to 1000 lines):
<cut>
commit 1b74b5cb4e9d7191f298245063a8f9c3a1bbeff4
Author: Jeff Law <jeffreyalaw@gmail.com>
Date:   Sun Sep 25 12:23:59 2022 -0400

    [RFA] Minor improvement to coremark, avoid unconditional jump to return
    
    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.
---
 gcc/cfgcleanup.cc                      |  2 +-
 gcc/cfgcleanup.h                       |  1 +
 gcc/cfgrtl.cc                          | 29 +++++++++++++++++++++++-
 gcc/testsuite/gcc.target/riscv/ret-1.c | 41 ++++++++++++++++++++++++++++++++++
 4 files changed, 71 insertions(+), 2 deletions(-)

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 } } */
+
</cut>

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

end of thread, other threads:[~2022-09-27  1:09 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-27  1:09 [TCWG CI] Failure after basepoints/gcc-13-2871-g1b74b5cb4e9: [RFA] Minor improvement to coremark, avoid unconditional jump to return ci_notify
  -- strict thread matches above, loose matches on Subject: below --
2022-09-26 20:53 ci_notify
2022-09-26 20:33 ci_notify

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