Re: [PATCH v4 4/4] ree: Improve ree pass for rs6000 target using defined ABI interfaces.

[Jeff Law via Gcc-patches]

On 4/28/23 16:42, Hans-Peter Nilsson wrote:
On Sat, 22 Apr 2023, Ajit Agarwal via Gcc-patches wrote:

Hello All:

This new version of patch 4 use improve ree pass for rs6000 target using 
defined ABI interfaces.
Bootstrapped and regtested on power64-linux-gnu.

Thanks & Regards
Ajit


        ree: Improve ree pass for rs6000 target using defined abi interfaces

         For rs6000 target we see redundant zero and sign
         extension and done to improve ree pass to eliminate
         such redundant zero and sign extension using defines
         ABI interfaces.

         2023-04-22  Ajit Kumar Agarwal  <aagar...@linux.ibm.com>

gcc/ChangeLog:

         * ree.cc (combline_reaching_defs): Add zero_extend
         using defined abi interfaces.
         (add_removable_extension): use of defined abi interfaces
         for no reaching defs.
         (abi_extension_candidate_return_reg_p): New defined ABI function.
         (abi_extension_candidate_p): New defined ABI function.
         (abi_extension_candidate_argno_p): New defined ABI function.
         (abi_handle_regs_without_defs_p): New defined ABI function.

gcc/testsuite/ChangeLog:

         * g++.target/powerpc/zext-elim-3.C
---
  gcc/ree.cc                                    | 176 +++++++++++++++---
  .../g++.target/powerpc/zext-elim-3.C          |  16 ++
  2 files changed, 162 insertions(+), 30 deletions(-)
  create mode 100644 gcc/testsuite/g++.target/powerpc/zext-elim-3.C

diff --git a/gcc/ree.cc b/gcc/ree.cc
index 413aec7c8eb..0de96b1ece1 100644
--- a/gcc/ree.cc
+++ b/gcc/ree.cc
@@ -473,7 +473,8 @@ get_defs (rtx_insn *insn, rtx reg, vec<rtx_insn *> *dest)
        break;
      }
  
-  gcc_assert (use != NULL);
+  if (use == NULL)
+    return NULL;
  
    ref_chain = DF_REF_CHAIN (use);
  
@@ -514,7 +515,8 @@ get_uses (rtx_insn *insn, rtx reg)
      if (REGNO (DF_REF_REG (def)) == REGNO (reg))
        break;
  
-  gcc_assert (def != NULL);
+  if (def == NULL)
+    return NULL;
  
    ref_chain = DF_REF_CHAIN (def);
  
@@ -750,6 +752,103 @@ get_extended_src_reg (rtx src)
    return src;
  }
  
+/* Return TRUE if the candidate insn is zero extend and regno is
+   an return  registers.  */
+
+static bool
+abi_extension_candidate_return_reg_p (rtx_insn *insn, int regno)
+{
+  rtx set = single_set (insn);
+
+  if (GET_CODE (SET_SRC (set)) !=  ZERO_EXTEND)
+    return false;
+
+  if (FUNCTION_VALUE_REGNO_P (regno))
+    return true;
+
+  return false;
+}
+
+/* Return TRUE if reg source operand of zero_extend is argument registers
+   and not return registers and source and destination operand are same
+   and mode of source and destination operand are not same.  */
+
+static bool
+abi_extension_candidate_p (rtx_insn *insn)
+{
+  rtx set = single_set (insn);
+
+  if (GET_CODE (SET_SRC (set)) !=  ZERO_EXTEND)
+    return false;
+
+  machine_mode ext_dst_mode = GET_MODE (SET_DEST (set));
+  rtx orig_src = XEXP (SET_SRC (set),0);
+
+  bool copy_needed
+    = (REGNO (SET_DEST (set)) != REGNO (XEXP (SET_SRC (set), 0)));
+
+  if (!copy_needed && ext_dst_mode != GET_MODE (orig_src)
+      && FUNCTION_ARG_REGNO_P (REGNO (orig_src))
+      && !abi_extension_candidate_return_reg_p (insn, REGNO (orig_src)))
+    return true;
+
+  return false;
+}
+
+/* Return TRUE if the candidate insn is zero extend and regno is
+   an argument registers.  */
+
+static bool
+abi_extension_candidate_argno_p (rtx_code code, int regno)
+{
+  if (code !=  ZERO_EXTEND)
+    return false;
+
+  if (FUNCTION_ARG_REGNO_P (regno))
+    return true;
+
+  return false;
+}

I don't see anything in those functions that checks if
ZERO_EXTEND is actually a feature of the ABI, e.g. as opposed to
no extension or SIGN_EXTEND.  Do I miss something?
I don't think you missed anything.  That was one of the points I was 
making last week.  Somewhere, somehow we need to describe what the ABI 
mandates and guarantees.

So while what Ajit has done is a step forward, at some point the actual 
details of the ABI need to be described in a way that can be checked and 
consumed by REE.

Jeff