public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [patch RFC,PR50038]
@ 2011-10-04 10:16 Ilya Tocar
  2011-10-04 15:48 ` Joseph S. Myers
  0 siblings, 1 reply; 5+ messages in thread
From: Ilya Tocar @ 2011-10-04 10:16 UTC (permalink / raw)
  To: gcc-patches

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

Hi everyone,

This patch fixes PR 50038 (redundant zero extensions) by modifying
implicit-zee pass
to also remove unneeded zero extensions from QImode to SImode.
There is  6% improvement in rgbyiqv test from EEMBC 2.0 benchmark on x86-64.
I am not sure if this is correct approach ( tom modify  implicit-zee
pass), so comments are welcome.
Also if this aproach is correct we will need to enable  implicit-zee
pass on some new targets ( for example x86 32bit).

It passes bootstrap and make-check.

Here is a Changelog:

2011-09-27  Ilya Tocar  <ilya.tocar@intel.com>

    * implicit-zee.c: Added 2011 to copyright.
    (combine_set_zero_extend): Add QImode.
    (merge_def_and_ze): Likewise.
    (add_removable_zero_extend): Likewise.
    (not_qi_to_si): New.
    (make_defs_and_copies_lists): Add check for QImode.

[-- Attachment #2: zee.patch --]
[-- Type: application/octet-stream, Size: 5093 bytes --]

diff --git a/gcc/implicit-zee.c b/gcc/implicit-zee.c
index db42248..1a45275 100644
--- a/gcc/implicit-zee.c
+++ b/gcc/implicit-zee.c
@@ -1,6 +1,6 @@
 /* Redundant Zero-extension elimination for targets that implicitly
    zero-extend writes to the lower 32-bit portion of 64-bit registers.
-   Copyright (C) 2010 Free Software Foundation, Inc.
+   Copyright (C) 2010, 2011 Free Software Foundation, Inc.
    Contributed by Sriraman Tallam (tmsriram@google.com) and
                   Silvius Rus     (rus@google.com)
 
@@ -240,7 +240,7 @@ set_insn_status (rtx insn, enum insn_merge_code code)
 /* Given a insn (CURR_INSN) and a pointer to the SET rtx (ORIG_SET)
    that needs to be modified, this code modifies the SET rtx to a
    new SET rtx that zero_extends the right hand expression into a DImode
-   register (NEWREG) on the left hand side.  Note that multiple
+   or SImode register (NEWREG) on the left hand side. Note that multiple
    assumptions are made about the nature of the set that needs
    to be true for this to work and is called from merge_def_and_ze.
 
@@ -269,7 +269,7 @@ combine_set_zero_extend (rtx curr_insn, rtx *orig_set, rtx newreg)
   /* The right hand side can also be VOIDmode.  These cases have to be
      handled differently.  */
 
-  if (GET_MODE (orig_src) != SImode)
+  if (GET_MODE (orig_src) != SImode && GET_MODE (orig_src) != QImode)
     {
       /* Merge constants by directly moving the constant into the
          DImode register under some conditions.  */
@@ -321,9 +321,19 @@ combine_set_zero_extend (rtx curr_insn, rtx *orig_set, rtx newreg)
 
       return false;
     }
+  else if (GET_MODE (orig_src) == QImode)
+    {
+      /* This is QI case.  */
+
+      temp_extension = gen_rtx_ZERO_EXTEND (SImode, orig_src);
+      simplified_temp_extension = simplify_rtx (temp_extension);
+      if (simplified_temp_extension)
+        temp_extension = simplified_temp_extension;
+      new_set = gen_rtx_SET (VOIDmode, newreg, temp_extension);
+    }
   else
     {
-      /* This is the normal case we expect.  */
+      /* This is DI case.  */
 
       temp_extension = gen_rtx_ZERO_EXTEND (DImode, orig_src);
       simplified_temp_extension = simplify_rtx (temp_extension);
@@ -483,6 +493,40 @@ is_this_a_cmove (rtx expr, void *data)
   return 0;
 }
 
+/* This functions checks that we are trying to merge EXTEND to SImode
+   with TO_MERGE rtx not operating on QImode. Called from 
+   make_defs_and_copies_lists to prevent replacing QImode to SImode extend 
+   with extend to DImode.*/
+
+static int
+not_qi_to_si (rtx extend, rtx to_merge)
+{
+  int i;
+  rtx expr;
+  if (GET_CODE (SET_SRC (extend)) == ZERO_EXTEND
+      && GET_MODE (SET_SRC (extend)) == SImode)
+    {
+      if (GET_CODE (PATTERN (to_merge)) == SET
+          && GET_MODE (SET_SRC(PATTERN (to_merge))) != QImode)
+           return 1;
+      else if (GET_CODE (PATTERN (to_merge)) == PARALLEL)
+        {
+          for (i = 0; i < XVECLEN (PATTERN (to_merge), 0); i++)
+            {
+              expr = XVECEXP (PATTERN (to_merge), 0, i);
+              if (GET_CODE (expr) != SET)
+                continue;
+              if (GET_MODE (expr) != QImode)
+               return 1;
+            }
+           return 0;
+        }
+      else
+       return 0;
+    }
+   return 0;
+}
+
 /* This returns 1 if it found
    (SET (reg:SI REGNO (def_reg)) (if_then_else (cond) (REG:SI x1) (REG:SI x2)))
    in the DEF_INSN pattern.  It stores the x1 and x2 in COPY_REG_1
@@ -569,6 +613,12 @@ make_defs_and_copies_lists (rtx zero_extend_insn, rtx set_pat,
       def_insn = VEC_index (rtx, work_list, vec_index);
       gcc_assert (INSN_UID (def_insn) < max_insn_uid);
 
+      if (not_qi_to_si (set_pat,def_insn))
+        {
+          VEC_free (rtx, heap, work_list);
+          return 0;
+        }
+
       if (is_insn_visited[INSN_UID (def_insn)])
         {
           vec_index++;
@@ -667,6 +717,13 @@ merge_def_and_ze (rtx def_insn)
       setreg = get_reg_di (SET_DEST (*sub_rtx));
       return combine_set_zero_extend (def_insn, sub_rtx, setreg);
     }
+  else if (GET_CODE (SET_DEST (*sub_rtx)) == REG
+           && GET_MODE (SET_DEST (*sub_rtx)) == QImode)
+    {
+      setreg = gen_rtx_REG (SImode, REGNO (SET_DEST (*sub_rtx)));
+      gcc_assert (setreg);
+      return combine_set_zero_extend (def_insn, sub_rtx, setreg);
+    }
   else
     return false;
   return true;
@@ -838,11 +895,11 @@ add_removable_zero_extend (rtx x ATTRIBUTE_UNUSED, const_rtx expr, void *data)
   dest = SET_DEST (expr);
 
   if (REG_P (dest)
-      && GET_MODE (dest) == DImode
       && GET_CODE (src) == ZERO_EXTEND
       && REG_P (XEXP (src, 0))
-      && GET_MODE (XEXP (src, 0)) == SImode
-      && REGNO (dest) == REGNO (XEXP (src, 0)))
+      && REGNO (dest) == REGNO (XEXP (src, 0))
+      && ((GET_MODE (dest) == DImode  && GET_MODE (XEXP (src, 0)) == SImode)
+          || (GET_MODE (dest) == SImode  && GET_MODE (XEXP (src, 0)) == QImode)))
     {
       if (get_defs (zei->insn, XEXP (src, 0), NULL))
 	VEC_safe_push (rtx, heap, zei->insn_list, zei->insn);

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

* Re: [patch RFC,PR50038]
  2011-10-04 10:16 [patch RFC,PR50038] Ilya Tocar
@ 2011-10-04 15:48 ` Joseph S. Myers
  2011-10-04 19:59   ` Richard Henderson
  0 siblings, 1 reply; 5+ messages in thread
From: Joseph S. Myers @ 2011-10-04 15:48 UTC (permalink / raw)
  To: Ilya Tocar; +Cc: gcc-patches

On Tue, 4 Oct 2011, Ilya Tocar wrote:

> Hi everyone,
> 
> This patch fixes PR 50038 (redundant zero extensions) by modifying
> implicit-zee pass
> to also remove unneeded zero extensions from QImode to SImode.

Hardcoding particular modes like this in the target-independent parts of 
the compiler is fundamentally ill-conceived.  Right now it hardcodes the 
(SImode, DImode) pair.  You're adding hardcoding of (QImode, SImode) as 
well.  But really it should consider all pairs of (integer mode, wider 
integer mode), with the machine description (or target hooks) determining 
which pairs are relevant on a particular target.  Changing it not to 
hardcode particular modes would be better than adding a second pair.

-- 
Joseph S. Myers
joseph@codesourcery.com

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

* Re: [patch RFC,PR50038]
  2011-10-04 15:48 ` Joseph S. Myers
