public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* PATCH v5 4/4] ree: Improve ree pass for rs6000 target using defined ABI interfaces.
@ 2023-06-01  5:23 Ajit Agarwal
  2023-06-01  6:04 ` Bernhard Reutner-Fischer
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Ajit Agarwal @ 2023-06-01  5:23 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jeff Law, Segher Boessenkool, Peter Bergner, Richard Biener

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.

Review comments incorporated.

Thanks & Regards
Ajit

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 defined
ABI interfaces.

2023-06-01  Ajit Kumar Agarwal  <aagarwa1@linux.ibm.com>

gcc/ChangeLog:

	* ree.cc (combine_reaching_defs): Use of  zero_extend and sign_extend
	defined abi interfaces.
	(add_removable_extension): Use of defined abi interfaces for no
	reaching defs.
	(abi_extension_candidate_return_reg_p): New function.
	(abi_extension_candidate_p): New function.
	(abi_extension_candidate_argno_p): New function.
	(abi_handle_regs_without_defs_p): New function.
	(abi_target_promote_function_mode): New function.

gcc/testsuite/ChangeLog:

        * g++.target/powerpc/zext-elim-3.C
---
 gcc/ree.cc                                    | 199 +++++++++++++++---
 .../g++.target/powerpc/zext-elim-3.C          |  13 ++
 2 files changed, 183 insertions(+), 29 deletions(-)
 create mode 100644 gcc/testsuite/g++.target/powerpc/zext-elim-3.C

diff --git a/gcc/ree.cc b/gcc/ree.cc
index fc04249fa84..2025a7c43da 100644
--- a/gcc/ree.cc
+++ b/gcc/ree.cc
@@ -514,7 +514,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 +751,120 @@ get_extended_src_reg (rtx src)
   return src;
 }
 
+/* Return TRUE if target mode is equal to source mode of zero_extend
+   or sign_extend otherwise false.  */
+
+static bool
+abi_target_promote_function_mode (machine_mode mode)
+{
+  int unsignedp;
+  machine_mode tgt_mode =
+    targetm.calls.promote_function_mode (NULL_TREE, mode, &unsignedp,
+					 NULL_TREE, 1);
+
+  if (tgt_mode == mode)
+    return true;
+  else
+    return false;
+}
+
+/* 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;
+}
+
+/* Return TRUE if the candidate insn doesn't have defs and have
+ * uses without RTX_BIN_ARITH/RTX_COMM_ARITH/RTX_UNARY rtx class.  */
+
+static bool
+abi_handle_regs_without_defs_p (rtx_insn *insn)
+{
+  if (side_effects_p (PATTERN (insn)))
+    return false;
+
+  struct df_link *uses
+    = get_uses (insn, SET_DEST (PATTERN (insn)));
+
+  if (!uses)
+    return false;
+
+  for (df_link *use = uses; use; use = use->next)
+    {
+      if (!use->ref)
+	return false;
+
+      if (BLOCK_FOR_INSN (insn)
+	  != BLOCK_FOR_INSN (DF_REF_INSN (use->ref)))
+	return false;
+
+      rtx_insn *use_insn = DF_REF_INSN (use->ref);
+
+      if (GET_CODE (PATTERN (use_insn)) == SET)
+	{
+	  rtx_code code = GET_CODE (SET_SRC (PATTERN (use_insn)));
+
+	  if (GET_RTX_CLASS (code) == RTX_BIN_ARITH
+	      || GET_RTX_CLASS (code) == RTX_COMM_ARITH
+	      || GET_RTX_CLASS (code) == RTX_UNARY)
+	    return false;
+	}
+     }
+  return true;
+}
+
 /* This function goes through all reaching defs of the source
    of the candidate for elimination (CAND) and tries to combine
    the extension with the definition instruction.  The changes
@@ -770,6 +885,11 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
 
   state->defs_list.truncate (0);
   state->copies_list.truncate (0);
+  rtx orig_src = XEXP (SET_SRC (cand->expr),0);
+
+  if (abi_extension_candidate_p (cand->insn)
+      && (!get_defs (cand->insn, orig_src, NULL)))
+    return abi_handle_regs_without_defs_p (cand->insn);
 
   outcome = make_defs_and_copies_lists (cand->insn, set_pat, state);
 
@@ -1036,6 +1156,15 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
         }
     }
 
+  rtx insn_set = single_set (cand->insn);
+
+  machine_mode mode = (GET_MODE (XEXP (SET_SRC (insn_set), 0)));
+
+  bool promote_p = abi_target_promote_function_mode (mode);
+
+  if (promote_p)
+    return true;
+
   if (merge_successful)
     {
       /* Commit the changes here if possible
@@ -1112,26 +1241,34 @@ add_removable_extension (const_rtx expr, rtx_insn *insn,
       rtx reg = XEXP (src, 0);
       struct df_link *defs, *def;
       ext_cand *cand;
+      defs = get_defs (insn, reg, NULL);
 
       /* Zero-extension of an undefined value is partly defined (it's
 	 completely undefined for sign-extension, though).  So if there exists
 	 a path from the entry to this zero-extension that leaves this register
 	 uninitialized, removing the extension could change the behavior of
 	 correct programs.  So first, check it is not the case.  */
-      if (code == ZERO_EXTEND && !bitmap_bit_p (init_regs, REGNO (reg)))
+      if (!defs && abi_extension_candidate_argno_p (code, REGNO (reg)))
 	{
-	  if (dump_file)
-	    {
-	      fprintf (dump_file, "Cannot eliminate extension:\n");
-	      print_rtl_single (dump_file, insn);
-	      fprintf (dump_file, " because it can operate on uninitialized"
-			          " data\n");
-	    }
+	  ext_cand e = {expr, code, mode, insn};
+	  insn_list->safe_push (e);
 	  return;
 	}
 
+       if ((code == ZERO_EXTEND
+	    && !bitmap_bit_p (init_regs, REGNO (reg))))
+	  {
+	    if (dump_file)
+	      {
+		fprintf (dump_file, "Cannot eliminate extension:\n");
+		print_rtl_single (dump_file, insn);
+		fprintf (dump_file, " because it can operate on uninitialized"
+				    " data\n");
+	      }
+	    return;
+	  }
+
       /* Second, make sure we can get all the reaching definitions.  */
-      defs = get_defs (insn, reg, NULL);
       if (!defs)
 	{
 	  if (dump_file)
@@ -1321,7 +1458,8 @@ find_and_remove_re (void)
 	      && (REGNO (SET_DEST (set)) != REGNO (XEXP (SET_SRC (set), 0))))
 	    {
               reinsn_copy_list.safe_push (curr_cand->insn);
-              reinsn_copy_list.safe_push (state.defs_list[0]);
+	      if (state.defs_list.length() != 0)
+		reinsn_copy_list.safe_push (state.defs_list[0]);
 	    }
 	  reinsn_del_list.safe_push (curr_cand->insn);
 	  state.modified[INSN_UID (curr_cand->insn)].deleted = 1;
@@ -1342,24 +1480,27 @@ find_and_remove_re (void)
      Remember that the memory reference will be changed to refer to the
      destination of the extention.  So we're actually emitting a copy
      from the new destination to the old destination.  */
-  for (unsigned int i = 0; i < reinsn_copy_list.length (); i += 2)
-    {
-      rtx_insn *curr_insn = reinsn_copy_list[i];
-      rtx_insn *def_insn = reinsn_copy_list[i + 1];
-
-      /* Use the mode of the destination of the defining insn
-	 for the mode of the copy.  This is necessary if the
-	 defining insn was used to eliminate a second extension
-	 that was wider than the first.  */
-      rtx sub_rtx = *get_sub_rtx (def_insn);
-      rtx set = single_set (curr_insn);
-      rtx new_dst = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
-				 REGNO (XEXP (SET_SRC (set), 0)));
-      rtx new_src = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
-				 REGNO (SET_DEST (set)));
-      rtx new_set = gen_rtx_SET (new_dst, new_src);
-      emit_insn_after (new_set, def_insn);
-    }
+   for (unsigned int i = 0; i < reinsn_copy_list.length (); i += 2)
+     {
+       rtx_insn *curr_insn = reinsn_copy_list[i];
+
+       if ((i+1) < reinsn_copy_list.length())
+	 {
+	   rtx_insn *def_insn = reinsn_copy_list[i + 1];
+	   /* Use the mode of the destination of the defining insn
+	      for the mode of the copy.  This is necessary if the
+	      defining insn was used to eliminate a second extension
+	      that was wider than the first.  */
+	   rtx sub_rtx = *get_sub_rtx (def_insn);
+	   rtx set = single_set (curr_insn);
+	   rtx new_dst = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
+				      REGNO (XEXP (SET_SRC (set), 0)));
+	   rtx new_src = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
+				      REGNO (SET_DEST (set)));
+	   rtx new_set = gen_rtx_SET (new_dst, new_src);
+	   emit_insn_after (new_set, def_insn);
+	}
+      }
 
   /* Delete all useless extensions here in one sweep.  */
   FOR_EACH_VEC_ELT (reinsn_del_list, i, curr_insn)
diff --git a/gcc/testsuite/g++.target/powerpc/zext-elim-3.C b/gcc/testsuite/g++.target/powerpc/zext-elim-3.C
new file mode 100644
index 00000000000..5a050df06ff
--- /dev/null
+++ b/gcc/testsuite/g++.target/powerpc/zext-elim-3.C
@@ -0,0 +1,13 @@
+/* { dg-options "-mcpu=power9 -O2" } */
+
+void *memset(void *b, int c, unsigned long len)
+{
+  unsigned long i;
+
+  for (i = 0; i < len; i++)
+    ((unsigned char *)b)[i] = c;
+
+   return b;
+}
+
+/* { dg-final { scan-assembler-not "\mrlwinm\M" } } */
-- 
2.39.1


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

* Re: PATCH v5 4/4] ree: Improve ree pass for rs6000 target using defined ABI interfaces.
  2023-06-01  5:23 PATCH v5 4/4] ree: Improve ree pass for rs6000 target using defined ABI interfaces Ajit Agarwal
@ 2023-06-01  6:04 ` Bernhard Reutner-Fischer
  2023-06-26 12:36 ` [PING] " Ajit Agarwal
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Bernhard Reutner-Fischer @ 2023-06-01  6:04 UTC (permalink / raw)
  To: Ajit Agarwal via Gcc-patches
  Cc: rep.dot.nop, Ajit Agarwal, Jeff Law, Segher Boessenkool,
	Peter Bergner, Richard Biener

On Thu, 1 Jun 2023 10:53:02 +0530
Ajit Agarwal via Gcc-patches <gcc-patches@gcc.gnu.org> 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.
> 
> Review comments incorporated.

Not a review:

/scratch/src/gcc-14.mine$ ./contrib/check_GNU_style.py /tmp/rs6k-ree-v5.patch 
=== ERROR type #1: there should be exactly one space between function name and parenthesis (2 error(s)) ===
gcc/ree.cc:1461:33:	      if (state.defs_list.length() != 0)
gcc/ree.cc:1487:42:       if ((i+1) < reinsn_copy_list.length())

=== ERROR type #2: trailing operator (1 error(s)) ===
gcc/ree.cc:761:24:  machine_mode tgt_mode =

$ grep -E "^[-+].*[^[:space:]\.][[:space:]][[:space:]]" /tmp/rs6k-ree-v5.patch 
+   an return  registers.  */
+  if (GET_CODE (SET_SRC (set)) !=  ZERO_EXTEND)
+  if (GET_CODE (SET_SRC (set)) !=  ZERO_EXTEND)
+  if (code !=  ZERO_EXTEND)

And maybe you could add fewer empty lines in combine_reaching_defs()

thanks,

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

* [PING] PATCH v5 4/4] ree: Improve ree pass for rs6000 target using defined ABI interfaces.
  2023-06-01  5:23 PATCH v5 4/4] ree: Improve ree pass for rs6000 target using defined ABI interfaces Ajit Agarwal
  2023-06-01  6:04 ` Bernhard Reutner-Fischer
