This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Explicitly record which registers are inaccessible


This patch tries to fix one thing that has bugged me for a while:
there's no way of explicitly saying that some hard registers are
only available with certain target options.  The usual approach is
to make TARGET_CONDITIONAL_REGISTER_USAGE fix any registers that
don't exist, but this doesn't in itself stop the register from
being used in things like register variables.  The target really
has to make sure that HARD_REGNO_MODE_OK is false for non-existent
registers too.  And that means duplicating the logic in at least
two places.

If HARD_REGNO_MODE_OK doesn't check for inaccessible registers, those
registers will still seem to be both legitimate register_operands and
legitimate register variables.  If they are then used as register variables,
the likelihood is that we'll either ICE or silently generate code to
access something that doesn't exist.

The patch adds a new HARD_REG_SET, accessible_regs, to say which
registers can be accessed.  It explicitly checks whether register
variables belong to this set, and raises a (hopefully) more user-
friendly error message if not.  Also, to catch internal compiler bugs,
it makes sure that inaccessible registers are never treated as register
operands.

The last bit is indirect, via a new HARD_REG_SET called operand_reg_set.
And this set is the reason why I'm sending the patch now.  The MIPS16 port
has always had a problem with the HI and LO registers: they can only be
set by multiplication and division instructions, and only read by MFHI
and MFLO.  Unlike normal MIPS, there are no MTHI and MTLO instructions.

However, as for normal MIPS, we still exposed HI and LO as allocatable
registers.  This caused real problems if the register allocator wanted
to move something _into_ HI and LO.  In the old days, there was code to
move things into LO by faking an appropriate multiplication, but it
didn't handle the general case of a full HI/LO reload, and it was
something that we never wanted to generate anyway.  But because the
multiplication and division latencies are high, we do still want to
model the multiplication/division and MFHI/MFLO as separately-schedulable
RTL instructions.

For a while we tried to work around this by making the cost of moving
things into LO and HI very high.  But that was always a hack.  Then,
when IRA's cover classes came along, I went for a kind-of get-out: don't
include HI and LO in cover classes.  That was still a bit hackish, but
felt cleaner than relying on costs.  Now that we use pressure classes
instead (a good thing), I'm finally going to try to fix this "properly".
And that means (a) fixing HI and LO and (b) stopping them from being
treated as register operands.  (b) is important because if we start
out with this (valid) instruction before reload:

    (set (reg:SI lo) (mult:SI (reg:SI A) (reg:SI B)))

and we find that the multiplication is a constant, we'll try to match:

    (set (reg:SI lo) (const_int RESULT))

And because this is before reload, that pattern matches the normal
move patterns.  So we end up with unintended (and unimplementable)
moves into the registers even when they are fixed.

