Re: [PATCH][RTL-ree] PR rtl-optimization/68194: Restrict copy instruction in presence of conditional moves

[Kyrill Tkachov <kyrylo dot tkachov at arm dot com>]

On 17/11/15 12:10, Bernd Schmidt wrote:
> On 11/17/2015 10:08 AM, Kyrill Tkachov wrote:
> > Yes, I had considered that as well. It should be equivalent. I didn't
> > use !reg_used_between_p because I thought
> > it'd be more expensive than checking reg_overlap_mentioned_p since we
> > must iterate over a number of instructions
> > and call reg_overlap_mentioned_p on each one. But I suppose this case is
> > rare enough that it wouldn't make any
> > measurable difference.
> >
> > Would you prefer to use !reg_used_between_p here?
>
> I would but apparently it doesn't work, so that's kind of neither here nor there.
>
> > > The added comment could lead to some confusion since it's placed in
> > > front of an existing if statement that also tests a different
> > > condition. Also, if we go with your fix,
> > >
> > > > +      || !reg_overlap_mentioned_p (tmp_reg, SET_SRC (PATTERN
> > > > (cand->insn))))
> > >
> > > Shouldn't this really be !rtx_equal_p?
> >
> > Maybe, will it behave the right way if the two regs have different modes
> > or when subregs are involved?
>
> It would return false, in which case we'll conservatively fail here. I think that's desirable?

Well, I think the statement we want to make is
"return false from this function if the two expressions contain the same register number".
if (!rtx_equal_p (..., ...))
  return false;

will only return false if the two expressions are the same REG with the same mode.
if (!reg_overlap_mentioned_p (..., ...))
  return false;

should return false even if the modes are different or one is a subreg, which is what we want.

I did not see any codegen regressions using reg_overlap_mentioned_p on aarch64, so I don't think
it will restrict any legitimate cases.

Thanks,
Kyrill

> Bernd