@ 2023-06-26 12:36 ` Ajit Agarwal
  2023-06-26 12:42 ` Ajit Agarwal
  2023-07-18  7:58 ` [PING^2] " Ajit Agarwal
  3 siblings, 0 replies; 9+ messages in thread
From: Ajit Agarwal @ 2023-06-26 12:36 UTC (permalink / raw)
  To: gcc-patches

All:

Ok for trunk. Please review.


Thanks & Regards
Ajit

On 01/06/23 10:53 am, 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.
> 
> Review comments incorporated.
> 
> Thanks & Regards
> Ajit
> 
> 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 defined
> ABI interfaces.
> 
> 2023-06-01  Ajit Kumar Agarwal  <aagarwa1@linux.ibm.com>
> 
> gcc/ChangeLog:
> 
> 	* ree.cc (combine_reaching_defs): Use of  zero_extend and sign_extend
> 	defined abi interfaces.
> 	(add_removable_extension): Use of defined abi interfaces for no
> 	reaching defs.
> 	(abi_extension_candidate_return_reg_p): New function.
> 	(abi_extension_candidate_p): New function.
> 	(abi_extension_candidate_argno_p): New function.
> 	(abi_handle_regs_without_defs_p): New function.
> 	(abi_target_promote_function_mode): New function.
> 
> gcc/testsuite/ChangeLog:
> 
>         * g++.target/powerpc/zext-elim-3.C
> ---
>  gcc/ree.cc                                    | 199 +++++++++++++++---
>  .../g++.target/powerpc/zext-elim-3.C          |  13 ++
>  2 files changed, 183 insertions(+), 29 deletions(-)
>  create mode 100644 gcc/testsuite/g++.target/powerpc/zext-elim-3.C
> 
> diff --git a/gcc/ree.cc b/gcc/ree.cc
> index fc04249fa84..2025a7c43da 100644
> --- a/gcc/ree.cc
> +++ b/gcc/ree.cc
> @@ -514,7 +514,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 +751,120 @@ get_extended_src_reg (rtx src)
>    return src;
>  }
>  
> +/* Return TRUE if target mode is equal to source mode of zero_extend
> +   or sign_extend otherwise false.  */
> +
> +static bool
> +abi_target_promote_function_mode (machine_mode mode)
> +{
> +  int unsignedp;
> +  machine_mode tgt_mode =
> +    targetm.calls.promote_function_mode (NULL_TREE, mode, &unsignedp,
> +					 NULL_TREE, 1);
> +
> +  if (tgt_mode == mode)
> +    return true;
> +  else
> +    return false;
> +}
> +
> +/* 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;
> +}
> +
> +/* Return TRUE if the candidate insn doesn't have defs and have
> + * uses without RTX_BIN_ARITH/RTX_COMM_ARITH/RTX_UNARY rtx class.  */
> +
> +static bool
> +abi_handle_regs_without_defs_p (rtx_insn *insn)
> +{
> +  if (side_effects_p (PATTERN (insn)))
> +    return false;
> +
> +  struct df_link *uses
> +    = get_uses (insn, SET_DEST (PATTERN (insn)));
> +
> +  if (!uses)
> +    return false;
> +
> +  for (df_link *use = uses; use; use = use->next)
> +    {
> +      if (!use->ref)
> +	return false;
> +
> +      if (BLOCK_FOR_INSN (insn)
> +	  != BLOCK_FOR_INSN (DF_REF_INSN (use->ref)))
> +	return false;
> +
> +      rtx_insn *use_insn = DF_REF_INSN (use->ref);
> +
> +      if (GET_CODE (PATTERN (use_insn)) == SET)
> +	{
> +	  rtx_code code = GET_CODE (SET_SRC (PATTERN (use_insn)));
> +
> +	  if (GET_RTX_CLASS (code) == RTX_BIN_ARITH
> +	      || GET_RTX_CLASS (code) == RTX_COMM_ARITH
> +	      || GET_RTX_CLASS (code) == RTX_UNARY)
> +	    return false;
> +	}
> +     }
> +  return true;
> +}
> +
>  /* This function goes through all reaching defs of the source
>     of the candidate for elimination (CAND) and tries to combine
>     the extension with the definition instruction.  The changes
> @@ -770,6 +885,11 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
>  
>    state->defs_list.truncate (0);
>    state->copies_list.truncate (0);
> +  rtx orig_src = XEXP (SET_SRC (cand->expr),0);
> +
> +  if (abi_extension_candidate_p (cand->insn)
> +      && (!get_defs (cand->insn, orig_src, NULL)))
> +    return abi_handle_regs_without_defs_p (cand->insn);
>  
>    outcome = make_defs_and_copies_lists (cand->insn, set_pat, state);
>  
> @@ -1036,6 +1156,15 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
>          }
>      }
>  
> +  rtx insn_set = single_set (cand->insn);
> +
> +  machine_mode mode = (GET_MODE (XEXP (SET_SRC (insn_set), 0)));
> +
> +  bool promote_p = abi_target_promote_function_mode (mode);
> +
> +  if (promote_p)
> +    return true;
> +
>    if (merge_successful)
>      {
>        /* Commit the changes here if possible
> @@ -1112,26 +1241,34 @@ add_removable_extension (const_rtx expr, rtx_insn *insn,
>        rtx reg = XEXP (src, 0);
>        struct df_link *defs, *def;
>        ext_cand *cand;
> +      defs = get_defs (insn, reg, NULL);
>  
>        /* Zero-extension of an undefined value is partly defined (it's
>  	 completely undefined for sign-extension, though).  So if there exists
>  	 a path from the entry to this zero-extension that leaves this register
>  	 uninitialized, removing the extension could change the behavior of
>  	 correct programs.  So first, check it is not the case.  */
> -      if (code == ZERO_EXTEND && !bitmap_bit_p (init_regs, REGNO (reg)))
> +      if (!defs && abi_extension_candidate_argno_p (code, REGNO (reg)))
>  	{
> -	  if (dump_file)
> -	    {
> -	      fprintf (dump_file, "Cannot eliminate extension:\n");
> -	      print_rtl_single (dump_file, insn);
> -	      fprintf (dump_file, " because it can operate on uninitialized"
> -			          " data\n");
> -	    }
> +	  ext_cand e = {expr, code, mode, insn};
> +	  insn_list->safe_push (e);
>  	  return;
>  	}
>  
> +       if ((code == ZERO_EXTEND
> +	    && !bitmap_bit_p (init_regs, REGNO (reg))))
> +	  {
> +	    if (dump_file)
> +	      {
> +		fprintf (dump_file, "Cannot eliminate extension:\n");
> +		print_rtl_single (dump_file, insn);
> +		fprintf (dump_file, " because it can operate on uninitialized"
> +				    " data\n");
> +	      }
> +	    return;
> +	  }
> +
>        /* Second, make sure we can get all the reaching definitions.  */
> -      defs = get_defs (insn, reg, NULL);
>        if (!defs)
>  	{
>  	  if (dump_file)
> @@ -1321,7 +1458,8 @@ find_and_remove_re (void)
>  	      && (REGNO (SET_DEST (set)) != REGNO (XEXP (SET_SRC (set), 0))))
>  	    {
>                reinsn_copy_list.safe_push (curr_cand->insn);
> -              reinsn_copy_list.safe_push (state.defs_list[0]);
> +	      if (state.defs_list.length() != 0)
> +		reinsn_copy_list.safe_push (state.defs_list[0]);
>  	    }
>  	  reinsn_del_list.safe_push (curr_cand->insn);
>  	  state.modified[INSN_UID (curr_cand->insn)].deleted = 1;
> @@ -1342,24 +1480,27 @@ find_and_remove_re (void)
>       Remember that the memory reference will be changed to refer to the
>       destination of the extention.  So we're actually emitting a copy
>       from the new destination to the old destination.  */
> -  for (unsigned int i = 0; i < reinsn_copy_list.length (); i += 2)
> -    {
> -      rtx_insn *curr_insn = reinsn_copy_list[i];
> -      rtx_insn *def_insn = reinsn_copy_list[i + 1];
> -
> -      /* Use the mode of the destination of the defining insn
> -	 for the mode of the copy.  This is necessary if the
> -	 defining insn was used to eliminate a second extension
> -	 that was wider than the first.  */
> -      rtx sub_rtx = *get_sub_rtx (def_insn);
> -      rtx set = single_set (curr_insn);
> -      rtx new_dst = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
> -				 REGNO (XEXP (SET_SRC (set), 0)));
> -      rtx new_src = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
> -				 REGNO (SET_DEST (set)));
> -      rtx new_set = gen_rtx_SET (new_dst, new_src);
> -      emit_insn_after (new_set, def_insn);
> -    }
> +   for (unsigned int i = 0; i < reinsn_copy_list.length (); i += 2)
> +     {
> +       rtx_insn *curr_insn = reinsn_copy_list[i];
> +
> +       if ((i+1) < reinsn_copy_list.length())
> +	 {
> +	   rtx_insn *def_insn = reinsn_copy_list[i + 1];
> +	   /* Use the mode of the destination of the defining insn
> +	      for the mode of the copy.  This is necessary if the
> +	      defining insn was used to eliminate a second extension
> +	      that was wider than the first.  */
> +	   rtx sub_rtx = *get_sub_rtx (def_insn);
> +	   rtx set = single_set (curr_insn);
> +	   rtx new_dst = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
> +				      REGNO (XEXP (SET_SRC (set), 0)));
> +	   rtx new_src = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
> +				      REGNO (SET_DEST (set)));
> +	   rtx new_set = gen_rtx_SET (new_dst, new_src);
> +	   emit_insn_after (new_set, def_insn);
> +	}
> +      }
>  
>    /* Delete all useless extensions here in one sweep.  */
>    FOR_EACH_VEC_ELT (reinsn_del_list, i, curr_insn)
> diff --git a/gcc/testsuite/g++.target/powerpc/zext-elim-3.C b/gcc/testsuite/g++.target/powerpc/zext-elim-3.C
> new file mode 100644
> index 00000000000..5a050df06ff
> --- /dev/null
> +++ b/gcc/testsuite/g++.target/powerpc/zext-elim-3.C
> @@ -0,0 +1,13 @@
> +/* { dg-options "-mcpu=power9 -O2" } */
> +
> +void *memset(void *b, int c, unsigned long len)
> +{
> +  unsigned long i;
> +
> +  for (i = 0; i < len; i++)
> +    ((unsigned char *)b)[i] = c;
> +
> +   return b;
> +}
> +
> +/* { dg-final { scan-assembler-not "\mrlwinm\M" } } */

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

* Re: PATCH v5 4/4] ree: Improve ree pass for rs6000 target using defined ABI interfaces.
  2023-06-01  5:23 PATCH v5 4/4] ree: Improve ree pass for rs6000 target using defined ABI interfaces Ajit Agarwal
  2023-06-01  6:04 ` Bernhard Reutner-Fischer
  2023-06-26 12:36 ` [PING] " Ajit Agarwal
@ 2023-06-26 12:42 ` Ajit Agarwal
  2023-06-26 12:45   ` [PING] " Ajit Agarwal
  2023-07-18  7:58 ` [PING^2] " Ajit Agarwal
  3 siblings, 1 reply; 9+ messages in thread
