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: [RFC][PATCH][PR63586] Convert x+x+x+x into 4*x


On Wed, Mar 2, 2016 at 3:28 PM, Christophe Lyon
<christophe.lyon@linaro.org> wrote:
> On 29 February 2016 at 05:28, kugan <kugan.vivekanandarajah@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.

Thanks,
Richard.

>> Thanks,
>> Kugan


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