Re: [PATCH] Fix PR58775

[Zhenqiang Chen <zhenqiang dot chen at linaro dot org>]

On 18 October 2013 19:09, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Oct 18, 2013 at 06:36:44PM +0800, Zhenqiang Chen wrote:
> --- a/gcc/tree-ssa-reassoc.c
> +++ b/gcc/tree-ssa-reassoc.c
> @@ -2861,6 +2861,19 @@ swap_ops_for_binary_stmt (vec<operand_entry_t> ops,
>      }
>  }
>
> +/* Determine if stmt A is in th next list of stmt B.  */
> +static inline bool
> +next_stmt_of (gimple a, gimple b)
> +{
> +  gimple_stmt_iterator gsi;
> +  for (gsi = gsi_for_stmt (b); !gsi_end_p (gsi); gsi_next (&gsi))
> +    {
> +      if (gsi_stmt (gsi) == a)
> +       return true;
> +    }
> +  return false;
> +}
>
> How is that different from appears_later_in_bb?  More importantly,
> not_dominated_by shouldn't be called with the same uid and basic block,
> unless the stmts are the same, except for the is_gimple_debug case
> (which looks like a bug).  And more importantly, if a stmt has zero uid,
> we'd better set it from the previous or next non-zero uid stmt, so that
> we don't lineary traverse the whole bb many times.

Thanks for the comments.

Patch is updated to set uid in update_range_test after force_gimple_operand_gsi.

I am not sure the patch can cover all cases. If not, I think
force_gimple_operand_gsi_1 maybe the only place to set uid for all new
generated statements.

Thanks!
-Zhenqiang

> That said, I've lost track of the bugfixes for this, don't know if there is
> any patch still pending or not.
>
>         Jakub

Attachment: pr58775-2.patch
Description: Binary data