From: Ajit Agarwal @ 2023-06-26 12:42 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jeff Law, Segher Boessenkool, Peter Bergner, Richard Biener

All:

Ok for trunk. Please review.

Thanks & Regards
Ajit

On 01/06/23 10:53 am, 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.
> 
> Review comments incorporated.
> 
> Thanks & Regards
> Ajit
> 
> 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 defined
> ABI interfaces.
> 
> 2023-06-01  Ajit Kumar Agarwal  <aagarwa1@linux.ibm.com>
> 
> gcc/ChangeLog:
> 
> 	* ree.cc (combine_reaching_defs): Use of  zero_extend and sign_extend
> 	defined abi interfaces.
> 	(add_removable_extension): Use of defined abi interfaces for no
> 	reaching defs.
> 	(abi_extension_candidate_return_reg_p): New function.
> 	(abi_extension_candidate_p): New function.
> 	(abi_extension_candidate_argno_p): New function.
> 	(abi_handle_regs_without_defs_p): New function.
> 	(abi_target_promote_function_mode): New function.
> 
> gcc/testsuite/ChangeLog:
> 
>         * g++.target/powerpc/zext-elim-3.C
> ---
>  gcc/ree.cc                                    | 199 +++++++++++++++---
>  .../g++.target/powerpc/zext-elim-3.C          |  13 ++
>  2 files changed, 183 insertions(+), 29 deletions(-)
>  create mode 100644 gcc/testsuite/g++.target/powerpc/zext-elim-3.C
> 
> diff --git a/gcc/ree.cc b/gcc/ree.cc
> index fc04249fa84..2025a7c43da 100644
> --- a/gcc/ree.cc
> +++ b/gcc/ree.cc
> @@ -514,7 +514,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 +751,120 @@ get_extended_src_reg (rtx src)
>    return src;
>  }
>  
> +/* Return TRUE if target mode is equal to source mode of zero_extend
> +   or sign_extend otherwise false.  */
> +
> +static bool
> +abi_target_promote_function_mode (machine_mode mode)
> +{
> +  int unsignedp;
> +  machine_mode tgt_mode =
> +    targetm.calls.promote_function_mode (NULL_TREE, mode, &unsignedp,
> +					 NULL_TREE, 1);
> +
> +  if (tgt_mode == mode)
> +    return true;
> +  else
> +    return false;
> +}
> +
> +/* 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;
> +}
> +
> +/* Return TRUE if the candidate insn doesn't have defs and have
> + * uses without RTX_BIN_ARITH/RTX_COMM_ARITH/RTX_UNARY rtx class.  */
> +
> +static bool
> +abi_handle_regs_without_defs_p (rtx_insn *insn)
> +{
> +  if (side_effects_p (PATTERN (insn)))
> +    return false;
> +
> +  struct df_link *uses
> +    = get_uses (insn, SET_DEST (PATTERN (insn)));
> +
> +  if (!uses)
> +    return false;
> +
> +  for (df_link *use = uses; use; use = use->next)
> +    {
> +      if (!use->ref)
> +	return false;
> +
> +      if (BLOCK_FOR_INSN (insn)
> +	  != BLOCK_FOR_INSN (DF_REF_INSN (use->ref)))
> +	return false;
> +
> +      rtx_insn *use_insn = DF_REF_INSN (use->ref);
> +
> +      if (GET_CODE (PATTERN (use_insn)) == SET)
> +	{
> +	  rtx_code code = GET_CODE (SET_SRC (PATTERN (use_insn)));
> +
> +	  if (GET_RTX_CLASS (code) == RTX_BIN_ARITH
> +	      || GET_RTX_CLASS (code) == RTX_COMM_ARITH
> +	      || GET_RTX_CLASS (code) == RTX_UNARY)
> +	    return false;
> +	}
> +     }
> +  return true;
> +}
> +
>  /* This function goes through all reaching defs of the source
>     of the candidate for elimination (CAND) and tries to combine
>     the extension with the definition instruction.  The changes
> @@ -770,6 +885,11 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
>  
>    state->defs_list.truncate (0);
>    state->copies_list.truncate (0);
> +  rtx orig_src = XEXP (SET_SRC (cand->expr),0);
> +
> +  if (abi_extension_candidate_p (cand->insn)
> +      && (!get_defs (cand->insn, orig_src, NULL)))
> +    return abi_handle_regs_without_defs_p (cand->insn);
>  
>    outcome = make_defs_and_copies_lists (cand->insn, set_pat, state);
>  
> @@ -1036,6 +1156,15 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
>          }
>      }
>  
> +  rtx insn_set = single_set (cand->insn);
> +
> +  machine_mode mode = (GET_MODE (XEXP (SET_SRC (insn_set), 0)));
> +
> +  bool promote_p = abi_target_promote_function_mode (mode);
> +
> +  if (promote_p)
> +    return true;
> +
>    if (merge_successful)
>      {
>        /* Commit the changes here if possible
> @@ -1112,26 +1241,34 @@ add_removable_extension (const_rtx expr, rtx_insn *insn,
>        rtx reg = XEXP (src, 0);
>        struct df_link *defs, *def;
>        ext_cand *cand;
> +      defs = get_defs (insn, reg, NULL);
>  
>        /* Zero-extension of an undefined value is partly defined (it's
>  	 completely undefined for sign-extension, though).  So if there exists
>  	 a path from the entry to this zero-extension that leaves this register
>  	 uninitialized, removing the extension could change the behavior of
>  	 correct programs.  So first, check it is not the case.  */
> -      if (code == ZERO_EXTEND && !bitmap_bit_p (init_regs, REGNO (reg)))
> +      if (!defs && abi_extension_candidate_argno_p (code, REGNO (reg)))
>  	{
> -	  if (dump_file)
> -	    {
> -	      fprintf (dump_file, "Cannot eliminate extension:\n");
> -	      print_rtl_single (dump_file, insn);
> -	      fprintf (dump_file, " because it can operate on uninitialized"
> -			          " data\n");
> -	    }
> +	  ext_cand e = {expr, code, mode, insn};
> +	  insn_list->safe_push (e);
>  	  return;
>  	}
>  
> +       if ((code == ZERO_EXTEND
> +	    && !bitmap_bit_p (init_regs, REGNO (reg))))
> +	  {
> +	    if (dump_file)
> +	      {
> +		fprintf (dump_file, "Cannot eliminate extension:\n");
> +		print_rtl_single (dump_file, insn);
> +		fprintf (dump_file, " because it can operate on uninitialized"
> +				    " data\n");
> +	      }
> +	    return;
> +	  }
> +
>        /* Second, make sure we can get all the reaching definitions.  */
> -      defs = get_defs (insn, reg, NULL);
>        if (!defs)
>  	{
>  	  if (dump_file)
> @@ -1321,7 +1458,8 @@ find_and_remove_re (void)
>  	      && (REGNO (SET_DEST (set)) != REGNO (XEXP (SET_SRC (set), 0))))
>  	    {
>                reinsn_copy_list.safe_push (curr_cand->insn);
> -              reinsn_copy_list.safe_push (state.defs_list[0]);
> +	      if (state.defs_list.length() != 0)
> +		reinsn_copy_list.safe_push (state.defs_list[0]);
>  	    }
>  	  reinsn_del_list.safe_push (curr_cand->insn);
>  	  state.modified[INSN_UID (curr_cand->insn)].deleted = 1;
> @@ -1342,24 +1480,27 @@ find_and_remove_re (void)
>       Remember that the memory reference will be changed to refer to the
>       destination of the extention.  So we're actually emitting a copy
>       from the new destination to the old destination.  */
> -  for (unsigned int i = 0; i < reinsn_copy_list.length (); i += 2)
> -    {
> -      rtx_insn *curr_insn = reinsn_copy_list[i];
> -      rtx_insn *def_insn = reinsn_copy_list[i + 1];
> -
> -      /* Use the mode of the destination of the defining insn
> -	 for the mode of the copy.  This is necessary if the
> -	 defining insn was used to eliminate a second extension
> -	 that was wider than the first.  */
> -      rtx sub_rtx = *get_sub_rtx (def_insn);
> -      rtx set = single_set (curr_insn);
> -      rtx new_dst = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
> -				 REGNO (XEXP (SET_SRC (set), 0)));
> -      rtx new_src = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
> -				 REGNO (SET_DEST (set)));
> -      rtx new_set = gen_rtx_SET (new_dst, new_src);
> -      emit_insn_after (new_set, def_insn);
> -    }
> +   for (unsigned int i = 0; i < reinsn_copy_list.length (); i += 2)
> +     {
> +       rtx_insn *curr_insn = reinsn_copy_list[i];
> +
> +       if ((i+1) < reinsn_copy_list.length())
> +	 {
> +	   rtx_insn *def_insn = reinsn_copy_list[i + 1];
> +	   /* Use the mode of the destination of the defining insn
> +	      for the mode of the copy.  This is necessary if the
> +	      defining insn was used to eliminate a second extension
> +	      that was wider than the first.  */
> +	   rtx sub_rtx = *get_sub_rtx (def_insn);
> +	   rtx set = single_set (curr_insn);
> +	   rtx new_dst = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
> +				      REGNO (XEXP (SET_SRC (set), 0)));
> +	   rtx new_src = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
> +				      REGNO (SET_DEST (set)));
> +	   rtx new_set = gen_rtx_SET (new_dst, new_src);
> +	   emit_insn_after (new_set, def_insn);
> +	}
> +      }
>  
>    /* Delete all useless extensions here in one sweep.  */
>    FOR_EACH_VEC_ELT (reinsn_del_list, i, curr_insn)
> diff --git a/gcc/testsuite/g++.target/powerpc/zext-elim-3.C b/gcc/testsuite/g++.target/powerpc/zext-elim-3.C
> new file mode 100644
> index 00000000000..5a050df06ff
> --- /dev/null
> +++ b/gcc/testsuite/g++.target/powerpc/zext-elim-3.C
> @@ -0,0 +1,13 @@
> +/* { dg-options "-mcpu=power9 -O2" } */
> +
> +void *memset(void *b, int c, unsigned long len)
> +{
> +  unsigned long i;
> +
> +  for (i = 0; i < len; i++)
> +    ((unsigned char *)b)[i] = c;
> +
> +   return b;
> +}
> +
> +/* { dg-final { scan-assembler-not "\mrlwinm\M" } } */

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