@ 2011-10-04 19:59   ` Richard Henderson
  2011-10-11 15:00     ` Ilya Enkovich
  0 siblings, 1 reply; 5+ messages in thread
From: Richard Henderson @ 2011-10-04 19:59 UTC (permalink / raw)
  To: Joseph S. Myers; +Cc: Ilya Tocar, gcc-patches

On 10/04/2011 08:42 AM, Joseph S. Myers wrote:
> On Tue, 4 Oct 2011, Ilya Tocar wrote:
> 
>> Hi everyone,
>>
>> This patch fixes PR 50038 (redundant zero extensions) by modifying
>> implicit-zee pass
>> to also remove unneeded zero extensions from QImode to SImode.
> 
> Hardcoding particular modes like this in the target-independent parts of 
> the compiler is fundamentally ill-conceived.  Right now it hardcodes the 
> (SImode, DImode) pair.  You're adding hardcoding of (QImode, SImode) as 
> well.  But really it should consider all pairs of (integer mode, wider 
> integer mode), with the machine description (or target hooks) determining 
> which pairs are relevant on a particular target.  Changing it not to 
> hardcode particular modes would be better than adding a second pair.
> 

That along with not hard-coding ZERO_EXTEND.

Both MIPS and Alpha have much the same free operations, but with SIGN_EXTEND.

