On 20/11/15 01:41, Bernd Schmidt wrote:
I1 is def_insn, I3 is cand->insn. tmp_reg is 'ax'. What we want to do
is reject this transformation
because the destination of def_insn (aka I1), that is 'ax', is not the
operand of the extend operation
in cand->insn (aka I3). As you said, rtx_equal won't work on just
SET_SRC (PATTERN (cand->insn)) because
it's an extend operation. So reg_overlap_mentioned should be appropriate.
Yeah, so just use the src_reg variable for the comparison. I still don't see why you wouldn't want to use the stronger test. But the whole thing still feels not completely ideal somehow, so after reading through ree.c for a while and
getting a better feeling for how it works, I think the following (which you said is equivalent) would be the most understandable and direct fix.
You said that the two tests should be equivalent, and I agree. I've not found
cases where the change makes a difference, other than the testcase. Would you
mind running this version through the testsuite and committing if it passes?
I've shrunk the comment; massive explanations like this for every bug are inappropriate IMO, and the example also duplicates an earlier comment in the same function. And, as I said earlier, the way you placed the comment is confusing
because only one part of the following if statement is related to it.
Ok, thanks for the explanation.
When investigating this bug I tried a patch identical to yours and it had
worked just fine.
My patch was just an alternative approach to the same issue.
I'll retest it just to double-check and I'll incorporate the testsuite
additions.
Thanks for your help!
Kyrill
Bernd
