> As described both in the PR and patch comments, this patch fixes PR62151 by
> setting right value to ELIM_I0/ELIM_I1 when distributing REG_DEAD notes from
> i0/i1. It is said that distribute_notes had caused many bugs in the past.
> I think it still has bug in it, as noted in the PR. This patch doesn't
> touch distribute_notes because we are in stage3 and I want to have more
> discussion on it.
> Bootstrap and test on x86_64. aarch64 is ongoing. So is it ok?
>
> 2014-12-11 Bin Cheng <bin.ch...@arm.com>
>
> PR rtl-optimization/62151
> * combine.c (try_combine): Reset elim_i0 and elim_i1 when
> distributing notes from i0notes or i1notes, this time don't
> check whether newi2pat sets i1dest or i0dest.
The reasoning looks correct to me and the patch is certainly safe so it's OK
on principle, but I think that we should avoid the duplication of predicates.
Can you move the computation of the alternative elim_i1 & elim_i0 up to where
the original ones are computed along with the explanation of why we care about
newi2pat only for notes that were on I3 and I2? Something like:
/* Compute which registers we expect to eliminate. newi2pat may be setting
either i3dest or i2dest, so we must check it. */
rtx elim_i2 = ((newi2pat && reg_set_p (i2dest, newi2pat))
|| i2dest_in_i2src || i2dest_in_i1src || i2dest_in_i0src
|| !i2dest_killed
? 0 : i2dest);
/* For I1 we need to compute both local elimination and global elimination
because i1dest may be the same as i3dest, in which case newi2pat may be
setting i1dest. <big explanation of why this is needed> */
rtx local_elim_i1 = (i1 == 0 || i1dest_in_i1src || i1dest_in_i0src
|| !i1dest_killed
? 0 : i1dest);
rtx elim_i1 = (local_elim_i1 == 0
|| (newi2pat && reg_set_p (i1dest, newi2pat))
? 0 : i1dest);
/* Likewise for I0. */
rtx local_elim_i0 = (i0 == 0 || i0dest_in_i0src
|| !i0dest_killed
? 0 : i0dest);
rtx elim_i0 = (local_elim_i0 == 0
|| (newi2pat && reg_set_p (i0dest, newi2pat))
? 0 : i0dest);
--
Eric Botcazou
