On 01/16/14 15:07, Jakub Jelinek wrote:
On Thu, Jan 16, 2014 at 02:31:09PM -0700, Jeff Law wrote:
+2014-01-16 Jeff Law <l...@redhat.com>
+
+ * ree.c (combine_set_extension): Correct test for changing number
+ of hard registers when widening a reaching definition.
+
2014-01-16 Bernd Schmidt <ber...@codesourcery.com>
PR middle-end/56791
diff --git a/gcc/ree.c b/gcc/ree.c
index 19d821c..96cddd2 100644
--- a/gcc/ree.c
+++ b/gcc/ree.c
@@ -300,7 +300,8 @@ combine_set_extension (ext_cand *cand, rtx curr_insn, rtx
*orig_set)
/* We're going to be widening the result of DEF_INSN, ensure that doing so
doesn't change the number of hard registers needed for the result. */
if (HARD_REGNO_NREGS (REGNO (new_reg), cand->mode)
- != HARD_REGNO_NREGS (REGNO (orig_src), GET_MODE (SET_DEST (*orig_set))))
+ != HARD_REGNO_NREGS (REGNO (SET_DEST (*orig_set)),
+ GET_MODE (SET_DEST (*orig_set))))
Shouldn't that be:
if (HARD_REGNO_NREGS (REGNO (new_reg), cand->mode)
!= HARD_REGNO_NREGS (REGNO (new_reg), GET_MODE (SET_DEST (*orig_set))))
instead?
I mean, for the !copy_needed case it is obviously the same thing (and that
is what triggers in the testcase), but don't we generally want to check if
the same hard register in a wider mode will not occupy more registers, and
in particular the hard register we are considering to use on the lhs of the
defining insn (i.e. new_reg)?
I thought about using that conditional more than once. But talked
myself out of it every time on the grounds that I wanted to test the
original destination REGNO of the reaching def.
Obviously that is REGNO (new_reg) if !copy_needed. But it's something
completely different if copy_needed.
In the copy_needed case there's actually two destinations to consider.
The original destination as well as the new destination. Both will be
set in a mode wider than the destination of the original reaching def.
(one will be set in the modified reaching def and the other in a copy insn).
ISTM we need the # hard reg checked on the original destination as the
other (upper) hard regs might be live across the sequence, but not
used/set in the sequence. Then we need some kind of check on the upper
part of the new destination... But I thought I covered that elsewhere...
Anyway, I clearly need to rethink that test. Given this is something we
haven't seen in the wild, I'm going to disable it over the
weekend/monday so that enable-checking bugs pass and continue to ponder.
jeff