* [PING] PATCH v5 4/4] ree: Improve ree pass for rs6000 target using defined ABI interfaces.
  2023-06-26 12:42 ` Ajit Agarwal
@ 2023-06-26 12:45   ` Ajit Agarwal
  0 siblings, 0 replies; 9+ messages in thread
From: Ajit Agarwal @ 2023-06-26 12:45 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jeff Law, Segher Boessenkool, Peter Bergner, Richard Biener

All:

Ok for trunk. Please review.

Thanks & Regards
Ajit

On 26/06/23 6:12 pm, Ajit Agarwal via Gcc-patches wrote:
> All:
> 
> Ok for trunk. Please review.
> 
> Thanks & Regards
> Ajit
> 
> On 01/06/23 10:53 am, 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.
>>
>> Review comments incorporated.
>>
>> Thanks & Regards
>> Ajit
>>
>> 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 defined
>> ABI interfaces.
>>
>> 2023-06-01  Ajit Kumar Agarwal  <aagarwa1@linux.ibm.com>
>>
>> gcc/ChangeLog:
>>
>> 	* ree.cc (combine_reaching_defs): Use of  zero_extend and sign_extend
>> 	defined abi interfaces.
>> 	(add_removable_extension): Use of defined abi interfaces for no
>> 	reaching defs.
>> 	(abi_extension_candidate_return_reg_p): New function.
>> 	(abi_extension_candidate_p): New function.
>> 	(abi_extension_candidate_argno_p): New function.
>> 	(abi_handle_regs_without_defs_p): New function.
>> 	(abi_target_promote_function_mode): New function.
>>
>> gcc/testsuite/ChangeLog:
>>
>>         * g++.target/powerpc/zext-elim-3.C
>> ---
>>  gcc/ree.cc                                    | 199 +++++++++++++++---
>>  .../g++.target/powerpc/zext-elim-3.C          |  13 ++
>>  2 files changed, 183 insertions(+), 29 deletions(-)
>>  create mode 100644 gcc/testsuite/g++.target/powerpc/zext-elim-3.C
>>
>> diff --git a/gcc/ree.cc b/gcc/ree.cc
>> index fc04249fa84..2025a7c43da 100644
>> --- a/gcc/ree.cc
>> +++ b/gcc/ree.cc
>> @@ -514,7 +514,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 +751,120 @@ get_extended_src_reg (rtx src)
>>    return src;
>>  }
>>  
>> +/* Return TRUE if target mode is equal to source mode of zero_extend
>> +   or sign_extend otherwise false.  */
>> +
>> +static bool
>> +abi_target_promote_function_mode (machine_mode mode)
>> +{
>> +  int unsignedp;
>> +  machine_mode tgt_mode =
>> +    targetm.calls.promote_function_mode (NULL_TREE, mode, &unsignedp,
>> +					 NULL_TREE, 1);
>> +
>> +  if (tgt_mode == mode)
>> +    return true;
>> +  else
>> +    return false;
>> +}
>> +
>> +/* 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;
>> +}
>> +
>> +/* Return TRUE if the candidate insn doesn't have defs and have
>> + * uses without RTX_BIN_ARITH/RTX_COMM_ARITH/RTX_UNARY rtx class.  */
>> +
>> +static bool
>> +abi_handle_regs_without_defs_p (rtx_insn *insn)
>> +{
>> +  if (side_effects_p (PATTERN (insn)))
>> +    return false;
>> +
>> +  struct df_link *uses
>> +    = get_uses (insn, SET_DEST (PATTERN (insn)));
>> +
>> +  if (!uses)
>> +    return false;
>> +
>> +  for (df_link *use = uses; use; use = use->next)
>> +    {
>> +      if (!use->ref)
>> +	return false;
>> +
>> +      if (BLOCK_FOR_INSN (insn)
>> +	  != BLOCK_FOR_INSN (DF_REF_INSN (use->ref)))
>> +	return false;
>> +
>> +      rtx_insn *use_insn = DF_REF_INSN (use->ref);
>> +
>> +      if (GET_CODE (PATTERN (use_insn)) == SET)
>> +	{
>> +	  rtx_code code = GET_CODE (SET_SRC (PATTERN (use_insn)));
>> +
>> +	  if (GET_RTX_CLASS (code) == RTX_BIN_ARITH
>> +	      || GET_RTX_CLASS (code) == RTX_COMM_ARITH
>> +	      || GET_RTX_CLASS (code) == RTX_UNARY)
>> +	    return false;
>> +	}
>> +     }
>> +  return true;
>> +}
>> +
>>  /* This function goes through all reaching defs of the source
>>     of the candidate for elimination (CAND) and tries to combine
>>     the extension with the definition instruction.  The changes
>> @@ -770,6 +885,11 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
>>  
>>    state->defs_list.truncate (0);
>>    state->copies_list.truncate (0);
>> +  rtx orig_src = XEXP (SET_SRC (cand->expr),0);
>> +
>> +  if (abi_extension_candidate_p (cand->insn)
>> +      && (!get_defs (cand->insn, orig_src, NULL)))
>> +    return abi_handle_regs_without_defs_p (cand->insn);
>>  
>>    outcome = make_defs_and_copies_lists (cand->insn, set_pat, state);
>>  
>> @@ -1036,6 +1156,15 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
>>          }
>>      }
>>  
>> +  rtx insn_set = single_set (cand->insn);
>> +
>> +  machine_mode mode = (GET_MODE (XEXP (SET_SRC (insn_set), 0)));
>> +
>> +  bool promote_p = abi_target_promote_function_mode (mode);
>> +
>> +  if (promote_p)
>> +    return true;
>> +
>>    if (merge_successful)
>>      {
>>        /* Commit the changes here if possible
>> @@ -1112,26 +1241,34 @@ add_removable_extension (const_rtx expr, rtx_insn *insn,
>>        rtx reg = XEXP (src, 0);
>>        struct df_link *defs, *def;
>>        ext_cand *cand;
>> +      defs = get_defs (insn, reg, NULL);
>>  
>>        /* Zero-extension of an undefined value is partly defined (it's
>>  	 completely undefined for sign-extension, though).  So if there exists
>>  	 a path from the entry to this zero-extension that leaves this register
>>  	 uninitialized, removing the extension could change the behavior of
>>  	 correct programs.  So first, check it is not the case.  */
>> -      if (code == ZERO_EXTEND && !bitmap_bit_p (init_regs, REGNO (reg)))
>> +      if (!defs && abi_extension_candidate_argno_p (code, REGNO (reg)))
>>  	{
>> -	  if (dump_file)
>> -	    {
>> -	      fprintf (dump_file, "Cannot eliminate extension:\n");
>> -	      print_rtl_single (dump_file, insn);
>> -	      fprintf (dump_file, " because it can operate on uninitialized"
>> -			          " data\n");
>> -	    }
>> +	  ext_cand e = {expr, code, mode, insn};
>> +	  insn_list->safe_push (e);
>>  	  return;
>>  	}
>>  
>> +       if ((code == ZERO_EXTEND
>> +	    && !bitmap_bit_p (init_regs, REGNO (reg))))
>> +	  {
>> +	    if (dump_file)
>> +	      {
>> +		fprintf (dump_file, "Cannot eliminate extension:\n");
>> +		print_rtl_single (dump_file, insn);
>> +		fprintf (dump_file, " because it can operate on uninitialized"
>> +				    " data\n");
>> +	      }
>> +	    return;
>> +	  }
>> +
>>        /* Second, make sure we can get all the reaching definitions.  */
>> -      defs = get_defs (insn, reg, NULL);
>>        if (!defs)
>>  	{
>>  	  if (dump_file)
>> @@ -1321,7 +1458,8 @@ find_and_remove_re (void)
>>  	      && (REGNO (SET_DEST (set)) != REGNO (XEXP (SET_SRC (set), 0))))
>>  	    {
>>                reinsn_copy_list.safe_push (curr_cand->insn);
>> -              reinsn_copy_list.safe_push (state.defs_list[0]);
>> +	      if (state.defs_list.length() != 0)
>> +		reinsn_copy_list.safe_push (state.defs_list[0]);
>>  	    }
>>  	  reinsn_del_list.safe_push (curr_cand->insn);
>>  	  state.modified[INSN_UID (curr_cand->insn)].deleted = 1;
>> @@ -1342,24 +1480,27 @@ find_and_remove_re (void)
>>       Remember that the memory reference will be changed to refer to the
>>       destination of the extention.  So we're actually emitting a copy
>>       from the new destination to the old destination.  */
>> -  for (unsigned int i = 0; i < reinsn_copy_list.length (); i += 2)
>> -    {
>> -      rtx_insn *curr_insn = reinsn_copy_list[i];
>> -      rtx_insn *def_insn = reinsn_copy_list[i + 1];
>> -
>> -      /* Use the mode of the destination of the defining insn
>> -	 for the mode of the copy.  This is necessary if the
>> -	 defining insn was used to eliminate a second extension
>> -	 that was wider than the first.  */
>> -      rtx sub_rtx = *get_sub_rtx (def_insn);
>> -      rtx set = single_set (curr_insn);
>> -      rtx new_dst = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
>> -				 REGNO (XEXP (SET_SRC (set), 0)));
>> -      rtx new_src = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
>> -				 REGNO (SET_DEST (set)));
>> -      rtx new_set = gen_rtx_SET (new_dst, new_src);
>> -      emit_insn_after (new_set, def_insn);
>> -    }
>> +   for (unsigned int i = 0; i < reinsn_copy_list.length (); i += 2)
>> +     {
>> +       rtx_insn *curr_insn = reinsn_copy_list[i];
>> +
>> +       if ((i+1) < reinsn_copy_list.length())
>> +	 {
>> +	   rtx_insn *def_insn = reinsn_copy_list[i + 1];
>> +	   /* Use the mode of the destination of the defining insn
>> +	      for the mode of the copy.  This is necessary if the
>> +	      defining insn was used to eliminate a second extension
>> +	      that was wider than the first.  */
>> +	   rtx sub_rtx = *get_sub_rtx (def_insn);
>> +	   rtx set = single_set (curr_insn);
>> +	   rtx new_dst = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
>> +				      REGNO (XEXP (SET_SRC (set), 0)));
>> +	   rtx new_src = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
>> +				      REGNO (SET_DEST (set)));
>> +	   rtx new_set = gen_rtx_SET (new_dst, new_src);
>> +	   emit_insn_after (new_set, def_insn);
>> +	}
>> +      }
>>  
>>    /* Delete all useless extensions here in one sweep.  */
>>    FOR_EACH_VEC_ELT (reinsn_del_list, i, curr_insn)
>> diff --git a/gcc/testsuite/g++.target/powerpc/zext-elim-3.C b/gcc/testsuite/g++.target/powerpc/zext-elim-3.C
>> new file mode 100644
>> index 00000000000..5a050df06ff
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.target/powerpc/zext-elim-3.C
>> @@ -0,0 +1,13 @@
>> +/* { dg-options "-mcpu=power9 -O2" } */
>> +
>> +void *memset(void *b, int c, unsigned long len)
>> +{
>> +  unsigned long i;
>> +
>> +  for (i = 0; i < len; i++)
>> +    ((unsigned char *)b)[i] = c;
>> +
>> +   return b;
>> +}
>> +
>> +/* { dg-final { scan-assembler-not "\mrlwinm\M" } } */

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

