Fix ARM ICE for register var asm ("pc") (PR target/60606)

Fix ARM ICE for register var asm ("pc") (PR target/60606)

[Joseph S. Myers]

PR 60606 reports an ICE on ARM when declaring a register variable in
register pc.  Discussion in that bug suggests this usage should be
considered invalid and give an error.  It turns out similar ICEs also
occur (after errors) for other cases of invalid register variables.

This patch fixes at least the original bug and others I observed in
the course of fixing it (there may well be other cases of registers,
on ARM and elsewhere, that still create such ICEs).  To make pc
invalid for register variables, arm_regno_class is changed to return
NO_REGS for PC_REGNUM (which is consistent with REG_CLASS_CONTENTS).
Testing the scope of the bug showed similar issues for cc in Thumb
mode; that is made invalid by making arm_hard_regno_mode_ok disallow
it for non-MODE_CC modes (i.e. modes that might apply to a variable).
To avoid ICEs after errors, make_decl_rtl is make to clear
DECL_ASSEMBLER_NAME and DECL_HARD_REGISTER when a hard register
specification turns out to be invalid.  cfgexpand.c:expand_one_var is
then made to call expand_one_error_var in this case, consistent with
other cases of erroneous variables.

Tested with no regressions for cross to arm-none-eabi (it also fixes
failures of gcc.dg/noncompile/920507-1.c, which is PR 61330).  OK to
commit?

2014-08-22  Joseph Myers  <joseph@codesourcery.com>

	PR target/60606
	PR target/61330
	* varasm.c (make_decl_rtl): Clear DECL_ASSEMBLER_NAME and
	DECL_HARD_REGISTER and return for invalid register specifications.
	* cfgexpand.c (expand_one_var): If expand_one_hard_reg_var clears
	DECL_HARD_REGISTER, call expand_one_error_var.
	* config/arm/arm.c (arm_hard_regno_mode_ok): Do not allow
	CC_REGNUM with non-MODE_CC modes.
	(arm_regno_class): Return NO_REGS for PC_REGNUM.

