Patchwork [RFC] Finer grained reg classes.

login
register
mail settings
Submitter David Miller
Date March 19, 2013, 3:33 p.m.
Message ID <20130319.113349.779301153261368350.davem@davemloft.net>
Download mbox | patch
Permalink /patch/229098/
State New
Headers show

Comments

David Miller - March 19, 2013, 3:33 p.m.
This is very much a work in progress, but I think it has potential
to solve the problem at hand.

A major blocker for using LRA on sparc is a fundamental limitation of
register classes as currently implemented.

If you have an instruction that requires an evenly aligned hard
register pair, you cannot describe this alone using register classes.

That's because register classes must contain all registers of a
multiple register set, which effectively would allow both even
and odd registers, thus not constraining the hard registers which
are valid at all.

Targets work around this by using HARD_REGNO_MODE_OK(), which causes
pre-LRA reload to fix up all the hard register allocation mistakes
made by IRA, the latter of which relies largely upon register classes.

LRA relies upon register classes completely, so targets really have to
express an instruction's exact needs using register classes rather
than ad-hoc constraints like the "U" constraint sparc is using:

(define_constraint "U"
 "Pseudo-register or hard even-numbered integer register"
 (and (match_test "TARGET_ARCH32")
      (match_code "reg")
      (ior (match_test "REGNO (op) < FIRST_PSEUDO_REGISTER")
	   (not (match_test "reload_in_progress && reg_renumber [REGNO (op)] < 0")))
      (match_test "register_ok_for_ldd (op)")))

So this patch tries to rework the semantics of hard register classes,
such that if a hard register is present in the set it is implied that
the rest of the registers in a multi-register group are present as
well.  So we can add a register class called EVEN_REGS and only have
to set the even register bits.

The patch below is enough to get zero regressions for the c/c++
testsuite on 32-bit sparc, but I do know that there are some problems
that need to be resolved still.

For example, I think we need some way to validate a regclass based
upon the mode, something slightly different from HARD_REGNO_MODE_OK.

This would be used for things like the "a" constraint on x86 so that
on 32-bit a user couldn't try to use the "a" constraint in an inline
asm for a 64-bit value.

In the compiler itself, this really isn't an issue, because the
machine description would only use register classes for the proper
mode.

I intend to also use this new regclass scheme to represent FPU double
register pairs on sparc as well.

Anyways, any comments would be appreciated, thanks.
Ian Taylor - March 19, 2013, 5:58 p.m.
On Tue, Mar 19, 2013 at 8:33 AM, David Miller <davem@davemloft.net> wrote:
>
> So this patch tries to rework the semantics of hard register classes,
> such that if a hard register is present in the set it is implied that
> the rest of the registers in a multi-register group are present as
> well.  So we can add a register class called EVEN_REGS and only have
> to set the even register bits.

I haven't really looked at your patch, but I just want to say that I
think this is the right way to go.

Ian
Jeff Law - March 19, 2013, 6:31 p.m.
On 03/19/2013 11:58 AM, Ian Lance Taylor wrote:
> On Tue, Mar 19, 2013 at 8:33 AM, David Miller <davem@davemloft.net> wrote:
>>
>> So this patch tries to rework the semantics of hard register classes,
>> such that if a hard register is present in the set it is implied that
>> the rest of the registers in a multi-register group are present as
>> well.  So we can add a register class called EVEN_REGS and only have
>> to set the even register bits.
>
> I haven't really looked at your patch, but I just want to say that I
> think this is the right way to go.
Likewise.  I've certainly worked on ports where this would have been 
helpful in the past.

Jeff
Steven Bosscher - March 19, 2013, 7:17 p.m.
On Tue, Mar 19, 2013 at 7:31 PM, Jeff Law wrote:
> On 03/19/2013 11:58 AM, Ian Lance Taylor wrote:
>>
>> On Tue, Mar 19, 2013 at 8:33 AM, David Miller wrote:
>>>
>>>
>>> So this patch tries to rework the semantics of hard register classes,
>>> such that if a hard register is present in the set it is implied that
>>> the rest of the registers in a multi-register group are present as
>>> well.  So we can add a register class called EVEN_REGS and only have
>>> to set the even register bits.
>>
>>
>> I haven't really looked at your patch, but I just want to say that I
>> think this is the right way to go.
>
> Likewise.  I've certainly worked on ports where this would have been helpful
> in the past.

It could also help simplify existing ports. rs6000 has a few places
where this could be useful (dfp). At least arm
(VFP_REGNO_OK_FOR_DOUBLE), avr, and h8300, too. And ISTR s390 plays
funny subreg tricks to get this right. Finally, it's been discussed on
the gcc@ mailing list for out-of-tree ports a few times in recent
history.

