diff mbox

patch to fix a LRA crash on ppc

Message ID 526EF135.9040303@redhat.com
State New
Headers show

Commit Message

Vladimir Makarov Oct. 28, 2013, 11:20 p.m. UTC
The following patch is a new version of a fix for LRA crash on 32-bit 
ppc SPEC2006 gamess.

   The previous version broke mode-switching optimization on x86/x86-64.

   The patch was successfully bootstrapped and tested on x86/x86-64.

   Committed as rev. 204140.

2013-10-28  Vladimir Makarov  <vmakarov@redhat.com>

         * lra-spills.c (lra_final_code_change): Remove useless move insns
         originated from moves of pseudos.

Comments

Mike Stump Oct. 29, 2013, 9:45 p.m. UTC | #1
On Oct 28, 2013, at 4:20 PM, Vladimir Makarov <vmakarov@redhat.com> wrote:
>        * lra-spills.c (lra_final_code_change): Remove useless move insns
>        originated from moves of pseudos.

So I was facing a problem of extraneous moves of multiple registers:

(set (reg:TI 1) (mem …))

===

(set (reg:DI 2) (subreg:DI 1 0))
(set (reg:DI 3) (subreg:DI 1 8)) (dead (reg 1))

(set (subreg:DI (reg:TI 4) 0) (reg:DI 2)) (dead (reg 2))
(set (subreg:DI (reg:TI 4) 8) (reg:DI 3)) (dead (reg 3))

===

(set (mem …) (reg:TI 4))

and I thought I would try out LRA to see if it would notice that the subregs completely cover the original data, and that the ordering of subregs was the same.  All the instructions between the === pair are extraneous and can be eliminated and the registers 1 and 4 can be assigned to the same physical register by LRA.  Any thoughts on how to structure the emitted code in the port to try and help LRA out so that it can eliminate the extra moves (when possible)?  In the general case, the data motions can be necessary and there can be arbitrary operations on the data in flight, so I have to generate the moves.  The above is the degenerate case where the data is merely moved from point a to point b.  The moves are generated as part of the expansion of the instructions outside the ===.

If you can enhance lra to notice the subregs completely cover the value and that the lifetimes of the two don't overlap (save to do the copy) and eliminate the extraneous moves in the middle and use the same register for 1 and 4, that's be wonderful.

I think I can recognize larger chunks of code (the memory to memory move directly in this case), and ensure I don't generate the extra moves in the first place, but, it just seems like a hack.
diff mbox

Patch

Index: lra-spills.c
===================================================================
--- lra-spills.c	(revision 204131)
+++ lra-spills.c	(working copy)
@@ -625,7 +625,7 @@  lra_final_code_change (void)
 {
   int i, hard_regno;
   basic_block bb;
-  rtx insn, curr;
+  rtx insn, curr, set;
   int max_regno = max_reg_num ();
 
   for (i = FIRST_PSEUDO_REGISTER; i < max_regno; i++)
@@ -636,6 +636,7 @@  lra_final_code_change (void)
     FOR_BB_INSNS_SAFE (bb, insn, curr)
       if (INSN_P (insn))
 	{
+	  bool change_p;
 	  rtx pat = PATTERN (insn);
 
 	  if (GET_CODE (pat) == CLOBBER && LRA_TEMP_CLOBBER_P (pat))
@@ -648,6 +649,12 @@  lra_final_code_change (void)
 	      continue;
 	    }
 
+	  set = single_set (insn);
+	  change_p = (set != NULL
+		      && REG_P (SET_SRC (set)) && REG_P (SET_DEST (set))
+		      && REGNO (SET_SRC (set)) >= FIRST_PSEUDO_REGISTER
+		      && REGNO (SET_DEST (set)) >= FIRST_PSEUDO_REGISTER);
+	  
 	  lra_insn_recog_data_t id = lra_get_insn_recog_data (insn);
 	  struct lra_static_insn_data *static_id = id->insn_static_data;
 	  bool insn_change_p = false;
@@ -661,5 +668,20 @@  lra_final_code_change (void)
 	      }
 	  if (insn_change_p)
 	    lra_update_operator_dups (id);
+
+	  if (change_p && REGNO (SET_SRC (set)) == REGNO (SET_DEST (set)))
+	    {
+	      /* Remove an useless move insn but only involving
+		 pseudos as some subsequent optimizations are based on
+		 that move insns involving originally hard registers
+		 are preserved.  IRA can generate move insns involving
+		 pseudos.  It is better remove them earlier to speed
+		 up compiler a bit.  It is also better to do it here
+		 as they might not pass final RTL check in LRA,
+		 (e.g. insn moving a control register into
+		 itself).  */
+	      lra_invalidate_insn_data (insn);
+	      delete_insn (insn);
+	    }
 	}
 }