(Normally we'd expect the tree-level optimisers to optimise cases like
this, but we shouldn't rely on that, and it seems there are times when
they don't.)

An alternative to (b) would be to only expose HI and LO after reload.
It just feels like that's working around a limitation in the target
description rather than a real fix.

Tested on x86_64-linux-gnu and mips64-linux-gnu.  OK to install?

Richard


gcc/
	* hard-reg-set.h (target_hard_regs): Add x_accessible_reg_set
	and x_operand_reg_set.
	(accessible_reg_set, operand_reg_set): New macros.
	* reginfo.c (init_reg_sets): Initialize accessible_reg_set and
	operand_reg_set.
	(saved_accessible_reg_set, saved_operand_reg_set): New variables.
	(save_register_info): Save them.
	(restore_register_info): Restore them.
	(init_reg_sets_1): Limit operand_reg_set to accessible_reg_set.
	Remove NO_REGS registers from operand_reg_set.  Treat members
	of operand_reg_set as fixed.
	* recog.c (general_operand): Check operand_reg_set rather than
	NO_REGS.
	(register_operand, nonmemory_operand): Likewise.
	* varasm.c (make_decl_rtl): Always use DECL_MODE as the mode of
	register variables.  Check accessible_reg_set and operand_reg_set.
	* config/mips/mips.c (mips_conditional_register_usage): Remove
	inaccessible register from accessible_reg_set, rather than just
	making them fixed.

gcc/testsuite/
	* gcc.target/mips/mips.exp (mips-dg-options): Make -mno-dsp
	imply -mno-dspr2.
	* gcc.target/mips/no-dsp-1.c: New test.
	* gcc.target/mips/soft-float-1.c: Likewise.

Index: gcc/hard-reg-set.h
===================================================================
--- gcc/hard-reg-set.h	2011-09-25 17:36:28.000000000 +0100
+++ gcc/hard-reg-set.h	2011-09-25 18:13:47.000000000 +0100
@@ -583,6 +583,13 @@ #define EXECUTE_IF_SET_IN_HARD_REG_SET(S
 extern char global_regs[FIRST_PSEUDO_REGISTER];
 
 struct target_hard_regs {
+  /* The set of registers that actually exist on the current target.  */
+  HARD_REG_SET x_accessible_reg_set;
+
+  /* The set of registers that should be considered to be register
+     operands.  It is a subset of x_accessible_reg_set.  */
+  HARD_REG_SET x_operand_reg_set;
+
   /* Indexed by hard register number, contains 1 for registers
      that are fixed use (stack pointer, pc, frame pointer, etc.;.
      These are the registers that cannot be used to allocate
@@ -659,6 +666,10 @@ struct target_hard_regs {
 #define this_target_hard_regs (&default_target_hard_regs)
 #endif
 
+#define accessible_reg_set \
+  (this_target_hard_regs->x_accessible_reg_set)
+#define operand_reg_set \
+  (this_target_hard_regs->x_operand_reg_set)
 #define fixed_regs \
   (this_target_hard_regs->x_fixed_regs)
 #define fixed_reg_set \
Index: gcc/reginfo.c
===================================================================
--- gcc/reginfo.c	2011-09-25 17:36:27.000000000 +0100
+++ gcc/reginfo.c	2011-09-25 18:15:18.000000000 +0100
@@ -189,6 +189,9 @@ init_reg_sets (void)
   memcpy (reg_alloc_order, initial_reg_alloc_order, sizeof reg_alloc_order);
 #endif
   memcpy (reg_names, initial_reg_names, sizeof reg_names);
+
+  SET_HARD_REG_SET (accessible_reg_set);
+  SET_HARD_REG_SET (operand_reg_set);
 }
 
 /* Initialize may_move_cost and friends for mode M.  */
@@ -289,6 +292,8 @@ init_move_cost (enum machine_mode m)
 static char saved_call_really_used_regs[FIRST_PSEUDO_REGISTER];
 #endif
 static const char *saved_reg_names[FIRST_PSEUDO_REGISTER];
+static HARD_REG_SET saved_accessible_reg_set;
+static HARD_REG_SET saved_operand_reg_set;
 
 /* Save the register information.  */
 void
@@ -312,6 +317,8 @@ save_register_info (void)
   /* And similarly for reg_names.  */
   gcc_assert (sizeof reg_names == sizeof saved_reg_names);
   memcpy (saved_reg_names, reg_names, sizeof reg_names);
+  COPY_HARD_REG_SET (saved_accessible_reg_set, accessible_reg_set);
+  COPY_HARD_REG_SET (saved_operand_reg_set, operand_reg_set);
 }
 
 /* Restore the register information.  */
@@ -327,6 +334,8 @@ restore_register_info (void)
 #endif
 
   memcpy (reg_names, saved_reg_names, sizeof reg_names);
+  COPY_HARD_REG_SET (accessible_reg_set, saved_accessible_reg_set);
+  COPY_HARD_REG_SET (operand_reg_set, saved_operand_reg_set);
 }
 
 /* After switches have been processed, which perhaps alter
@@ -452,8 +461,27 @@ init_reg_sets_1 (void)
   else
     CLEAR_REG_SET (regs_invalidated_by_call_regset);
 
+  AND_HARD_REG_SET (operand_reg_set, accessible_reg_set);
   for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
     {
+      /* As a special exception, registers whose class is NO_REGS are
+	 not accepted by `register_operand'.  The reason for this change
+	 is to allow the representation of special architecture artifacts
+	 (such as a condition code register) without extending the rtl
+	 definitions.  Since registers of class NO_REGS cannot be used
+	 as registers in any case where register classes are examined,
+	 it is better to apply this exception in a target-independent way.  */
+      if (REGNO_REG_CLASS (i) == NO_REGS)
+	CLEAR_HARD_REG_BIT (operand_reg_set, i);
+
+      /* If a register is too limited to be treated as a register operand,
+	 then it should never be allocated to a psuedo.  */
+      if (!TEST_HARD_REG_BIT (operand_reg_set, i))
+	{
+	  fixed_regs[i] = 1;
+	  call_used_regs[i] = 1;
+	}
+
       /* call_used_regs must include fixed_regs.  */
       gcc_assert (!fixed_regs[i] || call_used_regs[i]);
 #ifdef CALL_REALLY_USED_REGISTERS
