On Tue, Apr 19, 2016 at 1:56 PM, Richard Biener <richard.guent...@gmail.com> wrote: > On Wed, Mar 2, 2016 at 3:28 PM, Christophe Lyon > <christophe.l...@linaro.org> wrote: >> On 29 February 2016 at 05:28, kugan <kugan.vivekanandara...@linaro.org> >> wrote: >>> >>>> That looks better, but I think the unordered_remove will break operand >>>> sorting >>>> and thus you probably don't handle x + x + x + x + y + y + y + y + y + >>>> y + z + z + z + z >>>> optimally. >>>> >>>> I'd say you simply want to avoid the recursion and collect a vector of >>>> [start, end] pairs >>>> before doing any modification to the ops vector. >>> >>> >>> Hi Richard, >>> >>> Is the attached patch looks better? >>> >> >> Minor comment, I've noticed typos in your updated comment: >> "There should be two multiplication left in test1 (inculding one generated" >> should be >> "There should be two multiplications left in test1 (including one generated" > > +/* Transoform repeated addition of same values into multiply with > + constant. */ > > Transform > > +static void > +transform_add_to_multiply (gimple_stmt_iterator *gsi, gimple *stmt, > vec<operand_entry *> *ops) > > split the long line > > op_list looks redundant - ops[start]->op gives you the desired value > already and if you > use a vec<std::pair<int, int>> you can have a more C++ish start,end pair. > > + tree tmp = make_temp_ssa_name (TREE_TYPE (op), NULL, "reassocmul"); > + gassign *mul_stmt = gimple_build_assign (tmp, MULT_EXPR, > + op, build_int_cst > (TREE_TYPE(op), count)); > > this won't work for floating point or complex numbers - you need to use sth > like > fold_convert (TREE_TYPE (op), build_int_cst (integer_type_node, count)); > > For FP types you need to guard the transform with > flag_unsafe_math_optimizations > > + gimple_set_location (mul_stmt, gimple_location (stmt)); > + gimple_set_uid (mul_stmt, gimple_uid (stmt)); > + gsi_insert_before (gsi, mul_stmt, GSI_SAME_STMT); > > I think you do not want to set the stmt uid and you want to insert the > stmt right > after the def of op (or at the original first add - though you can't > get your hands at > that easily). You also don't want to set the location to the last stmt of the > whole add sequence - simply leave it unset. > > + oe = operand_entry_pool.allocate (); > + oe->op = tmp; > + oe->rank = get_rank (op) * count; > > ? Why that? oe->rank should be get_rank (tmp). > > + oe->id = 0; > > other places use next_operand_entry_id++. I think you want to simply > use add_to_ops_vec (oe, tmp); here for all of the above. > > Please return whether you did any optimization and do the > qsort of the operand vector only if you did sth. > > Testcase with FP math missing. Likewise with complex or vector math.
Btw, does it handle associating x + 3 * x + x to 5 * x ? Richard. > Thanks, > Richard. > >>> Thanks, >>> Kugan
