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] Fix PR52976


On Sat, Apr 14, 2012 at 7:05 PM, William J. Schmidt
<wschmidt@linux.vnet.ibm.com> wrote:
> This patch corrects two errors in reassociating expressions with
> repeated factors. ?First, undistribution needs to recognize repeated
> factors. ?For now, repeated factors will be ineligible for this
> optimization. ?In the future, this can be improved. ?Second, when a
> __builtin_powi call is introduced, its target SSA name must be given a
> rank higher than other operands in the operand list. ?Otherwise, uses of
> the call result may be introduced prior to the call.
>
> Bootstrapped and regression tested on powerpc64-linux. ?Confirmed that
> cpu2000 and cpu2006 SPEC tests build cleanly. ?OK for trunk?

Ok, given it fixes quite some fallout.

But I wonder why the rank computation does not properly "work"
automagically in the powi case.

Also for undistribution it looks like this might introduce missed optimizations?
Thus, how hard would it be to teach it to properly handle ->count != 1?  ISTR
it does some counting itself.

Thanks,
Richard.

> Thanks,
> Bill
>
>
> 2012-04-14 ?Bill Schmidt ?<wschmidt@linux.vnet.ibm.com>
>
> ? ? ? ?PR tree-optimization/52976
> ? ? ? ?* tree-ssa-reassoc.c (add_to_ops_vec_max_rank): New function.
> ? ? ? ?(undistribute_ops_list): Ops with repeat counts aren't eligible for
> ? ? ? ?undistribution.
> ? ? ? ?(attempt_builtin_powi): Call add_to_ops_vec_max_rank.
>
>
> Index: gcc/tree-ssa-reassoc.c
> ===================================================================
> --- gcc/tree-ssa-reassoc.c ? ? ?(revision 186393)
> +++ gcc/tree-ssa-reassoc.c ? ? ?(working copy)
> @@ -544,6 +544,28 @@ add_repeat_to_ops_vec (VEC(operand_entry_t, heap)
> ? reassociate_stats.pows_encountered++;
> ?}
>
> +/* Add an operand entry to *OPS for the tree operand OP, giving the
> + ? new entry a larger rank than any other operand already in *OPS. ?*/
> +
> +static void
> +add_to_ops_vec_max_rank (VEC(operand_entry_t, heap) **ops, tree op)
> +{
> + ?operand_entry_t oe = (operand_entry_t) pool_alloc (operand_entry_pool);
> + ?operand_entry_t oe1;
> + ?unsigned i;
> + ?unsigned max_rank = 0;
> +
> + ?FOR_EACH_VEC_ELT (operand_entry_t, *ops, i, oe1)
> + ? ?if (oe1->rank > max_rank)
> + ? ? ?max_rank = oe1->rank;
> +
> + ?oe->op = op;
> + ?oe->rank = max_rank + 1;
> + ?oe->id = next_operand_entry_id++;
> + ?oe->count = 1;
> + ?VEC_safe_push (operand_entry_t, heap, *ops, oe);
> +}
> +
> ?/* Return true if STMT is reassociable operation containing a binary
> ? ?operation with tree code CODE, and is inside LOOP. ?*/
>
> @@ -1200,6 +1222,7 @@ undistribute_ops_list (enum tree_code opcode,
> ? ? ? dcode = gimple_assign_rhs_code (oe1def);
> ? ? ? if ((dcode != MULT_EXPR
> ? ? ? ? ? && dcode != RDIV_EXPR)
> + ? ? ? ? || oe1->count != 1
> ? ? ? ? ?|| !is_reassociable_op (oe1def, dcode, loop))
> ? ? ? ?continue;
>
> @@ -1243,6 +1266,8 @@ undistribute_ops_list (enum tree_code opcode,
> ? ? ? ? ?oecount c;
> ? ? ? ? ?void **slot;
> ? ? ? ? ?size_t idx;
> + ? ? ? ? if (oe1->count != 1)
> + ? ? ? ? ? continue;
> ? ? ? ? ?c.oecode = oecode;
> ? ? ? ? ?c.cnt = 1;
> ? ? ? ? ?c.id = next_oecount_id++;
> @@ -1311,7 +1336,7 @@ undistribute_ops_list (enum tree_code opcode,
>
> ? ? ? ? ?FOR_EACH_VEC_ELT (operand_entry_t, subops[i], j, oe1)
> ? ? ? ? ? ?{
> - ? ? ? ? ? ? if (oe1->op == c->op)
> + ? ? ? ? ? ? if (oe1->op == c->op && oe1->count == 1)
> ? ? ? ? ? ? ? ?{
> ? ? ? ? ? ? ? ? ?SET_BIT (candidates2, i);
> ? ? ? ? ? ? ? ? ?++nr_candidates2;
> @@ -3275,8 +3300,10 @@ attempt_builtin_powi (gimple stmt, VEC(operand_ent
> ? ? ? ? ?gsi_insert_before (&gsi, pow_stmt, GSI_SAME_STMT);
> ? ? ? ?}
>
> - ? ? ?/* Append the result of this iteration to the ops vector. ?*/
> - ? ? ?add_to_ops_vec (ops, iter_result);
> + ? ? ?/* Append the result of this iteration to the ops vector.
> + ? ? ? ? Give it a rank higher than all other ranks in the ops vector
> + ? ? ? ? so that all uses of it will be forced to come after it. ?*/
> + ? ? ?add_to_ops_vec_max_rank (ops, iter_result);
>
> ? ? ? /* Decrement the occurrence count of each element in the product
> ? ? ? ? by the count found above, and remove this many copies of each
>
>


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