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

[Kyrill Tkachov <kyrylo.tkachov@arm.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
>