This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [RFC][PATCH][PR63586] Convert x+x+x+x into 4*x
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Christophe Lyon <christophe dot lyon at linaro dot org>
- Cc: kugan <kugan dot vivekanandarajah at linaro dot org>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Tue, 19 Apr 2016 13:56:17 +0200
- Subject: Re: [RFC][PATCH][PR63586] Convert x+x+x+x into 4*x
- Authentication-results: sourceware.org; auth=none
- References: <56CFC059 dot 40108 at linaro dot org> <CAFiYyc3HM-Y526mhuuYbNSOs8pFG+Bqc_Uz30zvoU4D09c2bRA at mail dot gmail dot com> <56D3C8D9 dot 5020508 at linaro dot org> <CAKdteOYn7L=P1XZv9a9TdJ1y21zm5tk8oeSg+iJivyTiOpaXzg at mail dot gmail dot com>
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