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 <wschm...@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