>    The following patch also solves the problem, but it makes me feel the
> need to state that I've *not* tried to make it look as bad as possible. :-(

I think that your solution is the minimally correct one, so no need for any 
form of shyness. ;-)

> @@ -3297,23 +3299,6 @@ try_combine (rtx i3, rtx i2, rtx i1, int
>         undo_all ();
>         return 0;
>       }
> -
> -      PATTERN (undobuf.other_insn) = other_pat;
> -
> -      /* If any of the notes in OTHER_INSN were REG_UNUSED, ensure that
> they -         are still valid.  Then add any non-duplicate notes added by
> -      recog_for_combine.  */
> -      for (note = REG_NOTES (undobuf.other_insn); note; note = next)
> -     {
> -       next = XEXP (note, 1);
> -
> -       if (REG_NOTE_KIND (note) == REG_UNUSED
> -           && ! reg_set_p (XEXP (note, 0), PATTERN (undobuf.other_insn)))
> -         remove_note (undobuf.other_insn, note);
> -     }
> -
> -      distribute_notes (new_other_notes, undobuf.other_insn,
> -                     undobuf.other_insn, NULL_RTX, NULL_RTX, NULL_RTX);
>      }
>  #ifdef HAVE_cc0
>    /* If I2 is the CC0 setter and I3 is the CC0 user then check whether

Add the missing line skip before the #ifdef.

> @@ -3331,12 +3316,33 @@ try_combine (rtx i3, rtx i2, rtx i1, int
>
>    /* Only allow this combination if insn_rtx_costs reports that the
>       replacement instructions are cheaper than the originals.  */
> -  if (!combine_validate_cost (i1, i2, i3, newpat, newi2pat))
> +  if (!combine_validate_cost (i1, i2, i3, newpat, newi2pat, other_pat))
>      {
>        undo_all ();
>        return 0;
>      }
>
> +  if (undobuf.other_insn)
> +    {
> +      rtx note, next;
> +
> +      PATTERN (undobuf.other_insn) = other_pat;
> +
> +      /* If any of the notes in OTHER_INSN were REG_UNUSED, ensure that
> they +         are still valid.  Then add any non-duplicate notes added by
> +      recog_for_combine.  */
> +      for (note = REG_NOTES (undobuf.other_insn); note; note = next)
> +     {
> +       next = XEXP (note, 1);
> +
> +       if (REG_NOTE_KIND (note) == REG_UNUSED
> +           && ! reg_set_p (XEXP (note, 0), PATTERN (undobuf.other_insn)))
> +         remove_note (undobuf.other_insn, note);
> +     }
> +
> +      distribute_notes (new_other_notes, undobuf.other_insn,
> +                     undobuf.other_insn, NULL_RTX, NULL_RTX, NULL_RTX);
> +    }
>    /* We now know that we can do this combination.  Merge the insns and
>       update the status of registers and LOG_LINKS.  */

And put the new block just after the above comment, surrounded by a couple of 
line skips.

>    If I'm going to say something I like about this version of the patch,
> then it'll be that we don't mung the notes until we've checked the costs.

Right, I also think that this is a progress.

The patch is pre-approved for mainline with the 2 aforementioned changes.

-- 
Eric Botcazou

Reply via email to