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