* [PING^2] PATCH v5 4/4] ree: Improve ree pass for rs6000 target using defined ABI interfaces.
  2023-06-01  5:23 PATCH v5 4/4] ree: Improve ree pass for rs6000 target using defined ABI interfaces Ajit Agarwal
                   ` (2 preceding siblings ...)
  2023-06-26 12:42 ` Ajit Agarwal
@ 2023-07-18  7:58 ` Ajit Agarwal
  2023-08-01  8:18   ` [PING^3] " Ajit Agarwal
  3 siblings, 1 reply; 9+ messages in thread
From: Ajit Agarwal @ 2023-07-18  7:58 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jeff Law, Richard Biener, Segher Boessenkool, Peter Bergner


Ping^2.

Please review.

Thanks & Regards
Ajit


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

Review comments incorporated.

Thanks & Regards
Ajit

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 defined
ABI interfaces.

2023-06-01  Ajit Kumar Agarwal  <aagarwa1@linux.ibm.com>

gcc/ChangeLog:

	* ree.cc (combine_reaching_defs): Use of  zero_extend and sign_extend
	defined abi interfaces.
	(add_removable_extension): Use of defined abi interfaces for no
	reaching defs.
	(abi_extension_candidate_return_reg_p): New function.
	(abi_extension_candidate_p): New function.
	(abi_extension_candidate_argno_p): New function.
	(abi_handle_regs_without_defs_p): New function.
	(abi_target_promote_function_mode): New function.

gcc/testsuite/ChangeLog:

        * g++.target/powerpc/zext-elim-3.C
---
 gcc/ree.cc                                    | 199 +++++++++++++++---
 .../g++.target/powerpc/zext-elim-3.C          |  13 ++
 2 files changed, 183 insertions(+), 29 deletions(-)
 create mode 100644 gcc/testsuite/g++.target/powerpc/zext-elim-3.C

diff --git a/gcc/ree.cc b/gcc/ree.cc
index fc04249fa84..2025a7c43da 100644
--- a/gcc/ree.cc
+++ b/gcc/ree.cc
@@ -514,7 +514,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 +751,120 @@ get_extended_src_reg (rtx src)
   return src;
 }
 
+/* Return TRUE if target mode is equal to source mode of zero_extend
+   or sign_extend otherwise false.  */
+
+static bool
+abi_target_promote_function_mode (machine_mode mode)
+{
+  int unsignedp;
+  machine_mode tgt_mode =
+    targetm.calls.promote_function_mode (NULL_TREE, mode, &unsignedp,
+					 NULL_TREE, 1);
+
+  if (tgt_mode == mode)
+    return true;
+  else
+    return false;
+}
+
+/* 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;
+}
+
+/* Return TRUE if the candidate insn doesn't have defs and have
+ * uses without RTX_BIN_ARITH/RTX_COMM_ARITH/RTX_UNARY rtx class.  */
+
+static bool
+abi_handle_regs_without_defs_p (rtx_insn *insn)
+{
+  if (side_effects_p (PATTERN (insn)))
+    return false;
+
+  struct df_link *uses
+    = get_uses (insn, SET_DEST (PATTERN (insn)));
+
+  if (!uses)
+    return false;
+
+  for (df_link *use = uses; use; use = use->next)
+    {
+      if (!use->ref)
+	return false;
+
+      if (BLOCK_FOR_INSN (insn)
+	  != BLOCK_FOR_INSN (DF_REF_INSN (use->ref)))
+	return false;
+
+      rtx_insn *use_insn = DF_REF_INSN (use->ref);
+
+      if (GET_CODE (PATTERN (use_insn)) == SET)
+	{
+	  rtx_code code = GET_CODE (SET_SRC (PATTERN (use_insn)));
+
+	  if (GET_RTX_CLASS (code) == RTX_BIN_ARITH
+	      || GET_RTX_CLASS (code) == RTX_COMM_ARITH
+	      || GET_RTX_CLASS (code) == RTX_UNARY)
+	    return false;
+	}
+     }
+  return true;
+}
+
 /* This function goes through all reaching defs of the source
    of the candidate for elimination (CAND) and tries to combine
    the extension with the definition instruction.  The changes
@@ -770,6 +885,11 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
 
   state->defs_list.truncate (0);
   state->copies_list.truncate (0);
+  rtx orig_src = XEXP (SET_SRC (cand->expr),0);
+
+  if (abi_extension_candidate_p (cand->insn)
+      && (!get_defs (cand->insn, orig_src, NULL)))
+    return abi_handle_regs_without_defs_p (cand->insn);
 
   outcome = make_defs_and_copies_lists (cand->insn, set_pat, state);
 
@@ -1036,6 +1156,15 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
         }
     }
 
+  rtx insn_set = single_set (cand->insn);
+
+  machine_mode mode = (GET_MODE (XEXP (SET_SRC (insn_set), 0)));
+
+  bool promote_p = abi_target_promote_function_mode (mode);
+
+  if (promote_p)
+    return true;
+
   if (merge_successful)
     {
       /* Commit the changes here if possible
@@ -1112,26 +1241,34 @@ add_removable_extension (const_rtx expr, rtx_insn *insn,
       rtx reg = XEXP (src, 0);
       struct df_link *defs, *def;
       ext_cand *cand;
+      defs = get_defs (insn, reg, NULL);
 
       /* Zero-extension of an undefined value is partly defined (it's
 	 completely undefined for sign-extension, though).  So if there exists
 	 a path from the entry to this zero-extension that leaves this register
 	 uninitialized, removing the extension could change the behavior of
 	 correct programs.  So first, check it is not the case.  */
-      if (code == ZERO_EXTEND && !bitmap_bit_p (init_regs, REGNO (reg)))
+      if (!defs && abi_extension_candidate_argno_p (code, REGNO (reg)))
 	{
-	  if (dump_file)
-	    {
-	      fprintf (dump_file, "Cannot eliminate extension:\n");
-	      print_rtl_single (dump_file, insn);
-	      fprintf (dump_file, " because it can operate on uninitialized"
-			          " data\n");
-	    }
+	  ext_cand e = {expr, code, mode, insn};
+	  insn_list->safe_push (e);
 	  return;
 	}
 
+       if ((code == ZERO_EXTEND
+	    && !bitmap_bit_p (init_regs, REGNO (reg))))
+	  {
+	    if (dump_file)
+	      {
+		fprintf (dump_file, "Cannot eliminate extension:\n");
+		print_rtl_single (dump_file, insn);
+		fprintf (dump_file, " because it can operate on uninitialized"
+				    " data\n");
+	      }
+	    return;
+	  }
+
       /* Second, make sure we can get all the reaching definitions.  */
-      defs = get_defs (insn, reg, NULL);
       if (!defs)
 	{
 	  if (dump_file)
@@ -1321,7 +1458,8 @@ find_and_remove_re (void)
 	      && (REGNO (SET_DEST (set)) != REGNO (XEXP (SET_SRC (set), 0))))
 	    {
               reinsn_copy_list.safe_push (curr_cand->insn);
-              reinsn_copy_list.safe_push (state.defs_list[0]);
+	      if (state.defs_list.length() != 0)
+		reinsn_copy_list.safe_push (state.defs_list[0]);
 	    }
 	  reinsn_del_list.safe_push (curr_cand->insn);
 	  state.modified[INSN_UID (curr_cand->insn)].deleted = 1;
@@ -1342,24 +1480,27 @@ find_and_remove_re (void)
      Remember that the memory reference will be changed to refer to the
      destination of the extention.  So we're actually emitting a copy
      from the new destination to the old destination.  */
-  for (unsigned int i = 0; i < reinsn_copy_list.length (); i += 2)
-    {
-      rtx_insn *curr_insn = reinsn_copy_list[i];
-      rtx_insn *def_insn = reinsn_copy_list[i + 1];
-
-      /* Use the mode of the destination of the defining insn
-	 for the mode of the copy.  This is necessary if the
-	 defining insn was used to eliminate a second extension
-	 that was wider than the first.  */
-      rtx sub_rtx = *get_sub_rtx (def_insn);
-      rtx set = single_set (curr_insn);
-      rtx new_dst = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
-				 REGNO (XEXP (SET_SRC (set), 0)));
-      rtx new_src = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
-				 REGNO (SET_DEST (set)));
-      rtx new_set = gen_rtx_SET (new_dst, new_src);
-      emit_insn_after (new_set, def_insn);
-    }
+   for (unsigned int i = 0; i < reinsn_copy_list.length (); i += 2)
+     {
+       rtx_insn *curr_insn = reinsn_copy_list[i];
+
+       if ((i+1) < reinsn_copy_list.length())
+	 {
+	   rtx_insn *def_insn = reinsn_copy_list[i + 1];
+	   /* Use the mode of the destination of the defining insn
+	      for the mode of the copy.  This is necessary if the
+	      defining insn was used to eliminate a second extension
+	      that was wider than the first.  */
+	   rtx sub_rtx = *get_sub_rtx (def_insn);
+	   rtx set = single_set (curr_insn);
+	   rtx new_dst = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
+				      REGNO (XEXP (SET_SRC (set), 0)));
+	   rtx new_src = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
+				      REGNO (SET_DEST (set)));
+	   rtx new_set = gen_rtx_SET (new_dst, new_src);
+	   emit_insn_after (new_set, def_insn);
+	}
+      }
 
   /* Delete all useless extensions here in one sweep.  */
   FOR_EACH_VEC_ELT (reinsn_del_list, i, curr_insn)
diff --git a/gcc/testsuite/g++.target/powerpc/zext-elim-3.C b/gcc/testsuite/g++.target/powerpc/zext-elim-3.C
new file mode 100644
index 00000000000..5a050df06ff
--- /dev/null
+++ b/gcc/testsuite/g++.target/powerpc/zext-elim-3.C
@@ -0,0 +1,13 @@
+/* { dg-options "-mcpu=power9 -O2" } */
+
+void *memset(void *b, int c, unsigned long len)
+{
+  unsigned long i;
+
+  for (i = 0; i < len; i++)
+    ((unsigned char *)b)[i] = c;
+
+   return b;
+}
+
+/* { dg-final { scan-assembler-not "\mrlwinm\M" } } */
-- 
2.39.1


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

