RE: [PATCH] Fix register corruption bug in ree

["Wilco Dijkstra" <wdijkstr at arm dot com>]

> Thanks!  Jakub noticed a potential problem in this area a while back,
> but I never came up with any code to trigger and have kept that issue on
> my todo list ever since.
> 
> Rather than ensuring the inserted copy write a single register, it seems
> to me we're better off ensuring that the number of hard registers hasn't
> changed.  Obviously that's not much of an issue for things like aarch64,
> x86_64, but for the embedded 32 bit parts it's likely much more important.
> 
> So I think the test is something like
> 
> x = get_extended_src_reg (SET_SRC (PATTERN (cand->insn)))
> if (HARD_REGNO_NREGS (REGNO (new_reg), cand->mode)
>      != HARD_REGNO_NREGS  (REGNO (new_reg), GET_MODE (SET_DEST (*orig_set))
>    return false;

I'm not 100% sure the rest of the code will correctly deal with multiple 
registers in all possible combinations. I think adding more asserts would help 
avoid similar silent codegeneration failures. For example, should widening 
from 1 to 2 registers:

(set (reg:TI x2) (zero_extend:TI (reg:DI x2)))

be regarded as a "copy_needed"? And what about TI x2 = zero_extend DI x3?
Both cases have overlaps, but copy_needed is set differently...

I also noticed there are more places where get_extended_src_reg is not used
(eg. in the code that emits the final copy).

Anyway here is the modified check:

---
 gcc/ree.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/gcc/ree.c b/gcc/ree.c
index 1431b7a..fa40484 100644
--- a/gcc/ree.c
+++ b/gcc/ree.c
@@ -743,6 +743,14 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
       if (!SCALAR_INT_MODE_P (GET_MODE (SET_DEST (PATTERN (cand->insn)))))
        return false;

+      enum machine_mode dst_mode = GET_MODE (SET_DEST (PATTERN (cand->insn)));
+      rtx src_reg = get_extended_src_reg (SET_SRC (PATTERN (cand->insn)));
+
+      /* Ensure the number of hard registers of the copy match.  */
+      if (HARD_REGNO_NREGS (REGNO (src_reg), dst_mode)
+         != HARD_REGNO_NREGS (REGNO (src_reg), GET_MODE (src_reg)))
+        return false;
+
       /* There's only one reaching def.  */
       rtx_insn *def_insn = state->defs_list[0];

@@ -792,7 +800,7 @@ combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
       start_sequence ();
       rtx pat = PATTERN (cand->insn);
       rtx new_dst = gen_rtx_REG (GET_MODE (SET_DEST (pat)),
-                                 REGNO (XEXP (SET_SRC (pat), 0)));
+                                 REGNO (get_extended_src_reg (SET_SRC (pat))));
       rtx new_src = gen_rtx_REG (GET_MODE (SET_DEST (pat)),
                                  REGNO (SET_DEST (pat)));
       emit_move_insn (new_dst, new_src);
-- 
1.9.1