2014-08-22  Joseph Myers  <joseph@codesourcery.com>

	PR target/60606
	PR target/61330
	* gcc.dg/torture/pr60606-1.c, gcc.target/arm/pr60606-2.c,
	gcc.target/arm/pr60606-3.c, gcc.target/arm/pr60606-4.c: New tests.

Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c	(revision 214225)
+++ gcc/cfgexpand.c	(working copy)
@@ -1307,7 +1307,12 @@ expand_one_var (tree var, bool toplevel, bool real
   else if (TREE_CODE (var) == VAR_DECL && DECL_HARD_REGISTER (var))
     {
       if (really_expand)
-        expand_one_hard_reg_var (var);
+	{
+	  expand_one_hard_reg_var (var);
+	  if (!DECL_HARD_REGISTER (var))
+	    /* Invalid register specification.  */
+	    expand_one_error_var (var);
+	}
     }
   else if (use_register_for_decl (var))
     {
Index: gcc/testsuite/gcc.dg/torture/pr60606-1.c
===================================================================
--- gcc/testsuite/gcc.dg/torture/pr60606-1.c	(revision 0)
+++ gcc/testsuite/gcc.dg/torture/pr60606-1.c	(revision 0)
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-ffat-lto-objects" } */
+
+int
+f (void)
+{
+  register unsigned int r asm ("no-such-register"); /* { dg-error "invalid register name" } */
+  return r;
+}
Index: gcc/testsuite/gcc.target/arm/pr60606-2.c
===================================================================
--- gcc/testsuite/gcc.target/arm/pr60606-2.c	(revision 0)
+++ gcc/testsuite/gcc.target/arm/pr60606-2.c	(revision 0)
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-O" } */
+
+int
+f (void)
+{
+  register unsigned pc asm ("pc"); /* { dg-error "not general enough" } */
+  
+  return pc > 0x12345678;
+}
Index: gcc/testsuite/gcc.target/arm/pr60606-3.c
===================================================================
--- gcc/testsuite/gcc.target/arm/pr60606-3.c	(revision 0)
+++ gcc/testsuite/gcc.target/arm/pr60606-3.c	(revision 0)
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-O" } */
+
+int
+f (void)
+{
+  register unsigned int r asm ("cc"); /* { dg-error "not general enough|suitable for data type" } */
+  return r;
+}
Index: gcc/testsuite/gcc.target/arm/pr60606-4.c
===================================================================
--- gcc/testsuite/gcc.target/arm/pr60606-4.c	(revision 0)
+++ gcc/testsuite/gcc.target/arm/pr60606-4.c	(revision 0)
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-O" } */
+
+int
+f (void)
+{
+  register unsigned int r[50] asm ("r1"); /* { dg-error "suitable for a register" } */
+  return r[1];
+}
Index: gcc/config/arm/arm.c
===================================================================
--- gcc/config/arm/arm.c	(revision 214225)
+++ gcc/config/arm/arm.c	(working copy)
@@ -22969,6 +22969,9 @@ arm_hard_regno_mode_ok (unsigned int regno, enum m
 	    || (TARGET_HARD_FLOAT && TARGET_VFP
 		&& regno == VFPCC_REGNUM));
 
+  if (regno == CC_REGNUM && GET_MODE_CLASS (mode) != MODE_CC)
+    return false;
+
   if (TARGET_THUMB1)
     /* For the Thumb we only allow values bigger than SImode in
        registers 0 - 6, so that there is always a second low
@@ -23065,6 +23068,9 @@ arm_modes_tieable_p (enum machine_mode mode1, enum
 enum reg_class
 arm_regno_class (int regno)
 {
+  if (regno == PC_REGNUM)
+    return NO_REGS;
+
   if (TARGET_THUMB1)
     {
       if (regno == STACK_POINTER_REGNUM)
Index: gcc/varasm.c
===================================================================
--- gcc/varasm.c	(revision 214225)
+++ gcc/varasm.c	(working copy)
@@ -1372,6 +1372,11 @@ make_decl_rtl (tree decl)
 	  /* As a register variable, it has no section.  */
 	  return;
 	}
+      /* Avoid internal errors from invalid register
+	 specifications.  */
+      SET_DECL_ASSEMBLER_NAME (decl, NULL_TREE);
+      DECL_HARD_REGISTER (decl) = 0;
+      return;
     }
   /* Now handle ordinary static variables and functions (in memory).
      Also handle vars declared register invalidly.  */

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: Fix ARM ICE for register var asm ("pc") (PR target/60606)

[Richard Henderson]

On 08/22/2014 02:14 PM, Joseph S. Myers wrote:
> Tested with no regressions for cross to arm-none-eabi (it also fixes
> failures of gcc.dg/noncompile/920507-1.c, which is PR 61330).  OK to
> commit?
> 
> 2014-08-22  Joseph Myers  <joseph@codesourcery.com>
> 
> 	PR target/60606
> 	PR target/61330
> 	* varasm.c (make_decl_rtl): Clear DECL_ASSEMBLER_NAME and
> 	DECL_HARD_REGISTER and return for invalid register specifications.
> 	* cfgexpand.c (expand_one_var): If expand_one_hard_reg_var clears
> 	DECL_HARD_REGISTER, call expand_one_error_var.
> 	* config/arm/arm.c (arm_hard_regno_mode_ok): Do not allow
> 	CC_REGNUM with non-MODE_CC modes.
> 	(arm_regno_class): Return NO_REGS for PC_REGNUM.
> 
> 2014-08-22  Joseph Myers  <joseph@codesourcery.com>
> 
> 	PR target/60606
> 	PR target/61330
> 	* gcc.dg/torture/pr60606-1.c, gcc.target/arm/pr60606-2.c,
> 	gcc.target/arm/pr60606-3.c, gcc.target/arm/pr60606-4.c: New tests.

Ok.


r~

Re: Fix ARM ICE for register var asm ("pc") (PR target/60606)

[Alan Lawrence]