* [PING^3] PATCH v5 4/4] ree: Improve ree pass for rs6000 target using defined ABI interfaces.
  2023-07-18  7:58 ` [PING^2] " Ajit Agarwal
@ 2023-08-01  8:18   ` Ajit Agarwal
  2023-08-21  6:46     ` [PING^4] " Ajit Agarwal
  0 siblings, 1 reply; 9+ messages in thread
From: Ajit Agarwal @ 2023-08-01  8:18 UTC (permalink / raw)
  To: gcc-patches, Jeff Law, Richard Biener, Peter Bergner,
	Segher Boessenkool, Rashmi.Sridhar

Ping!


-------- Forwarded Message --------
Subject: [PING^2] PATCH v5 4/4] ree: Improve ree pass for rs6000 target using defined ABI interfaces.
Date: Tue, 18 Jul 2023 13:28:08 +0530
From: Ajit Agarwal <aagarwa1@linux.ibm.com>
To: gcc-patches <gcc-patches@gcc.gnu.org>
CC: Jeff Law <jeffreyalaw@gmail.com>, Richard Biener <richard.guenther@gmail.com>, Segher Boessenkool <segher@kernel.crashing.org>, Peter Bergner <bergner@linux.ibm.com>


Ping^2.

Please review.

Thanks & Regards
Ajit


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

Review comments incorporated.

Thanks & Regards
Ajit

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 defined
ABI interfaces.

2023-06-01  Ajit Kumar Agarwal  <aagarwa1@linux.ibm.com>

gcc/ChangeLog:

	* ree.cc (combine_reaching_defs): Use of  zero_extend and sign_extend
	defined abi interfaces.
	(add_removable_extension): Use of defined abi interfaces for no
	reaching defs.
	(abi_extension_candidate_return_reg_p): New function.
	(abi_extension_candidate_p): New function.
	(abi_extension_candidate_argno_p): New function.
	(abi_handle_regs_without_defs_p): New function.
	(abi_target_promote_function_mode): New function.

gcc/testsuite/ChangeLog:

        * g++.target/powerpc/zext-elim-3.C
---
 gcc/ree.cc                                    | 199 +++++++++++++++---
 .../g++.target/powerpc/zext-elim-3.C          |  13 ++
 2 files changed, 183 insertions(+), 29 deletions(-)
 create mode 100644 gcc/testsuite/g++.target/powerpc/zext-elim-3.C

diff --git a/gcc/ree.cc b/gcc/ree.cc
index fc04249fa84..2025a7c43da 100644
--- a/gcc/ree.cc
+++ b/gcc/ree.cc
@@ -514,7 +514,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 +751,120 @@ get_extended_src_reg (rtx src)
   return src;
 }
 
+/* Return TRUE if target mode is equal to source mode of zero_extend
+   or sign_extend otherwise false.  */
+
+static bool
+abi_target_promote_function_mode (machine_mode mode)
+{
+  int unsignedp;
+  machine_mode tgt_mode =
+    targetm.calls.promote_function_mode (NULL_TREE, mode, &unsignedp,
+					 NULL_TREE, 1);
+
+  if (tgt_mode == mode)
+    return true;
+  else
+    return false;
+}
+
+/* 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;
+}
+
+/* Return TRUE if the candidate insn doesn't have defs and have
+ * uses without RTX_BIN_ARITH/RTX_COMM_ARITH/RTX_UNARY rtx class.  */
+
+static bool
+abi_handle_regs_without_defs_p (rtx_insn *insn)
+{
+  if (side_effects_p (PATTERN (insn)))
+    return false;
+
+  struct df_link *uses
+    = get_uses (insn, SET_DEST (PATTERN (insn)));
+
+  if (!uses)
+    return false;
+
+  for (df_link *use = uses; use; use = use->next)
+    {
+      if (!use->ref)
+	return false;
+
+      if (BLOCK_FOR_INSN (insn)
+	  != BLOCK_FOR_INSN (DF_REF_INSN (use->ref)))
+	return false;
+
+      rtx_insn *use_insn = DF_REF_INSN (use->ref);
+
+      if (GET_CODE (PATTERN (use_insn)) == SET)
+	{
+	  rtx_code code = GET_CODE (SET_SRC (PATTERN (use_insn)));
+
+	  if (GET_RTX_CLASS (code) == RTX_BIN_ARITH
+	      || GET_RTX_CLASS (code) == RTX_COMM_ARITH
+	      || GET_RTX_CLASS (code) == RTX_UNARY)
+	    return false;
+	}
+     }
+  return true;
+}
+
 /* This function goes through all reaching defs of the source
    of the candidate for elimination (CAND) and tries to combine
    the extension with the definition instruction.  The changes
@@ -770,6 +885,11 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
 
   state->defs_list.truncate (0);
   state->copies_list.truncate (0);
+  rtx orig_src = XEXP (SET_SRC (cand->expr),0);
+
+  if (abi_extension_candidate_p (cand->insn)
+      && (!get_defs (cand->insn, orig_src, NULL)))
+    return abi_handle_regs_without_defs_p (cand->insn);
 
   outcome = make_defs_and_copies_lists (cand->insn, set_pat, state);
 
@@ -1036,6 +1156,15 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
         }
     }
 
+  rtx insn_set = single_set (cand->insn);
+
+  machine_mode mode = (GET_MODE (XEXP (SET_SRC (insn_set), 0)));
+
+  bool promote_p = abi_target_promote_function_mode (mode);
+
+  if (promote_p)
+    return true;
+
   if (merge_successful)
     {
       /* Commit the changes here if possible
@@ -1112,26 +1241,34 @@ add_removable_extension (const_rtx expr, rtx_insn *insn,
       rtx reg = XEXP (src, 0);
       struct df_link *defs, *def;
       ext_cand *cand;
+      defs = get_defs (insn, reg, NULL);
 
       /* Zero-extension of an undefined value is partly defined (it's
 	 completely undefined for sign-extension, though).  So if there exists
 	 a path from the entry to this zero-extension that leaves this register
 	 uninitialized, removing the extension could change the behavior of
 	 correct programs.  So first, check it is not the case.  */
-      if (code == ZERO_EXTEND && !bitmap_bit_p (init_regs, REGNO (reg)))
+      if (!defs && abi_extension_candidate_argno_p (code, REGNO (reg)))
 	{
-	  if (dump_file)
-	    {
-	      fprintf (dump_file, "Cannot eliminate extension:\n");
-	      print_rtl_single (dump_file, insn);
-	      fprintf (dump_file, " because it can operate on uninitialized"
-			          " data\n");
-	    }
+	  ext_cand e = {expr, code, mode, insn};
+	  insn_list->safe_push (e);
 	  return;
 	}
 
+       if ((code == ZERO_EXTEND
+	    && !bitmap_bit_p (init_regs, REGNO (reg))))
+	  {
+	    if (dump_file)
+	      {
+		fprintf (dump_file, "Cannot eliminate extension:\n");
+		print_rtl_single (dump_file, insn);
+		fprintf (dump_file, " because it can operate on uninitialized"
+				    " data\n");
+	      }
+	    return;
+	  }
+
       /* Second, make sure we can get all the reaching definitions.  */
-      defs = get_defs (insn, reg, NULL);
       if (!defs)
 	{
 	  if (dump_file)
@@ -1321,7 +1458,8 @@ find_and_remove_re (void)
 	      && (REGNO (SET_DEST (set)) != REGNO (XEXP (SET_SRC (set), 0))))
 	    {
               reinsn_copy_list.safe_push (curr_cand->insn);
-              reinsn_copy_list.safe_push (state.defs_list[0]);
+	      if (state.defs_list.length() != 0)
+		reinsn_copy_list.safe_push (state.defs_list[0]);
 	    }
 	  reinsn_del_list.safe_push (curr_cand->insn);
 	  state.modified[INSN_UID (curr_cand->insn)].deleted = 1;
@@ -1342,24 +1480,27 @@ find_and_remove_re (void)
      Remember that the memory reference will be changed to refer to the
      destination of the extention.  So we're actually emitting a copy
      from the new destination to the old destination.  */
-  for (unsigned int i = 0; i < reinsn_copy_list.length (); i += 2)
-    {
-      rtx_insn *curr_insn = reinsn_copy_list[i];
-      rtx_insn *def_insn = reinsn_copy_list[i + 1];
-
-      /* Use the mode of the destination of the defining insn
-	 for the mode of the copy.  This is necessary if the
-	 defining insn was used to eliminate a second extension
-	 that was wider than the first.  */
-      rtx sub_rtx = *get_sub_rtx (def_insn);
-      rtx set = single_set (curr_insn);
-      rtx new_dst = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
-				 REGNO (XEXP (SET_SRC (set), 0)));
-      rtx new_src = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
-				 REGNO (SET_DEST (set)));
-      rtx new_set = gen_rtx_SET (new_dst, new_src);
-      emit_insn_after (new_set, def_insn);
-    }
+   for (unsigned int i = 0; i < reinsn_copy_list.length (); i += 2)
+     {
+       rtx_insn *curr_insn = reinsn_copy_list[i];
+
+       if ((i+1) < reinsn_copy_list.length())
+	 {
+	   rtx_insn *def_insn = reinsn_copy_list[i + 1];
+	   /* Use the mode of the destination of the defining insn
+	      for the mode of the copy.  This is necessary if the
+	      defining insn was used to eliminate a second extension
+	      that was wider than the first.  */
+	   rtx sub_rtx = *get_sub_rtx (def_insn);
+	   rtx set = single_set (curr_insn);
+	   rtx new_dst = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
+				      REGNO (XEXP (SET_SRC (set), 0)));
+	   rtx new_src = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
+				      REGNO (SET_DEST (set)));
+	   rtx new_set = gen_rtx_SET (new_dst, new_src);
+	   emit_insn_after (new_set, def_insn);
+	}
+      }
 
   /* Delete all useless extensions here in one sweep.  */
   FOR_EACH_VEC_ELT (reinsn_del_list, i, curr_insn)
diff --git a/gcc/testsuite/g++.target/powerpc/zext-elim-3.C b/gcc/testsuite/g++.target/powerpc/zext-elim-3.C
new file mode 100644
index 00000000000..5a050df06ff
--- /dev/null
+++ b/gcc/testsuite/g++.target/powerpc/zext-elim-3.C
@@ -0,0 +1,13 @@
+/* { dg-options "-mcpu=power9 -O2" } */
+
+void *memset(void *b, int c, unsigned long len)
+{
+  unsigned long i;
+
+  for (i = 0; i < len; i++)
+    ((unsigned char *)b)[i] = c;
+
+   return b;
+}
+
+/* { dg-final { scan-assembler-not "\mrlwinm\M" } } */
-- 
2.39.1


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

* [PING^4] PATCH v5 4/4] ree: Improve ree pass for rs6000 target using defined ABI interfaces.
  2023-08-01  8:18   ` [PING^3] " Ajit Agarwal
@ 2023-08-21  6:46     ` Ajit Agarwal
  2023-09-12  7:21       ` [PING^5] " Ajit Agarwal
  0 siblings, 1 reply; 9+ messages in thread
From: Ajit Agarwal @ 2023-08-21  6:46 UTC (permalink / raw)
  To: gcc-patches
  Cc: Jeff Law, Richard Biener, Segher Boessenkool, Peter Bergner,
	Rashmi.Sridhar


Ping!

-------- Forwarded Message --------
Subject: [PING^3] PATCH v5 4/4] ree: Improve ree pass for rs6000 target using defined ABI interfaces.
Date: Tue, 1 Aug 2023 13:48:58 +0530
From: Ajit Agarwal <aagarwa1@linux.ibm.com>
To: gcc-patches <gcc-patches@gcc.gnu.org>, Jeff Law <jeffreyalaw@gmail.com>, Richard Biener <richard.guenther@gmail.com>, Peter Bergner <bergner@linux.ibm.com>, Segher Boessenkool <segher@kernel.crashing.org>, Rashmi.Sridhar@ibm.com

Ping!