So yes, great if this can be done!

Ciao!
Steven
Richard Sandiford - April 27, 2013, 9:09 a.m.
David Miller <davem@davemloft.net> writes:
> So this patch tries to rework the semantics of hard register classes,
> such that if a hard register is present in the set it is implied that
> the rest of the registers in a multi-register group are present as
> well.  So we can add a register class called EVEN_REGS and only have
> to set the even register bits.

It would be great if this could be made to work.  I think the main
problem is going to be stopping registers from other register files
being included accidentally.  E.g. if on a 32-bit target you store
a 128-bit value in an even GPR, using the GPR class itself ensures
that all four registers are GPRs.  The last even GPR is automatically
disallowed.  If instead we say that only the first register needs to
belong to the class, I assume we'd need both an EVEN_REGS and an
EVEN_REGS_EXCEPT_... class.

I must admit I don't really understand why HARD_REGNO_MODE_OK() is a
problem for IRA and LRA.  Although they work on register classes,
most of the internal calculations are done on register sets,
which can be masked as needed.  E.g. ira_prohibited_class_mode_regs
already takes HARD_REGNO_MODE_OK() into account.  And even if the
register class itself only gives the first register, all the conflict
calculations are still going to need to include all registers.

I'm probably repeating a previous discussion here, sorry.

Richard

Patch

diff --git a/gcc/config/sparc/constraints.md b/gcc/config/sparc/constraints.md
index 5dec3dd..cd082ad 100644
--- a/gcc/config/sparc/constraints.md
+++ b/gcc/config/sparc/constraints.md
@@ -44,6 +44,9 @@ 
 (define_register_constraint "h" "(TARGET_V9 && TARGET_V8PLUS ? I64_REGS : NO_REGS)"
  "64-bit global or out register in V8+ mode")
 
+(define_register_constraint "U" "(TARGET_ARCH32 ? EVEN_REGS : NO_REGS)"
+ "Even-numbered integer register")
+
 ;; Floating-point constant constraints
 
 (define_constraint "G"
@@ -135,51 +138,6 @@ 
       (match_code "mem")
       (match_test "memory_ok_for_ldd (op)")))
 
