Re: Fix for PR42337

[Richard Guenther <richard dot guenther at gmail dot com>]

On Wed, Dec 9, 2009 at 12:33 AM, Xinliang David Li <davidxl@google.com> wrote:
> Hi, Please see attached patch for fixing PR42337.
>
> Testing:
> bootstrapped on x86_64/linux, regression test
>
> There is ?no observed (xalancbmk and gcc) compile time overhead.

The patch doesn't make sense to me.

From the description in the bugreport I understand that the problem is
we mark expressions as seen even though we do not translate them
(but return NULL later).

Your patch instead simply does not update seen at all when recursively
invoking from NARY_EXPRs (but not REFERENCEs?!).  So, why not
pass NULL for seen here instead of copying the old bitmap _for each operand_.

Certainly bad engineered.

Now I see:

  /* Prevent cycles when we have recursively dependent leaders.  This
     can only happen when phi translating the maximal set.  */
  if (seen)
    {
      unsigned int expr_id = get_expression_id (expr);
      if (bitmap_bit_p (seen, expr_id))
        return NULL;
      bitmap_set_bit (seen, expr_id);
    }

we do no longer translate the maximal set, so we can simply remove
this cycle detection at all?

So, not ok.

Thanks,
Richard.

> Thanks,
>
> David
>