Index: gcc/recog.c
===================================================================
--- gcc/recog.c	2011-09-25 17:36:28.000000000 +0100
+++ gcc/recog.c	2011-09-25 18:13:47.000000000 +0100
@@ -925,10 +925,7 @@ next_insn_tests_no_inequality (rtx insn)
    it has.
 
    The main use of this function is as a predicate in match_operand
-   expressions in the machine description.
-
-   For an explanation of this function's behavior for registers of
-   class NO_REGS, see the comment for `register_operand'.  */
+   expressions in the machine description.  */
 
 int
 general_operand (rtx op, enum machine_mode mode)
@@ -998,9 +995,8 @@ general_operand (rtx op, enum machine_mo
     }
 
   if (code == REG)
-    /* A register whose class is NO_REGS is not a general operand.  */
     return (REGNO (op) >= FIRST_PSEUDO_REGISTER
-	    || REGNO_REG_CLASS (REGNO (op)) != NO_REGS);
+	    || in_hard_reg_set_p (operand_reg_set, GET_MODE (op), REGNO (op)));
 
   if (code == MEM)
     {
@@ -1033,15 +1029,7 @@ address_operand (rtx op, enum machine_mo
    If MODE is VOIDmode, accept a register in any mode.
 
    The main use of this function is as a predicate in match_operand
-   expressions in the machine description.
-
-   As a special exception, registers whose class is NO_REGS are
-   not accepted by `register_operand'.  The reason for this change
-   is to allow the representation of special architecture artifacts
-   (such as a condition code register) without extending the rtl
-   definitions.  Since registers of class NO_REGS cannot be used
-   as registers in any case where register classes are examined,
-   it is most consistent to keep this function from accepting them.  */
+   expressions in the machine description.  */
 
 int
 register_operand (rtx op, enum machine_mode mode)
@@ -1080,11 +1068,10 @@ register_operand (rtx op, enum machine_m
       op = sub;
     }
 
-  /* We don't consider registers whose class is NO_REGS
-     to be a register operand.  */
   return (REG_P (op)
 	  && (REGNO (op) >= FIRST_PSEUDO_REGISTER
-	      || REGNO_REG_CLASS (REGNO (op)) != NO_REGS));
+	      || in_hard_reg_set_p (operand_reg_set,
+				    GET_MODE (op), REGNO (op))));
 }
 
 /* Return 1 for a register in Pmode; ignore the tested mode.  */
@@ -1203,11 +1190,10 @@ nonmemory_operand (rtx op, enum machine_
       op = SUBREG_REG (op);
     }
 
