| Message ID | Pine.LNX.4.64.1408222113120.16713@digraph.polyomino.org.uk |
|---|---|
| State | New |
| Headers | show |
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~
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. */
>
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?).
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?). >
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. */