[PR rtl-optimization/107455] Eliminate unnecessary constant load

This resurrects a patch from a bit over 2 years ago that I never wrapped up.
IIRC, I ended up up catching covid, then in the hospital for an unrelated issue
and it just got dropped on the floor in the insanity.

The basic idea here is to help postreload-cse eliminate more const/copies by
recording a small set of conditional equivalences (as Richi said in 2022,
"Ick").

It was originally to help eliminate an unnecessary constant load I saw in
coremark, but as seen in BZ107455 the same issues show up in real code as well.

Bootstrapped and regression tested on x86-64, also been through multiple spins
in my tester.

Changes since v2:

  - Simplified logic for blocks to examine
  - Remove redundant tests when filtering blocks to examine
  - Remove bogus check which only allowed reg->reg copies

Changes since v1:

Richard B and Richard S both had good comments last time around and their
requests are reflected in this update:

  - Use rtx_equal_p rather than pointer equality
  - Restrict to register "destinations"
  - Restrict to integer modes
  - Adjust entry block handling

My own wider scale testing resulted in a few more changes.

  - Robustify extracting the (set (pc) ... ), which then required ...
  - Handle if src/dst are clobbered by the conditional branch
  - Fix logic error causing too many equivalences to be recorded

	PR rtl-optimization/107455
gcc/
	* postreload.cc (reload_cse_regs_1): Take advantage of conditional
	equivalences.

gcc/testsuite
	* gcc.target/riscv/pr107455-1.c: New test.
	* gcc.target/riscv/pr107455-2.c: New test.

diff --git a/gcc/postreload.cc b/gcc/postreload.cc
index 05c2e0e2644528c6e4fae83aeebf7cc72e8a87fb..487aa8aad054eb6fe5e7f2e6d65439c1359b2323 100644 (file)
--- a/gcc/postreload.cc
+++ b/gcc/postreload.cc
@@ -33,6 +33,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "emit-rtl.h"
 #include "recog.h"
 
+#include "cfghooks.h"
 #include "cfgrtl.h"
 #include "cfgbuild.h"
 #include "cfgcleanup.h"
@@ -221,13 +222,107 @@ reload_cse_regs_1 (void)
   init_alias_analysis ();
 
   FOR_EACH_BB_FN (bb, cfun)