We've just noticed this patch causes an ICE in 
gcc.c-torture/execute/scal-to-vec1.c at -O3 when running with -fPIC on 
arm-none-linux-gnueabi and arm-none-linux-gnueabihf; test logs:

spawn /tmp/alan/buildarm-none-linux-gnueabi/obj/gcc4/gcc/xgcc -B/tmp/alan/builda
rm-none-linux-gnueabi/obj/gcc4/gcc/ /work/alan/src/gcc/gcc/testsuite/gcc.c-tortu
re/execute/scal-to-vec1.c -fno-diagnostics-show-caret -fdiagnostics-color=never
-O3 -fomit-frame-pointer -w -lm -fPIC -o ./scal-to-vec1.exe
/work/alan/src/gcc/gcc/testsuite/gcc.c-torture/execute/scal-to-vec1.c: In functi
on 'main':
/work/alan/src/gcc/gcc/testsuite/gcc.c-torture/execute/scal-to-vec1.c:86:1: inte
rnal compiler error: in split_reg, at lra-constraints.c:4805
0x8b4cc7 split_reg
         /work/alan/src/gcc/gcc/lra-constraints.c:4805
0x8b4f52 split_if_necessary
         /work/alan/src/gcc/gcc/lra-constraints.c:4901
0x8b7d2c inherit_in_ebb
         /work/alan/src/gcc/gcc/lra-constraints.c:5395
0x8b8786 lra_inheritance()
         /work/alan/src/gcc/gcc/lra-constraints.c:5560
0x8abd84 lra(_IO_FILE*)
         /work/alan/src/gcc/gcc/lra.c:2218
0x8705f6 do_reload
         /work/alan/src/gcc/gcc/ira.c:5310
0x8705f6 execute
         /work/alan/src/gcc/gcc/ira.c:5469

Complete list (for both arm-none-linux-gnueabi and arm-none-linux-gnueabihf, 
both -fPIC):

FAIL: gcc.c-torture/execute/scal-to-vec1.c   -O3 -fomit-frame-pointer  (internal 
compiler error)
FAIL: gcc.c-torture/execute/scal-to-vec1.c   -O3 -fomit-frame-pointer  (test for 
excess errors)
FAIL: gcc.c-torture/execute/scal-to-vec1.c   -O3 -fomit-frame-pointer 
-funroll-loops  (internal compiler error)
FAIL: gcc.c-torture/execute/scal-to-vec1.c   -O3 -fomit-frame-pointer 
-funroll-loops  (test for excess errors)
FAIL: gcc.c-torture/execute/scal-to-vec1.c   -O3 -fomit-frame-pointer 
-funroll-all-loops -finline-functions  (internal compiler error)
FAIL: gcc.c-torture/execute/scal-to-vec1.c   -O3 -fomit-frame-pointer 
-funroll-all-loops -finline-functions  (test for excess errors)
FAIL: gcc.c-torture/execute/scal-to-vec1.c   -O3 -g  (internal compiler error)
FAIL: gcc.c-torture/execute/scal-to-vec1.c   -O3 -g  (test for excess errors)

Passing without -fPIC.

--Alan

