lra_in_progress check in simplify_subreg_regno

[Richard Sandiford]

Hi Vlad,

The initial LRA merge added the lra_in_progress check to this code
in simplify_subreg_regno:

  /* Give the backend a chance to disallow the mode change.  */
  if (GET_MODE_CLASS (xmode) != MODE_COMPLEX_INT
      && GET_MODE_CLASS (xmode) != MODE_COMPLEX_FLOAT
      && REG_CANNOT_CHANGE_MODE_P (xregno, xmode, ymode)
      /* We can use mode change in LRA for some transformations.  */
      && ! lra_in_progress)
    return -1;

I realise that LRA internally uses subregs that wouldn't normally be valid
(e.g. to handle matching constraints) but I think it's really dangerous
to ignore REG_CANNOT_CHANGE_MODE_P and reduce a subreg to a specific
hard reg anyway.  CLASS_CANNOT_CHANGE_MODE says that all bets are off in
terms of the normal subreg rules.  E.g. the register class might have the
opposite endianness to GENERAL_REGS or something extreme like that.

It wasn't obvious from the comment exactly what case this !lra_in_progress
was handling, but I tried reverting it and saw a failure in gcc.dg/pr49948.c.
The problem there was with a subreg of the form:

    (subreg:DI (reg:V2DI X) 0)

We were trying to reload this subreg in GENERAL_REGS and X had been
allocated an SSE reg.  We then hit this code in curr_insn_transform,
which was deciding whether to reload the reg or the subreg:

	      if (REG_P (reg)
		  /* Strict_low_part requires reload the register not
		     the sub-register.	*/
		  && (curr_static_id->operand[i].strict_low
		      || (GET_MODE_SIZE (mode)
			  <= GET_MODE_SIZE (GET_MODE (reg))
			  && (hard_regno
			      = get_try_hard_regno (REGNO (reg))) >= 0
			  && (simplify_subreg_regno
			      (hard_regno,
			       GET_MODE (reg), byte, mode) < 0)
			  && (goal_alt[i] == NO_REGS
			      || (simplify_subreg_regno
				  (ira_class_hard_regs[goal_alt[i]][0],
				   GET_MODE (reg), byte, mode) >= 0)))))
		{
		  loc = &SUBREG_REG (*loc);
		  mode = GET_MODE (*loc);
		}

The first simplify_subreg_regno (on the SSE reg) used to return >= 0
thanks to the !lra_in_progress, even though the change is usually supposed
to be disallowed.  With the !lra_in_progress removed simplify_subreg_regno
rejects the simplification instead.  Then the second simplify_subreg_regno
(on the first allocatable GENERAL_REGS) succeeds and we go on to reload
the inner reg.

But the key in this case is that GENERAL_REGS isn't allowed to hold V2DImode,
so the simplification performed by the second simplify_subreg_regno
doesn't really have any meaning.  I think we should check that
ira_class_hard_regs[goal_alt[i]][0] can hold GET_MODE (reg) before
trying to simplify a subreg of it.

Tested on x86_64-linux-gnu ({,-m32}, all languages including Go and Ada).
Does it look OK?  Or, if the check was originally added for a different
situation, are you still able to reproduce it?

FWIW, this is all part of an attempt to fix the vec_select thing for x86_64.
I also want to get rid of the GET_MODE_CLASS checks seen in the first hunk,
but that's an entirely separate issue.

Thanks,
Richard


gcc/
	* rtlanal.c (simplify_subreg_regno): Remove lra_in_progress check.
	* lra-constraints.c (curr_insn_transform): Before trying to simplify
	a subreg, check whether the original reg is valid.

Index: gcc/rtlanal.c
===================================================================
--- gcc/rtlanal.c	2013-12-09 08:03:31.931201686 +0000
+++ gcc/rtlanal.c	2013-12-18 21:22:10.605987652 +0000
@@ -3533,9 +3533,7 @@ simplify_subreg_regno (unsigned int xreg
   /* Give the backend a chance to disallow the mode change.  */
   if (GET_MODE_CLASS (xmode) != MODE_COMPLEX_INT
       && GET_MODE_CLASS (xmode) != MODE_COMPLEX_FLOAT
-      && REG_CANNOT_CHANGE_MODE_P (xregno, xmode, ymode)
-      /* We can use mode change in LRA for some transformations.  */
-      && ! lra_in_progress)
+      && REG_CANNOT_CHANGE_MODE_P (xregno, xmode, ymode))
     return -1;
 #endif
 
Index: gcc/lra-constraints.c
===================================================================
--- gcc/lra-constraints.c	2013-12-09 21:35:05.589588280 +0000
+++ gcc/lra-constraints.c	2013-12-18 21:22:10.585987483 +0000
@@ -3493,9 +3493,12 @@ curr_insn_transform (void)
 			      (hard_regno,
 			       GET_MODE (reg), byte, mode) < 0)
 			  && (goal_alt[i] == NO_REGS
-			      || (simplify_subreg_regno
+			      || (HARD_REGNO_MODE_OK
 				  (ira_class_hard_regs[goal_alt[i]][0],
-				   GET_MODE (reg), byte, mode) >= 0)))))
+				   GET_MODE (reg))
+				  && (simplify_subreg_regno
+				      (ira_class_hard_regs[goal_alt[i]][0],
+				       GET_MODE (reg), byte, mode) >= 0))))))
 		{
 		  loc = &SUBREG_REG (*loc);
 		  mode = GET_MODE (*loc);