-    FOR_BB_INSNS (bb, insn)
-      {
-       if (INSN_P (insn))
-         cfg_changed |= reload_cse_simplify (insn, testreg);
+    {
+      /* If BB has a small number of predecessors, see if each of the
+        has the same implicit set.  If so, record that implicit set so
+        that we can add it to the cselib tables.  */
+      rtx_insn *implicit_set;
 
-       cselib_process_insn (insn);
-      }
+      implicit_set = NULL;
+      if (EDGE_COUNT (bb->preds) <= 3)
+       {
+         edge e;
+         edge_iterator ei;
+         rtx src = NULL_RTX;
+         rtx dest = NULL_RTX;
+
+         /* Iterate over each incoming edge and see if they
+            all have the same implicit set.  */
+         FOR_EACH_EDGE (e, ei, bb->preds)
+           {
+             /* Skip the entry/exit block.  */
+             if (e->src == ENTRY_BLOCK_PTR_FOR_FN (cfun))
+               break;
+
+             /* Verify this block ends with a suitable condjump  */
+             rtx_insn *condjump = BB_END (e->src);
+             if (!condjump || ! any_condjump_p (condjump))
+               break;
+
+             /* This predecessor ends with a possible equivalence
+                producing conditional branch.  Extract the condition
+                and try to use it to create an equivalence.  */
+             rtx pat = pc_set (condjump);
+             rtx i_t_e = SET_SRC (pat);
+             gcc_assert (GET_CODE (i_t_e) == IF_THEN_ELSE);
+             rtx cond = XEXP (i_t_e, 0);
+
+             if ((((e->flags & EDGE_FALLTHRU) != 0)
+                  == (XEXP (i_t_e, 1) == pc_rtx))
+                 ? GET_CODE (cond) == EQ
+                 : GET_CODE (cond) == NE)
+               {
+                 /* If this is the first time through record
+                    the source and destination.  */
+                 if (!dest)
+                   {
+                     dest = XEXP (cond, 0);
+                     src = XEXP (cond, 1);
+                   }
+                 /* If this is not the first time through, then
+                    verify the source and destination match.  */
+                 else if (rtx_equal_p (dest, XEXP (cond, 0))
+                          && rtx_equal_p (src, XEXP (cond, 1)))
+                   ;
+                 else
+                   break;
+
+                 /* A few more checks.  First make sure we're dealing with
+                    integer modes, second make sure the values aren't clobbered
+                    by the conditional branch itself.  Do this for every
+                    conditional jump participating in creation of the
+                    equivalence.  */
+                 if (!REG_P (dest)
+                     || !(REG_P (src) || CONST_INT_P (src))
+                     || GET_MODE_CLASS (GET_MODE (dest)) != MODE_INT
+                     || reg_set_p (dest, condjump)
+                     || reg_set_p (src, condjump))
+                   break;
+
+               }
+             else
+               break;
+           }
+
+         /* If all the incoming edges had the same implicit
+            set, then create a dummy insn for that set.
+
+            It will be entered into the cselib tables before
+            we process the first real insn in this block.  */
+         if (dest && ei_end_p (ei))
+           implicit_set = make_insn_raw (gen_rtx_SET (dest, src));
+       }
+
+      FOR_BB_INSNS (bb, insn)
+       {
+         if (INSN_P (insn))
+           {
+             /* If we recorded an implicit set, enter it
+                into the tables before the first real insn.
+
+                We have to do it this way because a CODE_LABEL
+                will flush the cselib tables.  */
+             if (implicit_set)
+               {
+                 cselib_process_insn (implicit_set);
+                 implicit_set = NULL;
+               }
+             cfg_changed |= reload_cse_simplify (insn, testreg);
+           }
+
+         cselib_process_insn (insn);
+       }
+    }
 
   /* Clean up.  */
   end_alias_analysis ();

diff --git a/gcc/testsuite/gcc.target/riscv/pr107455-1.c b/gcc/testsuite/gcc.target/riscv/pr107455-1.c
new file mode 100644 (file)
index 0000000..59616b8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/pr107455-1.c
@@ -0,0 +1,46 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-dp" } */
+/* { dg-skip-if "" { *-*-* } { "-O0" "-O1" "-O2" "-O3" "-Og" } } */
+
+
+typedef struct dllist
+{
+  int i;
+  struct dllist *ptr_to_next;
+  struct dllist *ptr_to_previous;
+} dllist;
+
+int sglib_dllist_len(dllist *list) {
+    int res;
+    dllist *_dl_;
+    int _r1_, _r2_;
+    if (list== ((void *)0)) {
+        res = 0;
+    } else { 
+        dllist *_ce_;
+        dllist *_ne_;
+        _r1_ = 0;
+        _ce_ = list;
+        while (_ce_!= ((void *)0)) {
+            _ne_ = _ce_->ptr_to_previous;
+            _r1_++;
+            _ce_ = _ne_;
+        }
+        _dl_ = list->ptr_to_next;
+        _r2_ = 0;
+        _ce_ = _dl_;
+        while (_ce_!= (void *)0) {
+            _ne_ = _ce_->ptr_to_next;
+            _r2_++;
+            _ce_ = _ne_;
+        }
+        res = _r1_ + _r2_;
+    }
+    return res;
+}
+
+
+/* There was an unnecessary assignment to the return value until
+   recently.  Scan for that in the resulting output.  */
+/* { dg-final { scan-assembler-times "li\\ta0,0" 1 } } */
+

diff --git a/gcc/testsuite/gcc.target/riscv/pr107455-2.c b/gcc/testsuite/gcc.target/riscv/pr107455-2.c
new file mode 100644 (file)
index 0000000..91106bb
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/pr107455-2.c
@@ -0,0 +1,40 @@
+/* { 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 was an unnecessary assignment to the return value until
+   recently.  Scan for that in the resulting output.  */
+/* { dg-final { scan-assembler-not "li\\ta0,0" } } */
+