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.
Bernd
diff --git a/gcc/ree.c b/gcc/ree.c
index 4550cc3..2c9d4d6 100644
--- a/gcc/ree.c
+++ b/gcc/ree.c
@@ -772,6 +772,12 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
if (state->defs_list.length () != 1)
return false;
+ /* We don't have the structure described above if there are
+ conditional moves in between the def and the candidate,
+ and we will not handle them correctly. See PR68194. */
+ if (state->copies_list.length () > 0)
+ return false;
+
/* We require the candidate not already be modified. It may,
for example have been changed from a (sign_extend (reg))
into (zero_extend (sign_extend (reg))).