-  /* We don't consider registers whose class is NO_REGS
-     to be a register operand.  */
   return (REG_P (op)
 	  && (REGNO (op) >= FIRST_PSEUDO_REGISTER
-	      || REGNO_REG_CLASS (REGNO (op)) != NO_REGS));
+	      || in_hard_reg_set_p (operand_reg_set,
+				    GET_MODE (op), REGNO (op))));
 }
 
 /* Return 1 if OP is a valid operand that stands for pushing a
Index: gcc/varasm.c
===================================================================
--- gcc/varasm.c	2011-09-25 17:36:28.000000000 +0100
+++ gcc/varasm.c	2011-09-25 18:13:47.000000000 +0100
@@ -1198,16 +1198,23 @@ make_decl_rtl (tree decl)
   else if (TREE_CODE (decl) != FUNCTION_DECL && DECL_REGISTER (decl))
     {
       const char *asmspec = name+1;
+      enum machine_mode mode = DECL_MODE (decl);
       reg_number = decode_reg_name (asmspec);
       /* First detect errors in declaring global registers.  */
       if (reg_number == -1)
 	error ("register name not specified for %q+D", decl);
       else if (reg_number < 0)
 	error ("invalid register name for %q+D", decl);
-      else if (TYPE_MODE (TREE_TYPE (decl)) == BLKmode)
+      else if (mode == BLKmode)
 	error ("data type of %q+D isn%'t suitable for a register",
 	       decl);
-      else if (! HARD_REGNO_MODE_OK (reg_number, TYPE_MODE (TREE_TYPE (decl))))
+      else if (!in_hard_reg_set_p (accessible_reg_set, mode, reg_number))
+	error ("the register specified for %q+D cannot be accessed"
+	       " by the current target", decl);
+      else if (!in_hard_reg_set_p (operand_reg_set, mode, reg_number))
+	error ("the register specified for %q+D is not general enough"
+	       " to be used as a register variable", decl);
+      else if (!HARD_REGNO_MODE_OK (reg_number, mode))
 	error ("register specified for %q+D isn%'t suitable for data type",
                decl);
       /* Now handle properly declared static register variables.  */
@@ -1230,7 +1237,7 @@ make_decl_rtl (tree decl)
 	     confused with that register and be eliminated.  This usage is
 	     somewhat suspect...  */
 
-	  SET_DECL_RTL (decl, gen_rtx_raw_REG (DECL_MODE (decl), reg_number));
+	  SET_DECL_RTL (decl, gen_rtx_raw_REG (mode, reg_number));
 	  ORIGINAL_REGNO (DECL_RTL (decl)) = reg_number;
 	  REG_USERVAR_P (DECL_RTL (decl)) = 1;
 
@@ -1242,7 +1249,7 @@ make_decl_rtl (tree decl)
 	      name = IDENTIFIER_POINTER (DECL_NAME (decl));
 	      ASM_DECLARE_REGISTER_GLOBAL (asm_out_file, decl, reg_number, name);
 #endif
-	      nregs = hard_regno_nregs[reg_number][DECL_MODE (decl)];
+	      nregs = hard_regno_nregs[reg_number][mode];
 	      while (nregs > 0)
 		globalize_reg (decl, reg_number + --nregs);
 	    }
Index: gcc/config/mips/mips.c
===================================================================
--- gcc/config/mips/mips.c	2011-09-25 17:36:27.000000000 +0100
+++ gcc/config/mips/mips.c	2011-09-25 18:13:47.000000000 +0100
@@ -15807,31 +15807,26 @@ mips_conditional_register_usage (void)
       global_regs[CCDSP_PO_REGNUM] = 1;
       global_regs[CCDSP_SC_REGNUM] = 1;
     }