-;; This awkward register constraint is necessary because it is not
-;; possible to express the "must be even numbered register" condition
-;; using register classes.  The problem is that membership in a
-;; register class requires that all registers of a multi-regno
-;; register be included in the set.  It is add_to_hard_reg_set
-;; and in_hard_reg_set_p which populate and test regsets with these
-;; semantics.
-;;
-;; So this means that we would have to put both the even and odd
-;; register into the register class, which would not restrict things
-;; at all.
-;;
-;; Using a combination of GENERAL_REGS and HARD_REGNO_MODE_OK is not a
-;; full solution either.  In fact, even though IRA uses the macro
-;; HARD_REGNO_MODE_OK to calculate which registers are prohibited from
-;; use in certain modes, it still can allocate an odd hard register
-;; for DImode values.  This is due to how IRA populates the table
-;; ira_useful_class_mode_regs[][].  It suffers from the same problem
-;; as using a register class to describe this restriction.  Namely, it
-;; sets both the odd and even part of an even register pair in the
-;; regset.  Therefore IRA can and will allocate odd registers for
-;; DImode values on 32-bit.
-;;
-;; There are legitimate cases where DImode values can end up in odd
-;; hard registers, the most notable example is argument passing.
-;;
-;; What saves us is reload and the DImode splitters.  Both are
-;; necessary.  The odd register splitters cannot match if, for
-;; example, we have a non-offsetable MEM.  Reload will notice this
-;; case and reload the address into a single hard register.
-;;
-;; The real downfall of this awkward register constraint is that it does
-;; not evaluate to a true register class like a bonafide use of
-;; define_register_constraint would.  This currently means that we cannot
-;; use LRA on Sparc, since the constraint processing of LRA really depends
-;; upon whether an extra constraint is for registers or not.  It uses
-;; REG_CLASS_FROM_CONSTRAINT, and checks it against NO_REGS.
-(define_constraint "U"
- "Pseudo-register or hard even-numbered integer register"
- (and (match_test "TARGET_ARCH32")
-      (match_code "reg")
-      (ior (match_test "REGNO (op) < FIRST_PSEUDO_REGISTER")
-	   (not (match_test "reload_in_progress && reg_renumber [REGNO (op)] < 0")))
-      (match_test "register_ok_for_ldd (op)")))
-
 ;; Equivalent to 'T' but available in 64-bit mode
 (define_memory_constraint "W"
  "Memory reference for 'e' constraint floating-point register"
diff --git a/gcc/config/sparc/sparc.c b/gcc/config/sparc/sparc.c
index 08c2894..08b4136 100644
--- a/gcc/config/sparc/sparc.c
+++ b/gcc/config/sparc/sparc.c
@@ -4492,6 +4492,8 @@  sparc_init_modes (void)
     {
       if (i < 16 && TARGET_V8PLUS)
 	sparc_regno_reg_class[i] = I64_REGS;
+      else if (i < 32 && (i & 1) == 0)
+	sparc_regno_reg_class[i] = EVEN_REGS;
       else if (i < 32 || i == FRAME_POINTER_REGNUM)
 	sparc_regno_reg_class[i] = GENERAL_REGS;
       else if (i < 64)
@@ -10592,12 +10594,13 @@  sparc_rtx_costs (rtx x, int code, int outer_code, int opno ATTRIBUTE_UNUSED,
     }
 }
 
-/* Return true if CLASS is either GENERAL_REGS or I64_REGS.  */
+/* Return true if CLASS is either GENERAL_REGS or I64_REGS or
+   EVEN_REGS.  */
 
 static inline bool
-general_or_i64_p (reg_class_t rclass)
+integer_regclass_p (reg_class_t rclass)
 {
-  return (rclass == GENERAL_REGS || rclass == I64_REGS);
+  return (rclass == GENERAL_REGS || rclass == I64_REGS || rclass == EVEN_REGS);
 }
 
 /* Implement TARGET_REGISTER_MOVE_COST.  */
@@ -10610,8 +10613,8 @@  sparc_register_move_cost (enum machine_mode mode ATTRIBUTE_UNUSED,
 
   if (from == FPCC_REGS || to == FPCC_REGS)
     need_memory = true;
-  else if ((FP_REG_CLASS_P (from) && general_or_i64_p (to))
-	   || (general_or_i64_p (from) && FP_REG_CLASS_P (to)))
+  else if ((FP_REG_CLASS_P (from) && integer_regclass_p (to))
+	   || (integer_regclass_p (from) && FP_REG_CLASS_P (to)))
     {
       if (TARGET_VIS3)
 	{
@@ -11926,7 +11929,7 @@  sparc_secondary_reload (bool in_p, rtx x, reg_class_t rclass_i,
 	 move between EXTRA_FP_REGS and GENERAL_REGS, we will need
 	 an FP_REGS intermediate move.  */
       if ((rclass == EXTRA_FP_REGS && SPARC_INT_REG_P (regno))
-	  || ((general_or_i64_p (rclass)
+	  || ((integer_regclass_p (rclass)
 	       || rclass == GENERAL_OR_FP_REGS)
 	      && SPARC_FP_REG_P (regno)))
 	{
diff --git a/gcc/config/sparc/sparc.h b/gcc/config/sparc/sparc.h
index 6b02b45..a53deb9 100644
--- a/gcc/config/sparc/sparc.h
+++ b/gcc/config/sparc/sparc.h
@@ -858,7 +858,7 @@  extern int sparc_mode_class[];
    ??? Should %fcc[0123] be handled similarly?
 */
 
-enum reg_class { NO_REGS, FPCC_REGS, I64_REGS, GENERAL_REGS, FP_REGS,
+enum reg_class { NO_REGS, FPCC_REGS, EVEN_REGS, I64_REGS, GENERAL_REGS, FP_REGS,
 		 EXTRA_FP_REGS, GENERAL_OR_FP_REGS, GENERAL_OR_EXTRA_FP_REGS,
 		 ALL_REGS, LIM_REG_CLASSES };
 
@@ -867,9 +867,9 @@  enum reg_class { NO_REGS, FPCC_REGS, I64_REGS, GENERAL_REGS, FP_REGS,
 /* Give names of register classes as strings for dump file.  */
 
 #define REG_CLASS_NAMES \
-  { "NO_REGS", "FPCC_REGS", "I64_REGS", "GENERAL_REGS", "FP_REGS",	\
-     "EXTRA_FP_REGS", "GENERAL_OR_FP_REGS", "GENERAL_OR_EXTRA_FP_REGS",	\
-     "ALL_REGS" }
+	{ "NO_REGS", "FPCC_REGS", "EVEN_REGS", "I64_REGS", "GENERAL_REGS", \
+     "FP_REGS", "EXTRA_FP_REGS", "GENERAL_OR_FP_REGS", \
+     "GENERAL_OR_EXTRA_FP_REGS", "ALL_REGS" }
 
 /* Define which registers fit in which classes.
    This is an initializer for a vector of HARD_REG_SET
@@ -878,6 +878,7 @@  enum reg_class { NO_REGS, FPCC_REGS, I64_REGS, GENERAL_REGS, FP_REGS,
 #define REG_CLASS_CONTENTS				\
   {{0, 0, 0, 0},	/* NO_REGS */			\
    {0, 0, 0, 0xf},	/* FPCC_REGS */			\
+   {0x55555555, 0, 0, 0},/* EVEN_REGS */		\
    {0xffff, 0, 0, 0},	/* I64_REGS */			\
    {-1, 0, 0, 0x20},	/* GENERAL_REGS */		\
    {0, -1, 0, 0},	/* FP_REGS */			\
diff --git a/gcc/regs.h b/gcc/regs.h
index 090d6b6..010f552 100644
--- a/gcc/regs.h
+++ b/gcc/regs.h
@@ -351,10 +351,6 @@  in_hard_reg_set_p (const HARD_REG_SET regs, enum machine_mode mode,
   if (!HARD_REGISTER_NUM_P (end_regno - 1))
     return false;
 
-  while (++regno < end_regno)
-    if (!TEST_HARD_REG_BIT (regs, regno))
-      return false;
-
   return true;
 }
 
diff --git a/gcc/reload.c b/gcc/reload.c
index 2546c1b..91a0fc0 100644
--- a/gcc/reload.c
+++ b/gcc/reload.c
@@ -680,8 +680,7 @@  find_valid_class (enum machine_mode outer ATTRIBUTE_UNUSED,
 	    if (HARD_REGNO_MODE_OK (regno, inner))
 	      {
 		good = 1;
-		if (! TEST_HARD_REG_BIT (reg_class_contents[rclass], regno + n)
-		    || ! HARD_REGNO_MODE_OK (regno + n, outer))
+		if (! HARD_REGNO_MODE_OK (regno + n, outer))
 		  bad = 1;
 	      }
 	  }
@@ -1648,19 +1647,20 @@  push_reload (rtx in, rtx out, rtx *inloc, rtx *outloc,
 	    unsigned int nregs = MAX (hard_regno_nregs[regno][inmode],
 				      hard_regno_nregs[regno][outmode]);
 
-	    for (offs = 0; offs < nregs; offs++)
-	      if (fixed_regs[regno + offs]
-		  || ! TEST_HARD_REG_BIT (reg_class_contents[(int) rclass],
-					  regno + offs))
-		break;
-
-	    if (offs == nregs
-		&& (! (refers_to_regno_for_reload_p
-		       (regno, end_hard_regno (inmode, regno), in, (rtx *) 0))
-		    || can_reload_into (in, regno, inmode)))
+	    if (TEST_HARD_REG_BIT (reg_class_contents[(int) rclass], regno))
 	      {
-		rld[i].reg_rtx = gen_rtx_REG (rel_mode, regno);
-		break;
+		for (offs = 0; offs < nregs; offs++)
+		  if (fixed_regs[regno + offs])
+		    break;
+
+		if (offs == nregs
+		    && (! (refers_to_regno_for_reload_p
+			   (regno, end_hard_regno (inmode, regno), in, (rtx *) 0))
+			|| can_reload_into (in, regno, inmode)))
+		  {
+		    rld[i].reg_rtx = gen_rtx_REG (rel_mode, regno);
+		    break;
+		  }
 	      }
 	  }
     }
@@ -2052,18 +2052,19 @@  find_dummy_reload (rtx real_in, rtx real_out, rtx *inloc, rtx *outloc,
 	{
 	  unsigned int i;
 
-	  for (i = 0; i < nwords; i++)
-	    if (! TEST_HARD_REG_BIT (reg_class_contents[(int) rclass],
-				     regno + i)
-		|| fixed_regs[regno + i])
-	      break;
-
-	  if (i == nwords)
+	  if (TEST_HARD_REG_BIT (reg_class_contents[(int) rclass], regno))
 	    {
-	      if (REG_P (real_out))
-		value = real_out;
-	      else
-		value = gen_rtx_REG (outmode, regno);
+	      for (i = 0; i < nwords; i++)
+		if (fixed_regs[regno + i])
+		  break;
+
+	      if (i == nwords)
+		{
+		  if (REG_P (real_out))
+		    value = real_out;
+		  else
+		    value = gen_rtx_REG (outmode, regno);
+		}
 	    }
 	}
 
@@ -2121,12 +2122,7 @@  find_dummy_reload (rtx real_in, rtx real_out, rtx *inloc, rtx *outloc,
 	{
 	  unsigned int i;
 
-	  for (i = 0; i < nwords; i++)
-	    if (! TEST_HARD_REG_BIT (reg_class_contents[(int) rclass],
-				     regno + i))
-	      break;
-
-	  if (i == nwords)
+	  if (TEST_HARD_REG_BIT (reg_class_contents[(int) rclass], regno))
 	    {
 	      /* If we were going to use OUT as the reload reg
 		 and changed our mind, it means OUT is a dummy that
@@ -4635,15 +4631,7 @@  find_reloads (rtx insn, int replace, int ind_levels, int live_known,
 	    && TEST_HARD_REG_BIT (reg_class_contents[rld[i].rclass], regno)
 	    && HARD_REGNO_MODE_OK (regno, rld[i].mode))
 	  {
-	    int nr = hard_regno_nregs[regno][rld[i].mode];
-	    int ok = 1, nri;
-
-	    for (nri = 1; nri < nr; nri ++)
-	      if (! TEST_HARD_REG_BIT (reg_class_contents[rld[i].rclass], regno + nri))
-		ok = 0;
-
-	    if (ok)
-	      rld[i].reg_rtx = dest;
+	    rld[i].reg_rtx = dest;
 	  }
       }
 
diff --git a/gcc/reload1.c b/gcc/reload1.c
index cbb945d..3759f25 100644
--- a/gcc/reload1.c
+++ b/gcc/reload1.c
@@ -1868,7 +1868,6 @@  find_reg (struct insn_chain *chain, int order)
 
   COPY_HARD_REG_SET (not_usable, bad_spill_regs);
   IOR_HARD_REG_SET (not_usable, bad_spill_regs_global);
-  IOR_COMPL_HARD_REG_SET (not_usable, reg_class_contents[rl->rclass]);
 
   CLEAR_HARD_REG_SET (used_by_other_reload);
   for (k = 0; k < order; k++)
@@ -1888,7 +1887,8 @@  find_reg (struct insn_chain *chain, int order)
       unsigned int regno = i;
 #endif
 
-      if (! TEST_HARD_REG_BIT (not_usable, regno)
+      if (TEST_HARD_REG_BIT (reg_class_contents[rl->rclass], regno)
+	  && ! TEST_HARD_REG_BIT (not_usable, regno)
 	  && ! TEST_HARD_REG_BIT (used_by_other_reload, regno)
 	  && HARD_REGNO_MODE_OK (regno, rl->mode))
 	{
@@ -6265,8 +6265,7 @@  allocate_reload_reg (struct insn_chain *chain ATTRIBUTE_UNUSED, int r,
 	      while (nr > 1)
 		{
 		  int regno = regnum + nr - 1;
-		  if (!(TEST_HARD_REG_BIT (reg_class_contents[rclass], regno)
-			&& spill_reg_order[regno] >= 0
+		  if (!(spill_reg_order[regno] >= 0
 			&& reload_reg_free_p (regno, rld[r].opnum,
 					      rld[r].when_needed)))
 		    break;
@@ -6604,10 +6603,9 @@  choose_reload_regs (struct insn_chain *chain)
 			  last_reg = (GET_MODE (last_reg) == mode
 				      ? last_reg : gen_rtx_REG (mode, i));
 
-			  bad_for_class = 0;
-			  for (k = 0; k < nr; k++)
-			    bad_for_class |= ! TEST_HARD_REG_BIT (reg_class_contents[(int) rld[r].rclass],
-								  i+k);
+			  bad_for_class =
+				  (! TEST_HARD_REG_BIT (reg_class_contents[(int) rld[r].rclass],
+							i));
 
 			  /* We found a register that contains the
 			     value we need.  If this register is the
@@ -6731,12 +6729,13 @@  choose_reload_regs (struct insn_chain *chain)
 		  int bad_for_class = 0;
 		  int max_regno = regno + rld[r].nregs;
 
+		  bad_for_class =
+			  (! TEST_HARD_REG_BIT (reg_class_contents[(int) rld[r].rclass],
+						regno));
 		  for (i = regno; i < max_regno; i++)
 		    {
 		      regs_used |= TEST_HARD_REG_BIT (reload_reg_used_at_all,
 						      i);
-		      bad_for_class |= ! TEST_HARD_REG_BIT (reg_class_contents[(int) rld[r].rclass],
-							   i);
 		    }
 
 		  if ((regs_used