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: [PR80693] drop value of parallel SETs dropped by combine


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


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