Re: [PATCH][RTL-ifcvt] PR rtl-optimization/68506: Fix emitting order of insns in IF-THEN-JOIN case

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

On 26/11/15 14:23, Bernd Schmidt wrote:
> On 11/26/2015 02:52 PM, Kyrill Tkachov wrote:
> > On 26/11/15 13:40, Bernd Schmidt wrote:
> > > On 11/26/2015 12:12 PM, Kyrill Tkachov wrote:
> > > >       modified_in_b = emit_b != NULL_RTX && modified_in_p (orig_a,
> > > > emit_b);
> > >
> > > Can this ever be true? We arrange for emit_b to set a new pseudo,
> > > don't we? Are we allowing cases where we copy a pattern that sets more
> > > than one register, and is that safe?
> >
> > You're right, this statement always sets modifieb_in_b to false. We
> > reject anything bug single_set insns
> > by this point in the code. I'll replace that with modified_in_b = false;
>
> Note that there's a mirrored test for modified_in_a, and both are already initialized to false.

Yeah, that can be changed to just false too. I'll do that in the next revision.

> Also - careful with single_set, it can return true even for multiple sets in case there's a REG_DEAD note on one of them. You might want to strengthen your tests to also include !multiple_sets. Then, maybe instead of deleting these tests, 
> turn them into gcc_checking_asserts.

I see. I think the best place to do that would be in insn_valid_noce_process_p and just get it to return
false if multiple_sets (insn) is true.

Would it be ok if I did that as a separate follow-up patch?
We don't have a testcase where this actually causes trouble and I'd like to keep the fix for
this PR as self-contained as possible.

Thanks,
Kyrill



> Bernd