I remember rejecting one iteration of this pass with this hard-coded, but the
pass was apparently approved by someone else without that being corrected.


r~

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

* Re: [patch RFC,PR50038]
  2011-10-04 19:59   ` Richard Henderson
@ 2011-10-11 15:00     ` Ilya Enkovich
  2011-10-17 11:16       ` Ilya Enkovich
  0 siblings, 1 reply; 5+ messages in thread
From: Ilya Enkovich @ 2011-10-11 15:00 UTC (permalink / raw)
  To: Richard Henderson, Joseph S. Myers, Ilya Tocar; +Cc: gcc-patches

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

2011/10/4 Richard Henderson <rth@redhat.com>:
> On 10/04/2011 08:42 AM, Joseph S. Myers wrote:
>> On Tue, 4 Oct 2011, Ilya Tocar wrote:
>>
>>> Hi everyone,
>>>
>>> This patch fixes PR 50038 (redundant zero extensions) by modifying
>>> implicit-zee pass
>>> to also remove unneeded zero extensions from QImode to SImode.
>>
>> Hardcoding particular modes like this in the target-independent parts of
>> the compiler is fundamentally ill-conceived.  Right now it hardcodes the
>> (SImode, DImode) pair.  You're adding hardcoding of (QImode, SImode) as
>> well.  But really it should consider all pairs of (integer mode, wider
>> integer mode), with the machine description (or target hooks) determining
>> which pairs are relevant on a particular target.  Changing it not to
>> hardcode particular modes would be better than adding a second pair.
>>
>
> That along with not hard-coding ZERO_EXTEND.
>
> Both MIPS and Alpha have much the same free operations, but with SIGN_EXTEND.
>
> I remember rejecting one iteration of this pass with this hard-coded, but the
> pass was apparently approved by someone else without that being corrected.
>
>
> r~
>

Hello guys,

Could you please look at my patch version? I tried to remove all
unnecessary mode restrictions and cover SIGN_EXTEND case. I did not
test this patch yet, just checked it worked on reproducer from
PR50038.

Thanks
Ilya
---
gcc/

	* implicit-zee.c (ext_cand): New.
	(ext_cand_pool): Likewise.
	(add_ext_candidate): Likewise.
	(zee_init): Likewise.
	(zee_cleanup): Likewise.
	(combine_set_zero_extend): Get extend candidate as new parameter.
	Now handle sign extend cases and all modes.
	(transform_ifelse): Likewise.
	(merge_def_and_ze): Likewise.
	(combine_reaching_defs): Change parameter type.
	(zero_extend_info): Changed insn_list type.
	(add_removable_zero_extend): Relaxed mode and code filter.
	(find_removable_zero_extends): Changed return type.
	(find_and_remove_ze): Var type changes.
	(rest_of_handle_zee): Initialization and cleanup added.

[-- Attachment #2: PR50038.diff --]
[-- Type: application/octet-stream, Size: 12943 bytes --]

diff --git a/gcc/implicit-zee.c b/gcc/implicit-zee.c
index db42248..dc8aff2 100644
--- a/gcc/implicit-zee.c
+++ b/gcc/implicit-zee.c
@@ -213,6 +213,15 @@ enum insn_merge_code
   MERGE_SUCCESS
 };
 
+typedef struct ext_cand
+{
+  rtx insn;
+  const_rtx ext_expr;
+  enum machine_mode src_mode;
+} *ext_cand_t;
+
+static alloc_pool ext_cand_pool;
+
 /* This says if a INSN UID or its definition has already been merged
    with a zero-extend or not.  */
 
@@ -255,60 +264,63 @@ set_insn_status (rtx insn, enum insn_merge_code code)
    assign it to the DI mode register.  */
 
 static bool
-combine_set_zero_extend (rtx curr_insn, rtx *orig_set, rtx newreg)
+combine_set_zero_extend (ext_cand_t cand, rtx curr_insn, rtx *orig_set)
 {
   rtx temp_extension, simplified_temp_extension, new_set, new_const_int;
-  rtx orig_src;
+  rtx orig_src, cand_src;
+  rtx newreg;
   HOST_WIDE_INT val;
   unsigned int mask, delta_width;
+  enum machine_mode dst_mode = GET_MODE (SET_DEST (cand->ext_expr));
 
   /* Change the SET rtx and validate it.  */
   orig_src = SET_SRC (*orig_set);
+  cand_src = SET_SRC (cand->ext_expr);
   new_set = NULL_RTX;
 
-  /* The right hand side can also be VOIDmode.  These cases have to be
-     handled differently.  */
+  newreg = gen_rtx_REG (dst_mode, REGNO (SET_DEST (*orig_set)));
 
-  if (GET_MODE (orig_src) != SImode)
-    {
-      /* Merge constants by directly moving the constant into the
-         DImode register under some conditions.  */
+  /* Merge constants by directly moving the constant into the
+     DImode register under some conditions.  */
 
-      if (GET_CODE (orig_src) == CONST_INT
-	  && HOST_BITS_PER_WIDE_INT >= GET_MODE_BITSIZE (SImode))
-        {
-          if (INTVAL (orig_src) >= 0)
-            new_set = gen_rtx_SET (VOIDmode, newreg, orig_src);
-          else if (INTVAL (orig_src) < 0)
-            {
-              /* Zero-extending a negative SImode integer into DImode
-                 makes it a positive integer.  Convert the given negative
-                 integer into the appropriate integer when zero-extended.  */
-
-              delta_width = HOST_BITS_PER_WIDE_INT - GET_MODE_BITSIZE (SImode);
-              mask = (~(unsigned HOST_WIDE_INT) 0) >> delta_width;
-              val = INTVAL (orig_src);
-              val = val & mask;
-              new_const_int = gen_rtx_CONST_INT (VOIDmode, val);
-              new_set = gen_rtx_SET (VOIDmode, newreg, new_const_int);
-            }
-          else
-            return false;
-        }
+  if (GET_CODE (orig_src) == CONST_INT
+      && HOST_BITS_PER_WIDE_INT >= GET_MODE_BITSIZE (dst_mode))
+    {
+      if (INTVAL (orig_src) >= 0)
+	new_set = gen_rtx_SET (VOIDmode, newreg, orig_src);
+      else if (INTVAL (orig_src) < 0)
+	{
+	  /* Zero-extending a negative SImode integer into DImode
+	     makes it a positive integer.  Convert the given negative
+	     integer into the appropriate integer when zero-extended.  */
+
+	  delta_width = HOST_BITS_PER_WIDE_INT - GET_MODE_BITSIZE (SImode);
+	  mask = (~(unsigned HOST_WIDE_INT) 0) >> delta_width;
+	  val = INTVAL (orig_src);
+	  val = val & mask;
+	  new_const_int = gen_rtx_CONST_INT (VOIDmode, val);
+	  new_set = gen_rtx_SET (VOIDmode, newreg, new_const_int);
+	}
       else
-        {
-          /* This is mostly due to a call insn that should not be
-             optimized.  */
+	return false;
+    }
+  else if (GET_MODE (orig_src) == VOIDmode)
+    {
+      /* This is mostly due to a call insn that should not be
+	 optimized.  */
 
-          return false;
-        }
+      return false;
     }
-  else if (GET_CODE (orig_src) == ZERO_EXTEND)
+  else if (GET_CODE (orig_src) == GET_CODE (cand_src))
     {
       /* Here a zero-extend is used to get to SI. Why not make it
          all the  way till DI.  */
 
-      temp_extension = gen_rtx_ZERO_EXTEND (DImode, XEXP (orig_src, 0));
+      if (GET_CODE (orig_src) == ZERO_EXTEND)
+	temp_extension = gen_rtx_ZERO_EXTEND (dst_mode, XEXP (orig_src, 0));
+      else
+	temp_extension = gen_rtx_SIGN_EXTEND (dst_mode, XEXP (orig_src, 0));
+
       simplified_temp_extension = simplify_rtx (temp_extension);
       if (simplified_temp_extension)
         temp_extension = simplified_temp_extension;
@@ -325,7 +337,11 @@ combine_set_zero_extend (rtx curr_insn, rtx *orig_set, rtx newreg)
     {
       /* This is the normal case we expect.  */
 
-      temp_extension = gen_rtx_ZERO_EXTEND (DImode, orig_src);
+      if (GET_CODE (cand_src) == ZERO_EXTEND)
+	temp_extension = gen_rtx_ZERO_EXTEND (dst_mode, orig_src);
+      else
+	temp_extension = gen_rtx_SIGN_EXTEND (dst_mode, orig_src);
+
       simplified_temp_extension = simplify_rtx (temp_extension);
       if (simplified_temp_extension)
         temp_extension = simplified_temp_extension;
@@ -349,18 +365,6 @@ combine_set_zero_extend (rtx curr_insn, rtx *orig_set, rtx newreg)
   return false;
 }
 
-/* This returns the DI mode for the SI register REG_SI.  */
-
-static rtx
-get_reg_di (rtx reg_si)
-{
-  rtx newreg;
-
-  newreg = gen_rtx_REG (DImode, REGNO (reg_si));
-  gcc_assert (newreg);
-  return newreg;
-}
-
 /* Treat if_then_else insns, where the operands of both branches
    are registers, as copies. For instance,
    Original :
@@ -370,7 +374,7 @@ get_reg_di (rtx reg_si)
    DEF_INSN is the if_then_else insn.  */
 
 static bool
-transform_ifelse (rtx def_insn)
+transform_ifelse (ext_cand_t cand, rtx def_insn)
 {
   rtx set_insn = PATTERN (def_insn);
   rtx srcreg, dstreg, srcreg2;
@@ -378,16 +382,17 @@ transform_ifelse (rtx def_insn)
   rtx ifexpr;
   rtx cond;
   rtx new_set;
+  enum machine_mode dst_mode = GET_MODE (SET_DEST (cand->ext_expr));
 
   gcc_assert (GET_CODE (set_insn) == SET);
   cond = XEXP (SET_SRC (set_insn), 0);
   dstreg = SET_DEST (set_insn);
   srcreg = XEXP (SET_SRC (set_insn), 1);
   srcreg2 = XEXP (SET_SRC (set_insn), 2);
-  map_srcreg = get_reg_di (srcreg);
-  map_srcreg2 = get_reg_di (srcreg2);
-  map_dstreg = get_reg_di (dstreg);
-  ifexpr = gen_rtx_IF_THEN_ELSE (DImode, cond, map_srcreg, map_srcreg2);
+  map_srcreg = gen_rtx_REG (dst_mode, REGNO (srcreg));
+  map_srcreg2 = gen_rtx_REG (dst_mode, REGNO (srcreg2));
+  map_dstreg = gen_rtx_REG (dst_mode, REGNO (dstreg));
+  ifexpr = gen_rtx_IF_THEN_ELSE (dst_mode, cond, map_srcreg, map_srcreg2);
   new_set = gen_rtx_SET (VOIDmode, map_dstreg, ifexpr);
 
   if (validate_change (def_insn, &PATTERN (def_insn), new_set, true))
@@ -623,14 +628,15 @@ make_defs_and_copies_lists (rtx zero_extend_insn, rtx set_pat,
    on the SET pattern.  */
 
 static bool
-merge_def_and_ze (rtx def_insn)
+merge_def_and_ze (ext_cand_t cand, rtx def_insn)
 {
+  enum machine_mode ext_src_mode;
   enum rtx_code code;
-  rtx setreg;
   rtx *sub_rtx;
   rtx s_expr;
   int i;
 
+  ext_src_mode = GET_MODE (XEXP (SET_SRC (cand->ext_expr), 0));
   code = GET_CODE (PATTERN (def_insn));
   sub_rtx = NULL;
 
@@ -662,10 +668,9 @@ merge_def_and_ze (rtx def_insn)
   gcc_assert (sub_rtx != NULL);
 
   if (GET_CODE (SET_DEST (*sub_rtx)) == REG
-      && GET_MODE (SET_DEST (*sub_rtx)) == SImode)
+      && GET_MODE (SET_DEST (*sub_rtx)) == ext_src_mode)
     {
-      setreg = get_reg_di (SET_DEST (*sub_rtx));
-      return combine_set_zero_extend (def_insn, sub_rtx, setreg);
+      return combine_set_zero_extend (cand, def_insn, sub_rtx);
     }
   else
     return false;
@@ -682,7 +687,7 @@ merge_def_and_ze (rtx def_insn)
    true upon success and false upon failure.  */
 
 static bool
-combine_reaching_defs (rtx zero_extend_insn, rtx set_pat)
+combine_reaching_defs (ext_cand_t cand, rtx set_pat)
 {
   rtx def_insn;
   bool merge_successful = true;
@@ -698,7 +703,7 @@ combine_reaching_defs (rtx zero_extend_insn, rtx set_pat)
   defs_list = VEC_alloc (rtx, heap, 8);
   copies_list = VEC_alloc (rtx, heap, 8);
 
-  outcome = make_defs_and_copies_lists (zero_extend_insn,
+  outcome = make_defs_and_copies_lists (cand->insn,
                                         set_pat, &defs_list, &copies_list);
 
   /* outcome == 2 implies that all the definitions for this zero_extend were
@@ -731,7 +736,7 @@ combine_reaching_defs (rtx zero_extend_insn, rtx set_pat)
       merge_code = get_insn_status (def_insn);
       gcc_assert (merge_code == MERGE_NOT_ATTEMPTED);
 
-      if (merge_def_and_ze (def_insn))
+      if (merge_def_and_ze (cand, def_insn))
         VEC_safe_push (rtx, heap, vec, def_insn);
       else
         {
@@ -747,7 +752,7 @@ combine_reaching_defs (rtx zero_extend_insn, rtx set_pat)
     {
       FOR_EACH_VEC_ELT (rtx, copies_list, i, def_insn)
         {
-          if (transform_ifelse (def_insn))
+          if (transform_ifelse (cand, def_insn))
             {
               VEC_safe_push (rtx, heap, vec, def_insn);
             }
@@ -812,15 +817,29 @@ combine_reaching_defs (rtx zero_extend_insn, rtx set_pat)
 
 /* Carry information about zero-extensions while walking the RTL.  */
 
+DEF_VEC_P(ext_cand_t);
+DEF_VEC_ALLOC_P(ext_cand_t, heap);
+
 struct zero_extend_info
 {
   /* The insn where the zero-extension is.  */
   rtx insn;
 
   /* The list of candidates.  */
-  VEC (rtx, heap) *insn_list;
+  VEC (ext_cand_t, heap) *insn_list;
 };
 
+static void
+add_ext_candidate (VEC(ext_cand_t, heap) **exts,
+		   rtx insn, const_rtx expr)
+{
+  ext_cand_t ec = (ext_cand_t) pool_alloc (ext_cand_pool);
+
+  ec->insn = insn;
+  ec->ext_expr = expr;
+  VEC_safe_push (ext_cand_t, heap, *exts, ec);
+}
+
 /* Add a zero-extend pattern that could be eliminated.  This is called via
    note_stores from find_removable_zero_extends.  */
 
@@ -838,14 +857,12 @@ add_removable_zero_extend (rtx x ATTRIBUTE_UNUSED, const_rtx expr, void *data)
   dest = SET_DEST (expr);
 
   if (REG_P (dest)
-      && GET_MODE (dest) == DImode
-      && GET_CODE (src) == ZERO_EXTEND
       && REG_P (XEXP (src, 0))
-      && GET_MODE (XEXP (src, 0)) == SImode
+      && (GET_CODE (src) == ZERO_EXTEND || GET_CODE (src) == SIGN_EXTEND)
       && REGNO (dest) == REGNO (XEXP (src, 0)))
     {
       if (get_defs (zei->insn, XEXP (src, 0), NULL))
-	VEC_safe_push (rtx, heap, zei->insn_list, zei->insn);
+	add_ext_candidate (&zei->insn_list, zei->insn, expr);
       else if (dump_file)
 	{
 	  fprintf (dump_file, "Cannot eliminate zero-extension: \n");
@@ -858,14 +875,14 @@ add_removable_zero_extend (rtx x ATTRIBUTE_UNUSED, const_rtx expr, void *data)
 /* Traverse the instruction stream looking for zero-extends and return the
    list of candidates.  */
 
-static VEC (rtx,heap)*
+static VEC (ext_cand_t,heap)*
 find_removable_zero_extends (void)
 {
   struct zero_extend_info zei;
   basic_block bb;
   rtx insn;
 
-  zei.insn_list = VEC_alloc (rtx, heap, 8);
+  zei.insn_list = VEC_alloc (ext_cand_t, heap, 8);
 
   FOR_EACH_BB (bb)
     FOR_BB_INSNS (bb, insn)
@@ -886,12 +903,13 @@ find_removable_zero_extends (void)
 static unsigned int
 find_and_remove_ze (void)
 {
+  ext_cand_t curr_cand = NULL;
   rtx curr_insn = NULL_RTX;
   int i;
   int ix;
   long long num_realized = 0;
   long long num_ze_opportunities = 0;
-  VEC (rtx, heap) *zeinsn_list;
+  VEC (ext_cand_t, heap) *zeinsn_list;
   VEC (rtx, heap) *zeinsn_del_list;
 
   /* Construct DU chain to get all reaching definitions of each
@@ -915,7 +933,7 @@ find_and_remove_ze (void)
 
   zeinsn_list = find_removable_zero_extends ();
 
-  FOR_EACH_VEC_ELT (rtx, zeinsn_list, ix, curr_insn)
+  FOR_EACH_VEC_ELT (ext_cand_t, zeinsn_list, ix, curr_cand)
     {
       num_ze_opportunities++;
       /* Try to combine the zero-extends with the definition here.  */
@@ -926,12 +944,12 @@ find_and_remove_ze (void)
           print_rtl_single (dump_file, curr_insn);
         }
 
-      if (combine_reaching_defs (curr_insn, PATTERN (curr_insn)))
+      if (combine_reaching_defs (curr_cand, PATTERN (curr_cand->insn)))
         {
           if (dump_file)
             fprintf (dump_file, "Eliminated the zero extension...\n");
           num_realized++;
-          VEC_safe_push (rtx, heap, zeinsn_del_list, curr_insn);
+          VEC_safe_push (rtx, heap, zeinsn_del_list, curr_cand->insn);
         }
     }
 
@@ -940,7 +958,7 @@ find_and_remove_ze (void)
     delete_insn (curr_insn);
 
   free (is_insn_merge_attempted);
-  VEC_free (rtx, heap, zeinsn_list);
+  VEC_free (ext_cand_t, heap, zeinsn_list);
   VEC_free (rtx, heap, zeinsn_del_list);
 
   if (dump_file && num_ze_opportunities > 0)
@@ -953,13 +971,32 @@ find_and_remove_ze (void)
   return 0;
 }
 
+/* Initialize the pass.  */
+
+static void
+zee_init ()
+{
+  ext_cand_pool = create_alloc_pool ("extend candidate pool",
+				     sizeof (struct ext_cand), 30);
+}
+
+/* Cleanup after the pass.  */
+
+static void
+zee_cleanup ()
+{
+  free_alloc_pool (ext_cand_pool);
+}
+
 /* Find and remove redundant zero extensions.  */
 
 static unsigned int
 rest_of_handle_zee (void)
 {
   timevar_push (TV_ZEE);
+  zee_init();
   find_and_remove_ze ();
+  zee_cleanup();
   timevar_pop (TV_ZEE);
   return 0;
 }

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

* Re: [patch RFC,PR50038]
  2011-10-11 15:00     ` Ilya Enkovich