-  else 
-    {
-      int regno;
+  else
+    AND_COMPL_HARD_REG_SET (accessible_reg_set,
+			    reg_class_contents[(int) DSP_ACC_REGS]);
 
-      for (regno = DSP_ACC_REG_FIRST; regno <= DSP_ACC_REG_LAST; regno++)
-	fixed_regs[regno] = call_used_regs[regno] = 1;
-    }
   if (!TARGET_HARD_FLOAT)
     {
-      int regno;
-
-      for (regno = FP_REG_FIRST; regno <= FP_REG_LAST; regno++)
-	fixed_regs[regno] = call_used_regs[regno] = 1;
-      for (regno = ST_REG_FIRST; regno <= ST_REG_LAST; regno++)
-	fixed_regs[regno] = call_used_regs[regno] = 1;
+      AND_COMPL_HARD_REG_SET (accessible_reg_set,
+			      reg_class_contents[(int) FP_REGS]);
+      AND_COMPL_HARD_REG_SET (accessible_reg_set,
+			      reg_class_contents[(int) ST_REGS]);
     }
-  else if (! ISA_HAS_8CC)
+  else if (!ISA_HAS_8CC)
     {
-      int regno;
-
       /* We only have a single condition-code register.  We implement
 	 this by fixing all the condition-code registers and generating
 	 RTL that refers directly to ST_REG_FIRST.  */
-      for (regno = ST_REG_FIRST; regno <= ST_REG_LAST; regno++)
-	fixed_regs[regno] = call_used_regs[regno] = 1;
+      AND_COMPL_HARD_REG_SET (accessible_reg_set,
+			      reg_class_contents[(int) ST_REGS]);
+      SET_HARD_REG_BIT (accessible_reg_set, FPSW_REGNUM);
+      fixed_regs[FPSW_REGNUM] = call_used_regs[FPSW_REGNUM] = 1;
     }
   /* In MIPS16 mode, we permit the $t temporary registers to be used
      for reload.  We prohibit the unused $s registers, since they
Index: gcc/testsuite/gcc.target/mips/mips.exp
===================================================================
--- gcc/testsuite/gcc.target/mips/mips.exp	2011-09-25 17:36:27.000000000 +0100
+++ gcc/testsuite/gcc.target/mips/mips.exp	2011-09-25 18:13:47.000000000 +0100
@@ -810,6 +810,10 @@ proc mips-dg-finish {} {
 #            |                           |
 #         -mexplicit-relocs           -mno-explicit-relocs
 #            |                           |
+#         -mdspr2                     -mno-dspr2
+#            |                           |
+#         -mdsp                       -mno-dsp
+#            |                           |
 #            +-- gp, abi & arch ---------+
 #
 # For these purposes, the "gp", "abi" & "arch" option groups are treated
@@ -1092,7 +1096,6 @@ proc mips-dg-options { args } {
 		mips_make_test_option options "-mfp32"
 	    }
 	    mips_make_test_option options "-mno-dsp"
-	    mips_make_test_option options "-mno-dspr2"
 	}
 	unset arch
 	unset isa
@@ -1100,6 +1103,7 @@ proc mips-dg-options { args } {
     }
 
     # Handle dependencies between options on the right of the diagram.
+    mips_option_dependency options "-mno-dsp" "-mno-dspr2"
     mips_option_dependency options "-mno-explicit-relocs" "-mgpopt"
     switch -- [mips_test_option options small-data] {
 	"" -
Index: gcc/testsuite/gcc.target/mips/no-dsp-1.c
===================================================================
--- /dev/null	2011-09-25 11:09:27.005839516 +0100
+++ gcc/testsuite/gcc.target/mips/no-dsp-1.c	2011-09-25 18:13:47.000000000 +0100
@@ -0,0 +1,7 @@
+/* { dg-options "-mno-dsp" } */
+
+void
+foo (void)
+{
+  register int x asm ("$ac1hi"); /* { dg-error "cannot be accessed" } */
+}
Index: gcc/testsuite/gcc.target/mips/soft-float-1.c
===================================================================
--- /dev/null	2011-09-25 11:09:27.005839516 +0100
+++ gcc/testsuite/gcc.target/mips/soft-float-1.c	2011-09-25 18:13:47.000000000 +0100
@@ -0,0 +1,7 @@
+/* { dg-options "-msoft-float" } */
+
+void
+foo (void)
+{
+  register float x asm ("$f0"); /* { dg-error "cannot be accessed" } */
+}


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]