-------- Forwarded Message --------
Subject: [PING^2] PATCH v5 4/4] ree: Improve ree pass for rs6000 target using defined ABI interfaces.
Date: Tue, 18 Jul 2023 13:28:08 +0530
From: Ajit Agarwal <aagarwa1@linux.ibm.com>
To: gcc-patches <gcc-patches@gcc.gnu.org>
CC: Jeff Law <jeffreyalaw@gmail.com>, Richard Biener <richard.guenther@gmail.com>, Segher Boessenkool <segher@kernel.crashing.org>, Peter Bergner <bergner@linux.ibm.com>


Ping^2.

Please review.

Thanks & Regards
Ajit


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

Review comments incorporated.

Thanks & Regards
Ajit

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 defined
ABI interfaces.

2023-06-01  Ajit Kumar Agarwal  <aagarwa1@linux.ibm.com>

gcc/ChangeLog:

	* ree.cc (combine_reaching_defs): Use of  zero_extend and sign_extend
	defined abi interfaces.
	(add_removable_extension): Use of defined abi interfaces for no
	reaching defs.
	(abi_extension_candidate_return_reg_p): New function.
	(abi_extension_candidate_p): New function.
	(abi_extension_candidate_argno_p): New function.
	(abi_handle_regs_without_defs_p): New function.
	(abi_target_promote_function_mode): New function.

gcc/testsuite/ChangeLog:

        * g++.target/powerpc/zext-elim-3.C
---
 gcc/ree.cc                                    | 199 +++++++++++++++---
 .../g++.target/powerpc/zext-elim-3.C          |  13 ++
 2 files changed, 183 insertions(+), 29 deletions(-)
 create mode 100644 gcc/testsuite/g++.target/powerpc/zext-elim-3.C

diff --git a/gcc/ree.cc b/gcc/ree.cc
index fc04249fa84..2025a7c43da 100644
--- a/gcc/ree.cc
+++ b/gcc/ree.cc
@@ -514,7 +514,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 +751,120 @@ get_extended_src_reg (rtx src)
   return src;
 }
 
+/* Return TRUE if target mode is equal to source mode of zero_extend
+   or sign_extend otherwise false.  */
+
+static bool
+abi_target_promote_function_mode (machine_mode mode)
+{
+  int unsignedp;
+  machine_mode tgt_mode =
+    targetm.calls.promote_function_mode (NULL_TREE, mode, &unsignedp,
+					 NULL_TREE, 1);
+
+  if (tgt_mode == mode)
+    return true;
+  else
+    return false;
+}
+
+/* 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;
+}
+
+/* Return TRUE if the candidate insn doesn't have defs and have
+ * uses without RTX_BIN_ARITH/RTX_COMM_ARITH/RTX_UNARY rtx class.  */
+
+static bool
+abi_handle_regs_without_defs_p (rtx_insn *insn)
+{
+  if (side_effects_p (PATTERN (insn)))
+    return false;
+
+  struct df_link *uses
+    = get_uses (insn, SET_DEST (PATTERN (insn)));
+
+  if (!uses)
+    return false;
+
+  for (df_link *use = uses; use; use = use->next)
+    {
+      if (!use->ref)
+	return false;
+
+      if (BLOCK_FOR_INSN (insn)
+	  != BLOCK_FOR_INSN (DF_REF_INSN (use->ref)))
+	return false;
+
+      rtx_insn *use_insn = DF_REF_INSN (use->ref);
+
+      if (GET_CODE (PATTERN (use_insn)) == SET)
+	{
+	  rtx_code code = GET_CODE (SET_SRC (PATTERN (use_insn)));
+
+	  if (GET_RTX_CLASS (code) == RTX_BIN_ARITH
+	      || GET_RTX_CLASS (code) == RTX_COMM_ARITH
+	      || GET_RTX_CLASS (code) == RTX_UNARY)
+	    return false;
+	}
+     }
+  return true;
+}
+
 /* This function goes through all reaching defs of the source
    of the candidate for elimination (CAND) and tries to combine
    the extension with the definition instruction.  The changes
@@ -770,6 +885,11 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
 
   state->defs_list.truncate (0);
   state->copies_list.truncate (0);
+  rtx orig_src = XEXP (SET_SRC (cand->expr),0);
+
+  if (abi_extension_candidate_p (cand->insn)
+      && (!get_defs (cand->insn, orig_src, NULL)))
+    return abi_handle_regs_without_defs_p (cand->insn);
 
   outcome = make_defs_and_copies_lists (cand->insn, set_pat, state);
 
@@ -1036,6 +1156,15 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
         }
     }
 
+  rtx insn_set = single_set (cand->insn);
+
+  machine_mode mode = (GET_MODE (XEXP (SET_SRC (insn_set), 0)));
+
+  bool promote_p = abi_target_promote_function_mode (mode);
+
+  if (promote_p)
+    return true;
+
   if (merge_successful)
     {
       /* Commit the changes here if possible
@@ -1112,26 +1241,34 @@ add_removable_extension (const_rtx expr, rtx_insn *insn,
       rtx reg = XEXP (src, 0);
       struct df_link *defs, *def;
       ext_cand *cand;
+      defs = get_defs (insn, reg, NULL);
 
       /* Zero-extension of an undefined value is partly defined (it's
 	 completely undefined for sign-extension, though).  So if there exists
 	 a path from the entry to this zero-extension that leaves this register
 	 uninitialized, removing the extension could change the behavior of
 	 correct programs.  So first, check it is not the case.  */
-      if (code == ZERO_EXTEND && !bitmap_bit_p (init_regs, REGNO (reg)))
+      if (!defs && abi_extension_candidate_argno_p (code, REGNO (reg)))
 	{
-	  if (dump_file)
-	    {
-	      fprintf (dump_file, "Cannot eliminate extension:\n");
-	      print_rtl_single (dump_file, insn);
-	      fprintf (dump_file, " because it can operate on uninitialized"
-			          " data\n");
-	    }
+	  ext_cand e = {expr, code, mode, insn};
+	  insn_list->safe_push (e);
 	  return;
 	}
 
+       if ((code == ZERO_EXTEND
+	    && !bitmap_bit_p (init_regs, REGNO (reg))))
+	  {
+	    if (dump_file)
+	      {
+		fprintf (dump_file, "Cannot eliminate extension:\n");
+		print_rtl_single (dump_file, insn);
+		fprintf (dump_file, " because it can operate on uninitialized"
+				    " data\n");
+	      }
+	    return;
+	  }
+
       /* Second, make sure we can get all the reaching definitions.  */
-      defs = get_defs (insn, reg, NULL);
       if (!defs)
 	{
 	  if (dump_file)
@@ -1321,7 +1458,8 @@ find_and_remove_re (void)
 	      && (REGNO (SET_DEST (set)) != REGNO (XEXP (SET_SRC (set), 0))))
 	    {
               reinsn_copy_list.safe_push (curr_cand->insn);
-              reinsn_copy_list.safe_push (state.defs_list[0]);
+	      if (state.defs_list.length() != 0)
+		reinsn_copy_list.safe_push (state.defs_list[0]);
 	    }
 	  reinsn_del_list.safe_push (curr_cand->insn);
 	  state.modified[INSN_UID (curr_cand->insn)].deleted = 1;
@@ -1342,24 +1480,27 @@ find_and_remove_re (void)
      Remember that the memory reference will be changed to refer to the
      destination of the extention.  So we're actually emitting a copy
      from the new destination to the old destination.  */
-  for (unsigned int i = 0; i < reinsn_copy_list.length (); i += 2)
-    {
-      rtx_insn *curr_insn = reinsn_copy_list[i];
-      rtx_insn *def_insn = reinsn_copy_list[i + 1];
-
-      /* Use the mode of the destination of the defining insn
-	 for the mode of the copy.  This is necessary if the
-	 defining insn was used to eliminate a second extension
-	 that was wider than the first.  */
-      rtx sub_rtx = *get_sub_rtx (def_insn);
-      rtx set = single_set (curr_insn);
-      rtx new_dst = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
-				 REGNO (XEXP (SET_SRC (set), 0)));
-      rtx new_src = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
-				 REGNO (SET_DEST (set)));
-      rtx new_set = gen_rtx_SET (new_dst, new_src);
-      emit_insn_after (new_set, def_insn);
-    }
+   for (unsigned int i = 0; i < reinsn_copy_list.length (); i += 2)
+     {
+       rtx_insn *curr_insn = reinsn_copy_list[i];
+
+       if ((i+1) < reinsn_copy_list.length())
+	 {
+	   rtx_insn *def_insn = reinsn_copy_list[i + 1];
+	   /* Use the mode of the destination of the defining insn
+	      for the mode of the copy.  This is necessary if the
+	      defining insn was used to eliminate a second extension
+	      that was wider than the first.  */
+	   rtx sub_rtx = *get_sub_rtx (def_insn);
+	   rtx set = single_set (curr_insn);
+	   rtx new_dst = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
+				      REGNO (XEXP (SET_SRC (set), 0)));
+	   rtx new_src = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
+				      REGNO (SET_DEST (set)));
+	   rtx new_set = gen_rtx_SET (new_dst, new_src);
+	   emit_insn_after (new_set, def_insn);
+	}
+      }
 
   /* Delete all useless extensions here in one sweep.  */
   FOR_EACH_VEC_ELT (reinsn_del_list, i, curr_insn)
diff --git a/gcc/testsuite/g++.target/powerpc/zext-elim-3.C b/gcc/testsuite/g++.target/powerpc/zext-elim-3.C
new file mode 100644
index 00000000000..5a050df06ff
--- /dev/null
+++ b/gcc/testsuite/g++.target/powerpc/zext-elim-3.C
@@ -0,0 +1,13 @@
+/* { dg-options "-mcpu=power9 -O2" } */
+
+void *memset(void *b, int c, unsigned long len)
+{
+  unsigned long i;
+
+  for (i = 0; i < len; i++)
+    ((unsigned char *)b)[i] = c;
+
+   return b;
+}
+
+/* { dg-final { scan-assembler-not "\mrlwinm\M" } } */
-- 
2.39.1


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

* [PING^5] PATCH v5 4/4] ree: Improve ree pass for rs6000 target using defined ABI interfaces.
  2023-08-21  6:46     ` [PING^4] " Ajit Agarwal
@ 2023-09-12  7:21       ` Ajit Agarwal
  0 siblings, 0 replies; 9+ messages in thread
From: Ajit Agarwal @ 2023-09-12  7:21 UTC (permalink / raw)
  To: gcc-patches
  Cc: Jeff Law, Richard Biener, Peter Bergner, Segher Boessenkool,
	Rashmi.Sridhar


Ping!

-------- Forwarded Message --------
Subject: [PING^4] PATCH v5 4/4] ree: Improve ree pass for rs6000 target using defined ABI interfaces.
Date: Mon, 21 Aug 2023 12:16:44 +0530
From: Ajit Agarwal <aagarwa1@linux.ibm.com>
To: gcc-patches <gcc-patches@gcc.gnu.org>
CC: Jeff Law <jeffreyalaw@gmail.com>, Richard Biener <richard.guenther@gmail.com>, Segher Boessenkool <segher@kernel.crashing.org>, Peter Bergner <bergner@linux.ibm.com>, Rashmi.Sridhar@ibm.com


Ping!

