This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: [PATCH GCC]Catch more MEM_REFs sharing common addressing part in gimple strength reduction


Thanks for reviewing, I will correct all stupid spelling problem in the next version of patch.

On Mon, Sep 9, 2013 at 8:15 AM, Bill Schmidt <wschmidt@linux.vnet.ibm.com> wrote:
>
>>>+   int (i * S).
>>>+   Otherwise, just return double int zero.  */
>
> This is sufficient, since you are properly checking the next_interp
> chain.  Another possible form would be
>
>     X = (B + i) * 1,
>
> but if this is present, then one of the forms you're checking for should
> also be present, so there's no need to check the MULT_CANDs.
I'm not very sure here since I didn't check MULT_CAND in the patch.  Could you please explain more about this?

>
>>>+
>>>+static double_int
>>>+backtrace_base_for_ref (tree *pbase)
>>>+{
>>>+  tree base_in = *pbase;
>>>+  slsr_cand_t base_cand;
>>>+
>>>+  STRIP_NOPS (base_in);
>>>+  if (TREE_CODE (base_in) != SSA_NAME)
>>>+    return tree_to_double_int (integer_zero_node);
>>>+
>>>+  base_cand = base_cand_from_table (base_in);
>>>+
>>>+  while (base_cand && base_cand->kind != CAND_PHI)
>>>+    {
>>>+      if (base_cand->kind == CAND_ADD
>>>+       && base_cand->index.is_one ()
>>>+       && TREE_CODE (base_cand->stride) == INTEGER_CST)
>>>+     {
>>>+       /* X = B + (1 * S), S is integer constant.  */
>>>+       *pbase = base_cand->base_expr;
>>>+       return tree_to_double_int (base_cand->stride);
>>>+     }
>>>+      else if (base_cand->kind == CAND_ADD
>>>+            && TREE_CODE (base_cand->stride) == INTEGER_CST
>>>+            && integer_onep (base_cand->stride))
>>>+        {
>>>+       /* X = B + (i * S), S is integer one.  */
>>>+       *pbase = base_cand->base_expr;
>>>+       return base_cand->index;
>>>+     }
>>>+
>>>+      if (base_cand->next_interp)
>>>+     base_cand = lookup_cand (base_cand->next_interp);
>>>+      else
>>>+     base_cand = NULL;
>>>+    }
>>>+
>>>+  return tree_to_double_int (integer_zero_node);
>>>+}
>>>+
>>> /* Look for the following pattern:
>>>
>>>     *PBASE:    MEM_REF (T1, C1)
>>>@@ -767,8 +818,15 @@ slsr_process_phi (gimple phi, bool speed)
>>>
>>>     *PBASE:    T1
>>>     *POFFSET:  MULT_EXPR (T2, C3)
>>>-    *PINDEX:   C1 + (C2 * C3) + C4  */
>>>+    *PINDEX:   C1 + (C2 * C3) + C4
>>>
>>>+   When T2 is recorded by an CAND_ADD in the form of (T2' + C5), It
>                              ^                                      ^
>                              a                                      it
>
>>>+   will be further restructured to:
>>>+
>>>+    *PBASE:    T1
>>>+    *POFFSET:  MULT_EXPR (T2', C3)
>>>+    *PINDEX:   C1 + (C2 * C3) + C4 + (C5 * C3)  */
>>>+
>>> static bool
>>> restructure_reference (tree *pbase, tree *poffset, double_int
> *pindex,
>>>                     tree *ptype)
>>>@@ -777,7 +835,7 @@ restructure_reference (tree *pbase, tree *poffset,
>>>   double_int index = *pindex;
>>>   double_int bpu = double_int::from_uhwi (BITS_PER_UNIT);
>>>   tree mult_op0, mult_op1, t1, t2, type;
>>>-  double_int c1, c2, c3, c4;
>>>+  double_int c1, c2, c3, c4, c5;
>>>
>>>   if (!base
>>>       || !offset
>>>@@ -823,11 +881,12 @@ restructure_reference (tree *pbase, tree
> *poffset,
>>>     }
>>>
>>>   c4 = index.udiv (bpu, FLOOR_DIV_EXPR);
>>>+  c5 = backtrace_base_for_ref (&t2);
>>>
>>>   *pbase = t1;
>>>-  *poffset = fold_build2 (MULT_EXPR, sizetype, t2,
>>>-                       double_int_to_tree (sizetype, c3));
>>>-  *pindex = c1 + c2 * c3 + c4;
>>>+  *poffset = size_binop (MULT_EXPR, fold_convert (sizetype, t2),
>>>+                      double_int_to_tree (sizetype, c3));
>
> I am not sure why you changed this call.  fold_build2 is a more
> efficient call than size_binop.  size_binop makes several checks that
> will fail in this case, and then calls fold_build2_loc, right?  Not a
> big deal but seems like changing it back would be better.  Perhaps I'm
> missing something (as usual ;).
I rely on size_binop to convert T2 into sizetype, because T2' may be in other kind of type.  Otherwise there will be ssa_verify error later.

Thanks.
bin





Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]