On Mon, Mar 30, 2020 at 11:23:12AM -0500, Peter Bergner wrote:
> On 3/30/20 6:26 AM, Segher Boessenkool wrote:
> > On Mon, Mar 30, 2020 at 09:50:05AM +0100, Richard Sandiford wrote:
> >> Peter Bergner via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> >>> - if (HARD_REGISTER_NUM_P (rd) || HARD_REGISTER_NUM_P (rs))
> >>> + if (HARD_REGISTER_NUM_P (rd))
> >>> return false;
> >>>
> >>> b = reg_copy_graph[rs];
> >>
> >> I guess this would also work if we dropped the rd check instead.
> >> So how about s/||/&&/ instead, to avoid the assymetry?
> >>
> >> I agree something like this is a better fix long-term, since we
> >> shouldn't be relying on make_more_copies outside combine.
> >
> > Yes; on the other hand, most RTL passes should do something to not have
> > hard registers forwarded into non-move instructions (where they can
> > cause problems later). (make_more_copies itself is a technicality
> > specific to how combine works, and we might be able to drop it in the
> > future).
>
> I kind of agree with Richard above on making it more applicable/symmetric,
> but why can't we just remove the HARD_REGISTER_NUM_P() tests altogether?
> It's not like lower-subreg is extending hard register lifetime usage than
> what is already there in the rtl. We're just decomposing what's already
> there into smaller register sized chunks. Is there a problem with that
> I'm not aware of?
The function comment is (since day 1):
/* If SET is a copy from one multi-word pseudo-register to another,
record that in reg_copy_graph. Return whether it is such a
copy. */
so a) that needs fixing; and b) what does this change for the (single)
caller of find_pseudo_copy? The comment there isn't very enlightening:
/* We mark pseudo-to-pseudo copies as decomposable during the
second pass only. The first pass is so early that there is
good chance such moves will be optimized away completely by
subsequent optimizations anyway.
However, we call find_pseudo_copy even during the first pass
so as to properly set up the reg_copy_graph. */
(The function *name* should change as well, if you make this change).
Segher