@ 2011-10-17 11:16       ` Ilya Enkovich
  0 siblings, 0 replies; 5+ messages in thread
From: Ilya Enkovich @ 2011-10-17 11:16 UTC (permalink / raw)
  To: Richard Henderson, Joseph S. Myers, Ilya Tocar; +Cc: gcc-patches

Ping.

Could please someone check if my approach is OK and it is worth to
continue work on patch for PR50038?

Thanks
Ilya

2011/10/11 Ilya Enkovich <enkovich.gnu@gmail.com>:
> 2011/10/4 Richard Henderson <rth@redhat.com>:
>> On 10/04/2011 08:42 AM, Joseph S. Myers wrote:
>>> On Tue, 4 Oct 2011, Ilya Tocar wrote:
>>>
>>>> Hi everyone,
>>>>
>>>> This patch fixes PR 50038 (redundant zero extensions) by modifying
>>>> implicit-zee pass
>>>> to also remove unneeded zero extensions from QImode to SImode.
>>>
>>> Hardcoding particular modes like this in the target-independent parts of
>>> the compiler is fundamentally ill-conceived.  Right now it hardcodes the
>>> (SImode, DImode) pair.  You're adding hardcoding of (QImode, SImode) as
>>> well.  But really it should consider all pairs of (integer mode, wider
>>> integer mode), with the machine description (or target hooks) determining
>>> which pairs are relevant on a particular target.  Changing it not to
>>> hardcode particular modes would be better than adding a second pair.
>>>
>>
>> That along with not hard-coding ZERO_EXTEND.
>>
>> Both MIPS and Alpha have much the same free operations, but with SIGN_EXTEND.
>>
>> I remember rejecting one iteration of this pass with this hard-coded, but the
>> pass was apparently approved by someone else without that being corrected.
>>
>>
>> r~
>>
>
> Hello guys,
>
> Could you please look at my patch version? I tried to remove all
> unnecessary mode restrictions and cover SIGN_EXTEND case. I did not
> test this patch yet, just checked it worked on reproducer from
> PR50038.
>
> Thanks
> Ilya
> ---
> gcc/
>
>        * implicit-zee.c (ext_cand): New.
>        (ext_cand_pool): Likewise.
>        (add_ext_candidate): Likewise.
>        (zee_init): Likewise.
>        (zee_cleanup): Likewise.
>        (combine_set_zero_extend): Get extend candidate as new parameter.
>        Now handle sign extend cases and all modes.
>        (transform_ifelse): Likewise.
>        (merge_def_and_ze): Likewise.
>        (combine_reaching_defs): Change parameter type.
>        (zero_extend_info): Changed insn_list type.
>        (add_removable_zero_extend): Relaxed mode and code filter.
>        (find_removable_zero_extends): Changed return type.
>        (find_and_remove_ze): Var type changes.
>        (rest_of_handle_zee): Initialization and cleanup added.
>

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

end of thread, other threads:[~2011-10-17 11:06 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-10-04 10:16 [patch RFC,PR50038] Ilya Tocar
2011-10-04 15:48 ` Joseph S. Myers
2011-10-04 19:59   ` Richard Henderson
2011-10-11 15:00     ` Ilya Enkovich
2011-10-17 11:16       ` Ilya Enkovich

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