-------- Forwarded Message --------
Subject: [PING^3] PATCH v5 4/4] ree: Improve ree pass for rs6000 target using defined ABI interfaces.
Date: Tue, 1 Aug 2023 13:48:58 +0530
From: Ajit Agarwal <aagarwa1@linux.ibm.com>
To: gcc-patches <gcc-patches@gcc.gnu.org>, Jeff Law <jeffreyalaw@gmail.com>, Richard Biener <richard.guenther@gmail.com>, Peter Bergner <bergner@linux.ibm.com>, Segher Boessenkool <segher@kernel.crashing.org>, Rashmi.Sridhar@ibm.com

Ping!


-------- Forwarded Message --------
Subject: [PING^2] PATCH v5 4/4] ree: Improve ree pass for rs6000 target using defined ABI interfaces.
Date: Tue, 18 Jul 2023 13:28:08 +0530
From: Ajit Agarwal <aagarwa1@linux.ibm.com>
To: gcc-patches <gcc-patches@gcc.gnu.org>
CC: Jeff Law <jeffreyalaw@gmail.com>, Richard Biener <richard.guenther@gmail.com>, Segher Boessenkool <segher@kernel.crashing.org>, Peter Bergner <bergner@linux.ibm.com>


Ping^2.

Please review.

Thanks & Regards
Ajit


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

Review comments incorporated.

Thanks & Regards
Ajit

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 defined
ABI interfaces.

2023-06-01  Ajit Kumar Agarwal  <aagarwa1@linux.ibm.com>

gcc/ChangeLog:

	* ree.cc (combine_reaching_defs): Use of  zero_extend and sign_extend
	defined abi interfaces.
	(add_removable_extension): Use of defined abi interfaces for no
	reaching defs.
	(abi_extension_candidate_return_reg_p): New function.
	(abi_extension_candidate_p): New function.
	(abi_extension_candidate_argno_p): New function.
	(abi_handle_regs_without_defs_p): New function.
	(abi_target_promote_function_mode): New function.

gcc/testsuite/ChangeLog:

        * g++.target/powerpc/zext-elim-3.C
---
 gcc/ree.cc                                    | 199 +++++++++++++++---
 .../g++.target/powerpc/zext-elim-3.C          |  13 ++
 2 files changed, 183 insertions(+), 29 deletions(-)
 create mode 100644 gcc/testsuite/g++.target/powerpc/zext-elim-3.C

diff --git a/gcc/ree.cc b/gcc/ree.cc
index fc04249fa84..2025a7c43da 100644
--- a/gcc/ree.cc
+++ b/gcc/ree.cc
@@ -514,7 +514,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 +751,120 @@ get_extended_src_reg (rtx src)
   return src;
 }
 
+/* Return TRUE if target mode is equal to source mode of zero_extend
+   or sign_extend otherwise false.  */
+
+static bool
+abi_target_promote_function_mode (machine_mode mode)
+{
+  int unsignedp;
+  machine_mode tgt_mode =
+    targetm.calls.promote_function_mode (NULL_TREE, mode, &unsignedp,
+					 NULL_TREE, 1);
+
+  if (tgt_mode == mode)
+    return true;
+  else
+    return false;
+}
+
+/* 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;
+}
+
+/* Return TRUE if the candidate insn doesn't have defs and have
+ * uses without RTX_BIN_ARITH/RTX_COMM_ARITH/RTX_UNARY rtx class.  */
+
+static bool
+abi_handle_regs_without_defs_p (rtx_insn *insn)
+{
+  if (side_effects_p (PATTERN (insn)))
+    return false;
+
+  struct df_link *uses
+    = get_uses (insn, SET_DEST (PATTERN (insn)));
+
+  if (!uses)
+    return false;
+
+  for (df_link *use = uses; use; use = use->next)
+    {
+      if (!use->ref)
+	return false;
+
+      if (BLOCK_FOR_INSN (insn)
+	  != BLOCK_FOR_INSN (DF_REF_INSN (use->ref)))
+	return false;
+
+      rtx_insn *use_insn = DF_REF_INSN (use->ref);
+
+      if (GET_CODE (PATTERN (use_insn)) == SET)
+	{
+	  rtx_code code = GET_CODE (SET_SRC (PATTERN (use_insn)));
+
+	  if (GET_RTX_CLASS (code) == RTX_BIN_ARITH
+	      || GET_RTX_CLASS (code) == RTX_COMM_ARITH
+	      || GET_RTX_CLASS (code) == RTX_UNARY)
+	    return false;
+	}
+     }
+  return true;
+}
+
 /* This function goes through all reaching defs of the source
    of the candidate for elimination (CAND) and tries to combine
    the extension with the definition instruction.  The changes
@@ -770,6 +885,11 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
 
   state->defs_list.truncate (0);
   state->copies_list.truncate (0);
+  rtx orig_src = XEXP (SET_SRC (cand->expr),0);
+
+  if (abi_extension_candidate_p (cand->insn)
+      && (!get_defs (cand->insn, orig_src, NULL)))
+    return abi_handle_regs_without_defs_p (cand->insn);
 
   outcome = make_defs_and_copies_lists (cand->insn, set_pat, state);
 
@@ -1036,6 +1156,15 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
         }
     }
 
+  rtx insn_set = single_set (cand->insn);
+
+  machine_mode mode = (GET_MODE (XEXP (SET_SRC (insn_set), 0)));
+
+  bool promote_p = abi_target_promote_function_mode (mode);
+
+  if (promote_p)
+    return true;
+
   if (merge_successful)
     {
       /* Commit the changes here if possible
@@ -1112,26 +1241,34 @@ add_removable_extension (const_rtx expr, rtx_insn *insn,
       rtx reg = XEXP (src, 0);
       struct df_link *defs, *def;
       ext_cand *cand;
+      defs = get_defs (insn, reg, NULL);
 
       /* Zero-extension of an undefined value is partly defined (it's
 	 completely undefined for sign-extension, though).  So if there exists
 	 a path from the entry to this zero-extension that leaves this register
 	 uninitialized, removing the extension could change the behavior of
 	 correct programs.  So first, check it is not the case.  */
-      if (code == ZERO_EXTEND && !bitmap_bit_p (init_regs, REGNO (reg)))
+      if (!defs && abi_extension_candidate_argno_p (code, REGNO (reg)))
 	{
-	  if (dump_file)
-	    {
-	      fprintf (dump_file, "Cannot eliminate extension:\n");
-	      print_rtl_single (dump_file, insn);
-	      fprintf (dump_file, " because it can operate on uninitialized"
-			          " data\n");
-	    }
+	  ext_cand e = {expr, code, mode, insn};
+	  insn_list->safe_push (e);
 	  return;
 	}
 
+       if ((code == ZERO_EXTEND
+	    && !bitmap_bit_p (init_regs, REGNO (reg))))
+	  {
+	    if (dump_file)
+	      {
+		fprintf (dump_file, "Cannot eliminate extension:\n");
+		print_rtl_single (dump_file, insn);
+		fprintf (dump_file, " because it can operate on uninitialized"
+				    " data\n");
+	      }
+	    return;
+	  }
+
       /* Second, make sure we can get all the reaching definitions.  */
-      defs = get_defs (insn, reg, NULL);
       if (!defs)
 	{
 	  if (dump_file)
@@ -1321,7 +1458,8 @@ find_and_remove_re (void)
 	      && (REGNO (SET_DEST (set)) != REGNO (XEXP (SET_SRC (set), 0))))
 	    {
               reinsn_copy_list.safe_push (curr_cand->insn);
-              reinsn_copy_list.safe_push (state.defs_list[0]);
+	      if (state.defs_list.length() != 0)
+		reinsn_copy_list.safe_push (state.defs_list[0]);
 	    }
 	  reinsn_del_list.safe_push (curr_cand->insn);
 	  state.modified[INSN_UID (curr_cand->insn)].deleted = 1;
@@ -1342,24 +1480,27 @@ find_and_remove_re (void)
      Remember that the memory reference will be changed to refer to the
      destination of the extention.  So we're actually emitting a copy
      from the new destination to the old destination.  */
-  for (unsigned int i = 0; i < reinsn_copy_list.length (); i += 2)
-    {
-      rtx_insn *curr_insn = reinsn_copy_list[i];
-      rtx_insn *def_insn = reinsn_copy_list[i + 1];
-
-      /* Use the mode of the destination of the defining insn
-	 for the mode of the copy.  This is necessary if the
-	 defining insn was used to eliminate a second extension
-	 that was wider than the first.  */
-      rtx sub_rtx = *get_sub_rtx (def_insn);
-      rtx set = single_set (curr_insn);
-      rtx new_dst = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
-				 REGNO (XEXP (SET_SRC (set), 0)));
-      rtx new_src = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
-				 REGNO (SET_DEST (set)));
-      rtx new_set = gen_rtx_SET (new_dst, new_src);
-      emit_insn_after (new_set, def_insn);
-    }
+   for (unsigned int i = 0; i < reinsn_copy_list.length (); i += 2)
+     {
+       rtx_insn *curr_insn = reinsn_copy_list[i];
+
+       if ((i+1) < reinsn_copy_list.length())
+	 {
+	   rtx_insn *def_insn = reinsn_copy_list[i + 1];
+	   /* Use the mode of the destination of the defining insn
+	      for the mode of the copy.  This is necessary if the
+	      defining insn was used to eliminate a second extension
+	      that was wider than the first.  */
+	   rtx sub_rtx = *get_sub_rtx (def_insn);
+	   rtx set = single_set (curr_insn);
+	   rtx new_dst = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
+				      REGNO (XEXP (SET_SRC (set), 0)));
+	   rtx new_src = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
+				      REGNO (SET_DEST (set)));
+	   rtx new_set = gen_rtx_SET (new_dst, new_src);
+	   emit_insn_after (new_set, def_insn);
+	}
+      }
 
   /* Delete all useless extensions here in one sweep.  */
   FOR_EACH_VEC_ELT (reinsn_del_list, i, curr_insn)
diff --git a/gcc/testsuite/g++.target/powerpc/zext-elim-3.C b/gcc/testsuite/g++.target/powerpc/zext-elim-3.C
new file mode 100644
index 00000000000..5a050df06ff
--- /dev/null
+++ b/gcc/testsuite/g++.target/powerpc/zext-elim-3.C
@@ -0,0 +1,13 @@
+/* { dg-options "-mcpu=power9 -O2" } */
+
+void *memset(void *b, int c, unsigned long len)
+{
+  unsigned long i;
+
+  for (i = 0; i < len; i++)
+    ((unsigned char *)b)[i] = c;
+
+   return b;
+}
+
+/* { dg-final { scan-assembler-not "\mrlwinm\M" } } */
-- 
2.39.1


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

end of thread, other threads:[~2023-09-12  7:21 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-01  5:23 PATCH v5 4/4] ree: Improve ree pass for rs6000 target using defined ABI interfaces Ajit Agarwal
2023-06-01  6:04 ` Bernhard Reutner-Fischer
2023-06-26 12:36 ` [PING] " Ajit Agarwal
2023-06-26 12:42 ` Ajit Agarwal
2023-06-26 12:45   ` [PING] " Ajit Agarwal
2023-07-18  7:58 ` [PING^2] " Ajit Agarwal
2023-08-01  8:18   ` [PING^3] " Ajit Agarwal
2023-08-21  6:46     ` [PING^4] " Ajit Agarwal
2023-09-12  7:21       ` [PING^5] " Ajit Agarwal

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