Joseph S. Myers wrote:
> PR 60606 reports an ICE on ARM when declaring a register variable in
> register pc.  Discussion in that bug suggests this usage should be
> considered invalid and give an error.  It turns out similar ICEs also
> occur (after errors) for other cases of invalid register variables.
> 
> This patch fixes at least the original bug and others I observed in
> the course of fixing it (there may well be other cases of registers,
> on ARM and elsewhere, that still create such ICEs).  To make pc
> invalid for register variables, arm_regno_class is changed to return
> NO_REGS for PC_REGNUM (which is consistent with REG_CLASS_CONTENTS).
> Testing the scope of the bug showed similar issues for cc in Thumb
> mode; that is made invalid by making arm_hard_regno_mode_ok disallow
> it for non-MODE_CC modes (i.e. modes that might apply to a variable).
> To avoid ICEs after errors, make_decl_rtl is make to clear
> DECL_ASSEMBLER_NAME and DECL_HARD_REGISTER when a hard register
> specification turns out to be invalid.  cfgexpand.c:expand_one_var is
> then made to call expand_one_error_var in this case, consistent with
> other cases of erroneous variables.
> 
> Tested with no regressions for cross to arm-none-eabi (it also fixes
> failures of gcc.dg/noncompile/920507-1.c, which is PR 61330).  OK to
> commit?
> 
> 2014-08-22  Joseph Myers  <joseph@codesourcery.com>
> 
> 	PR target/60606
> 	PR target/61330
> 	* varasm.c (make_decl_rtl): Clear DECL_ASSEMBLER_NAME and
> 	DECL_HARD_REGISTER and return for invalid register specifications.
> 	* cfgexpand.c (expand_one_var): If expand_one_hard_reg_var clears
> 	DECL_HARD_REGISTER, call expand_one_error_var.
> 	* config/arm/arm.c (arm_hard_regno_mode_ok): Do not allow
> 	CC_REGNUM with non-MODE_CC modes.
> 	(arm_regno_class): Return NO_REGS for PC_REGNUM.
> 
> 2014-08-22  Joseph Myers  <joseph@codesourcery.com>
> 
> 	PR target/60606
> 	PR target/61330
> 	* gcc.dg/torture/pr60606-1.c, gcc.target/arm/pr60606-2.c,
> 	gcc.target/arm/pr60606-3.c, gcc.target/arm/pr60606-4.c: New tests.
> 
> Index: gcc/cfgexpand.c
> ===================================================================
> --- gcc/cfgexpand.c	(revision 214225)
> +++ gcc/cfgexpand.c	(working copy)
> @@ -1307,7 +1307,12 @@ expand_one_var (tree var, bool toplevel, bool real
>    else if (TREE_CODE (var) == VAR_DECL && DECL_HARD_REGISTER (var))
>      {
>        if (really_expand)
> -        expand_one_hard_reg_var (var);
> +	{
> +	  expand_one_hard_reg_var (var);
> +	  if (!DECL_HARD_REGISTER (var))
> +	    /* Invalid register specification.  */
> +	    expand_one_error_var (var);
> +	}
>      }
>    else if (use_register_for_decl (var))
>      {
> Index: gcc/testsuite/gcc.dg/torture/pr60606-1.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/torture/pr60606-1.c	(revision 0)
> +++ gcc/testsuite/gcc.dg/torture/pr60606-1.c	(revision 0)
> @@ -0,0 +1,9 @@
> +/* { dg-do compile } */
> +/* { dg-options "-ffat-lto-objects" } */
> +
> +int
> +f (void)
> +{
> +  register unsigned int r asm ("no-such-register"); /* { dg-error "invalid register name" } */
> +  return r;
> +}
> Index: gcc/testsuite/gcc.target/arm/pr60606-2.c
> ===================================================================
> --- gcc/testsuite/gcc.target/arm/pr60606-2.c	(revision 0)
> +++ gcc/testsuite/gcc.target/arm/pr60606-2.c	(revision 0)
> @@ -0,0 +1,10 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O" } */
> +
> +int
> +f (void)
> +{
> +  register unsigned pc asm ("pc"); /* { dg-error "not general enough" } */
> +  
> +  return pc > 0x12345678;
> +}
> Index: gcc/testsuite/gcc.target/arm/pr60606-3.c
> ===================================================================
> --- gcc/testsuite/gcc.target/arm/pr60606-3.c	(revision 0)
> +++ gcc/testsuite/gcc.target/arm/pr60606-3.c	(revision 0)
> @@ -0,0 +1,9 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O" } */
> +
> +int
> +f (void)
> +{
> +  register unsigned int r asm ("cc"); /* { dg-error "not general enough|suitable for data type" } */
> +  return r;
> +}
> Index: gcc/testsuite/gcc.target/arm/pr60606-4.c
> ===================================================================
> --- gcc/testsuite/gcc.target/arm/pr60606-4.c	(revision 0)
> +++ gcc/testsuite/gcc.target/arm/pr60606-4.c	(revision 0)
> @@ -0,0 +1,9 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O" } */
> +
> +int
> +f (void)
> +{
> +  register unsigned int r[50] asm ("r1"); /* { dg-error "suitable for a register" } */
> +  return r[1];
> +}
> Index: gcc/config/arm/arm.c
> ===================================================================
> --- gcc/config/arm/arm.c	(revision 214225)
> +++ gcc/config/arm/arm.c	(working copy)
> @@ -22969,6 +22969,9 @@ arm_hard_regno_mode_ok (unsigned int regno, enum m
>  	    || (TARGET_HARD_FLOAT && TARGET_VFP
>  		&& regno == VFPCC_REGNUM));
>  
> +  if (regno == CC_REGNUM && GET_MODE_CLASS (mode) != MODE_CC)
> +    return false;
> +
>    if (TARGET_THUMB1)
>      /* For the Thumb we only allow values bigger than SImode in
>         registers 0 - 6, so that there is always a second low
> @@ -23065,6 +23068,9 @@ arm_modes_tieable_p (enum machine_mode mode1, enum
>  enum reg_class
>  arm_regno_class (int regno)
>  {
> +  if (regno == PC_REGNUM)
> +    return NO_REGS;
> +
>    if (TARGET_THUMB1)
>      {
>        if (regno == STACK_POINTER_REGNUM)
> Index: gcc/varasm.c
> ===================================================================
> --- gcc/varasm.c	(revision 214225)
> +++ gcc/varasm.c	(working copy)
> @@ -1372,6 +1372,11 @@ make_decl_rtl (tree decl)
>  	  /* As a register variable, it has no section.  */
>  	  return;
>  	}
> +      /* Avoid internal errors from invalid register
> +	 specifications.  */
> +      SET_DECL_ASSEMBLER_NAME (decl, NULL_TREE);
> +      DECL_HARD_REGISTER (decl) = 0;
> +      return;
>      }
>    /* Now handle ordinary static variables and functions (in memory).
>       Also handle vars declared register invalidly.  */
> 

