diff mbox

RFC/RFA: Fix bug with REE optimization corrupting extended registers

Message ID 564E0452.50504@redhat.com
State New
Headers show

Commit Message

Bernd Schmidt Nov. 19, 2015, 5:18 p.m. UTC 
On 11/19/2015 06:06 PM, Jeff Law wrote:

> Frankly, the overall structure of ree is a mess -- I've found it
> incredibly difficult to follow every time I've had to look at it.

Yeah, no kidding. The check seems to be in the wrong place - it's done 
very late when we're replacing things, and IMO we should just restrict 
the candidates for optimization much earlier.

Also, I want to apply the following. Ok if testing succeeds?


Bernd

Comments

Jeff Law Nov. 19, 2015, 6:01 p.m. UTC | #1 
On 11/19/2015 10:18 AM, Bernd Schmidt wrote:
> On 11/19/2015 06:06 PM, Jeff Law wrote:
>
>> Frankly, the overall structure of ree is a mess -- I've found it
>> incredibly difficult to follow every time I've had to look at it.
>
> Yeah, no kidding. The check seems to be in the wrong place - it's done
> very late when we're replacing things, and IMO we should just restrict
> the candidates for optimization much earlier.
And I didn't make it any cleaner when I hacked it up to handle one more 
case :(    I'm hoping that a year away will allow me to look and think 
about the code again without getting a horrid headache.


>
> Also, I want to apply the following. Ok if testing succeeds?
>
>
> Bernd
>
> clean-ree1.diff
>
>
> commit ce68938b5150f5d41a54ed317ab97d98461be064
> Author: Bernd Schmidt<bernds_cb1@t-online.de>
> Date:   Thu Nov 19 17:38:15 2015 +0100
>
>      Readability improvements in ree.c:combine_reaching_defs.
>
>      	* ree.c (combine_reaching_defs): Simplify code by caching expressions
>      	in variables.
Yes. That's good with me.

jeff
diff mbox

Patch

commit ce68938b5150f5d41a54ed317ab97d98461be064
Author: Bernd Schmidt <bernds_cb1@t-online.de>
Date:   Thu Nov 19 17:38:15 2015 +0100

    Readability improvements in ree.c:combine_reaching_defs.
    
    	* ree.c (combine_reaching_defs): Simplify code by caching expressions
    	in variables.

diff --git a/gcc/ree.c b/gcc/ree.c
index b8436f2..4550cc3 100644
--- a/gcc/ree.c
+++ b/gcc/ree.c
@@ -731,6 +731,9 @@  get_extended_src_reg (rtx src)
 static bool
 combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
 {
+  rtx cand_pat = PATTERN (cand->insn);
+  rtx cand_dest = SET_DEST (cand_pat);
+  rtx cand_src = SET_SRC (cand_pat);
   rtx_insn *def_insn;
   bool merge_successful = true;
   int i;
@@ -749,9 +752,8 @@  combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
      register than the source operand, then additional restrictions
      are needed.  Note we have to handle cases where we have nested
      extensions in the source operand.  */
-  bool copy_needed
-    = (REGNO (SET_DEST (PATTERN (cand->insn)))
-       != REGNO (get_extended_src_reg (SET_SRC (PATTERN (cand->insn)))));
+  rtx src_reg = get_extended_src_reg (cand_src);
+  bool copy_needed = REGNO (cand_dest) != REGNO (src_reg);
   if (copy_needed)
     {
       /* Considering transformation of
@@ -780,8 +782,7 @@  combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
       if (state->modified[INSN_UID (cand->insn)].kind != EXT_MODIFIED_NONE)
 	return false;
 
-      machine_mode dst_mode = GET_MODE (SET_DEST (PATTERN (cand->insn)));
-      rtx src_reg = get_extended_src_reg (SET_SRC (PATTERN (cand->insn)));
+      machine_mode dst_mode = GET_MODE (cand_dest);
 
       /* Ensure the number of hard registers of the copy match.  */
       if (HARD_REGNO_NREGS (REGNO (src_reg), dst_mode)
@@ -812,17 +813,15 @@  combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
 	  || !REG_P (SET_DEST (*dest_sub_rtx)))
 	return false;
 
-      rtx tmp_reg = gen_rtx_REG (GET_MODE (SET_DEST (PATTERN (cand->insn))),
+      rtx tmp_reg = gen_rtx_REG (GET_MODE (cand_dest),
 				 REGNO (SET_DEST (*dest_sub_rtx)));
-      if (reg_overlap_mentioned_p (tmp_reg, SET_DEST (PATTERN (cand->insn))))
+      if (reg_overlap_mentioned_p (tmp_reg, cand_dest))
 	return false;
 
       /* The destination register of the extension insn must not be
 	 used or set between the def_insn and cand->insn exclusive.  */
-      if (reg_used_between_p (SET_DEST (PATTERN (cand->insn)),
-			      def_insn, cand->insn)
-	  || reg_set_between_p (SET_DEST (PATTERN (cand->insn)),
-				def_insn, cand->insn))
+      if (reg_used_between_p (cand_dest, def_insn, cand->insn)
+	  || reg_set_between_p (cand_dest, def_insn, cand->insn))
 	return false;
 
       /* We must be able to copy between the two registers.   Generate,
@@ -836,11 +835,8 @@  combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
 	 is different than in the code to emit the copy as we have not
 	 modified the defining insn yet.  */
       start_sequence ();
-      rtx pat = PATTERN (cand->insn);
-      rtx new_dst = gen_rtx_REG (GET_MODE (SET_DEST (pat)),
-                                 REGNO (get_extended_src_reg (SET_SRC (pat))));
-      rtx new_src = gen_rtx_REG (GET_MODE (SET_DEST (pat)),
-                                 REGNO (SET_DEST (pat)));
+      rtx new_dst = gen_rtx_REG (GET_MODE (cand_dest), REGNO (src_reg));
+      rtx new_src = gen_rtx_REG (GET_MODE (cand_dest), REGNO (cand_dest));
       emit_move_insn (new_dst, new_src);
 
       rtx_insn *insn = get_insns();
@@ -854,7 +850,6 @@  combine_reaching_defs (ext_cand *cand, const_rtx set_pat, ext_state *state)
 	return false;
     }
 
-
   /* If cand->insn has been already modified, update cand->mode to a wider
      mode if possible, or punt.  */
   if (state->modified[INSN_UID (cand->insn)].kind != EXT_MODIFIED_NONE)