Hi!
[ I missed this patch the first time around; please cc: me to prevent this ]
On Thu, May 18, 2017 at 07:25:57AM -0300, Alexandre Oliva wrote:
> When an insn used by combine has multiple SETs, only the non-REG_UNUSED
> set is used: others will end up dropped on the floor.
Sometimes, yes; not always.
> We have to take
> note of the dropped REG_UNUSED SETs, clearing their cached values, so
> that, even if the REGs remain used (e.g. because they were referenced
> in the used SET_SRC), we will not use properties of the latest value
> as if they applied to the earlier one.
The reg_stat stuff is no end of pain, sigh.
> PR rtl-optimization/80693
> * combine.c (distribute_notes): Add IDEST parameter. Reset any
> REG_UNUSED REGs that are not IDEST, if IDEST is given. Adjust
> all callers.
Most callers use NULL_RTX for idest. It isn't obvious to me that this
is correct.
> @@ -14087,6 +14090,26 @@ distribute_notes (rtx notes, rtx_insn *from_insn,
> rtx_insn *i3, rtx_insn *i2,
> PUT_REG_NOTE_KIND (note, REG_DEAD);
> place = i3;
> }
> +
> + /* If there were any parallel sets in FROM_INSN other than
> + the one setting IDEST, it must be REG_UNUSED, otherwise
> + we could not have used FROM_INSN in combine. Since this
> + combine attempt succeeded, we know this unused SET was
> + dropped on the floor, because the insn was either deleted
> + or created from a new pattern that does not use its
> + SET_DEST. We must forget whatever we knew about the
> + value that was stored by that SET, since the prior value
> + may still be present in IDEST's src expression or
> + elsewhere, and we do not want to use properties of the
> + dropped value as if they applied to the prior one when
> + simplifying e.g. subsequent combine attempts. */
> + if (idest && XEXP (note, 0) != idest)
Would it work to just have "else" instead if this "if"? Or hrm, we'll
need to kill the recorded reg_stat value in the last case before this
as well?
> + {
> + gcc_assert (REG_P (XEXP (note, 0)));
> + record_value_for_reg (XEXP (note, 0), NULL, NULL_RTX);
> + INC_REG_N_SETS (REGNO (XEXP (note, 0)), -1);
> + }
> +
> break;
Could you try that out? Or I can do it, let me know what you prefer.
Thanks,
Segher