Re: Fix ARM ICE for register var asm ("pc") (PR target/60606)

[Joseph S. Myers]

On Wed, 17 Sep 2014, Alan Lawrence wrote:

> We've just noticed this patch causes an ICE in
> gcc.c-torture/execute/scal-to-vec1.c at -O3 when running with -fPIC on
> arm-none-linux-gnueabi and arm-none-linux-gnueabihf; test logs:

Which part causes the ICE?  The arm_hard_regno_mode_ok change relating to 
modes assigned to CC_REGNUM, the arm_regno_class change relating to 
PC_REGNUM, or something else?  Either of those would indicate something 
very strange going on in LRA (maybe something else needs to change 
somewhere as well to stop attempts to use CC_REGNUM or PC_REGNUM 
inappropriately?).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: Fix ARM ICE for register var asm ("pc") (PR target/60606)

[Alan Lawrence]

It seems to be the change to arm_regno_class relating to PC_REGNUM. I see 
scal-to-vec1.c failing with just that, or that in combination with the changes 
to cfgexpand.c+varasm.c.

And scal-to-vec1.c is OK on -fPIC if I apply the changes to cfgexpand.c, 
varasm.c, and arm.c (arm_hard_regno_ok), i.e. all bar the change to arm_regno_class.

A change relating to the program counter affecting -fPIC does sound plausible, I 
haven't looked any further than that...

--Alan

Joseph S. Myers wrote:
> On Wed, 17 Sep 2014, Alan Lawrence wrote:
> 
>> We've just noticed this patch causes an ICE in
>> gcc.c-torture/execute/scal-to-vec1.c at -O3 when running with -fPIC on
>> arm-none-linux-gnueabi and arm-none-linux-gnueabihf; test logs:
> 
> Which part causes the ICE?  The arm_hard_regno_mode_ok change relating to 
> modes assigned to CC_REGNUM, the arm_regno_class change relating to 
> PC_REGNUM, or something else?  Either of those would indicate something 
> very strange going on in LRA (maybe something else needs to change 
> somewhere as well to stop attempts to use CC_REGNUM or PC_REGNUM 
> inappropriately?).
>