[PATCH v4] PR48344: Fix unrecognizable insn error with -fstack-limit-register=r2

[PATCH v4] PR48344: Fix unrecognizable insn error with -fstack-limit-register=r2

[Kelvin Nilsen]

[-- Attachment #1: Type: text/plain, Size: 1871 bytes --]

This patch has bootstrapped and tested on powerpc64le-unknown-linux-gnu 
with no regressions.  Is this ok for the trunk?

See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=48344 for the original 
problem report.  The error resulted because gcc's processing of 
command-line options within gcc initialization code originally preceded 
the processing of target-specific configuration hooks.

In the unpatched gcc implementation, the Pmode (pointer mode) variable 
has not been initialized at the time the -fstack-limit-register 
command-line option is processed.  As a consequence, the stack-limiting 
register is not assigned a proper mode.  Thus, rtl instructions that 
make use of this stack-limiting register have an unspecified mode, and 
are therefore not matched by any known instructions.

The fix represented in this patch is to defer the command-line 
processing related to command-line specification of a stack-limiting 
register until after target-specific initialization has been completed.

Some questions and issues raised in response to version 2 of this patch 
are addressed below:

1. Concerns regarding possible unintended consequences that might result 
from moving all target-specific initialization to precede the invocation 
of the handle_common_deferred_options () function are addressed by 
preserving the original initialization order and moving the relevant 
command-line processing to follow the target-specific initialization.

2. A question was raised as to whether Pmode can change with attribute 
target.  It cannot.

An issue raised in response to version 3 of this patch is addressed as 
follows:

1.  Abbreviated comments.

2. Fixed spacing following periods within comments.

3. Moved stack_limit_rtx initialization code into init_emit_once(void) 
and eliminated special initialization function that had been proposed in 
the V3 patch.



[-- Attachment #2: svn.ChangeLog --]
[-- Type: text/plain, Size: 574 bytes --]

gcc/testsuite/ChangeLog:

2016-02-11  Kelvin Nilsen  <kelvin@gcc.gnu.org>

	* gcc.target/powerpc/pr48344-1.c: New test.

gcc/ChangeLog:

2016-02-11  Kelvin Nilsen  <kelvin@gcc.gnu.org>

	* opts-global.c (handle_common_deferred_options): Introduce and
        initialize two global variables to remember command-line options
        specifying a stack-limiting register.
	* opts.h: Add extern declarations of the two new global variables. 
	* emit-rtl.c (init_emit_once): Initialize the stack_limit_rtx
        variable based on the values of the two new global variables.



[-- Attachment #3: svn.patch --]
[-- Type: text/x-patch, Size: 3199 bytes --]

Index: gcc/testsuite/gcc.target/powerpc/pr48344-1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr48344-1.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr48344-1.c	(revision 232633)
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-fstack-limit-register=r2" } */
+void foo ()
+{
+  int N = 2;
+  int slots[N];
+
+}
+
Index: gcc/emit-rtl.c
===================================================================
--- gcc/emit-rtl.c	(revision 232135)
+++ gcc/emit-rtl.c	(working copy)
@@ -57,6 +57,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "builtins.h"
 #include "rtl-iter.h"
 #include "stor-layout.h"
+#include "opts.h"
 
 struct target_rtl default_target_rtl;
 #if SWITCHABLE_TARGET
@@ -5895,6 +5896,13 @@ init_emit_once (void)
 
   /* Create the unique rtx's for certain rtx codes and operand values.  */
 
+  /* Process stack-limiting command-line options.  */
+  if (opt_fstack_limit_symbol_arg != NULL)
+    stack_limit_rtx 
+      = gen_rtx_SYMBOL_REF (Pmode, ggc_strdup (opt_fstack_limit_symbol_arg));
+  if (opt_fstack_limit_register_no >= 0)
+    stack_limit_rtx = gen_rtx_REG (Pmode, opt_fstack_limit_register_no);
+
   /* Don't use gen_rtx_CONST_INT here since gen_rtx_CONST_INT in this case
      tries to use these variables.  */
   for (i = - MAX_SAVED_CONST_INT; i <= MAX_SAVED_CONST_INT; i++)
Index: gcc/opts.h
===================================================================
--- gcc/opts.h	(revision 232135)
+++ gcc/opts.h	(working copy)
@@ -296,6 +296,10 @@ struct cl_option_handlers
   struct cl_option_handler_func handlers[3];
 };
 
+/* Hold command-line options associated with stack limitation.  */
+extern const char *opt_fstack_limit_symbol_arg;
+extern int opt_fstack_limit_register_no;
+
 /* Input file names.  */
 
 extern const char **in_fnames;
Index: gcc/opts-global.c
===================================================================
--- gcc/opts-global.c	(revision 232135)
+++ gcc/opts-global.c	(working copy)
@@ -310,6 +310,10 @@ decode_options (struct gcc_options *opts, struct g
   finish_options (opts, opts_set, loc);
 }
 
+/* Hold command-line options associated with stack limitation.  */
+const char *opt_fstack_limit_symbol_arg = NULL;
+int opt_fstack_limit_register_no = -1;
+
 /* Process common options that have been deferred until after the
    handlers have been called for all options.  */
 
@@ -417,12 +421,18 @@ handle_common_deferred_options (void)
 	    if (reg < 0)
 	      error ("unrecognized register name %qs", opt->arg);
 	    else
-	      stack_limit_rtx = gen_rtx_REG (Pmode, reg);
+	      {
+		/* Deactivate previous OPT_fstack_limit_symbol_ options.  */
+		opt_fstack_limit_symbol_arg = NULL;
+		opt_fstack_limit_register_no = reg;
+	      }
 	  }
 	  break;
 
 	case OPT_fstack_limit_symbol_:
-	  stack_limit_rtx = gen_rtx_SYMBOL_REF (Pmode, ggc_strdup (opt->arg));
+	  /* Deactivate previous OPT_fstack_limit_register_ options.  */
+	  opt_fstack_limit_register_no = -1;
+	  opt_fstack_limit_symbol_arg = opt->arg;
 	  break;
 
 	case OPT_fasan_shadow_offset_:
@@ -442,3 +452,4 @@ handle_common_deferred_options (void)
 	}
     }
 }
+

Re: [PATCH v4] PR48344: Fix unrecognizable insn error with -fstack-limit-register=r2

[Bernd Schmidt]

On 02/11/2016 04:12 PM, Kelvin Nilsen wrote:
>
> 	* opts-global.c (handle_common_deferred_options): Introduce and
>          initialize two global variables to remember command-line options
>          specifying a stack-limiting register.
> 	* opts.h: Add extern declarations of the two new global variables.
> 	* emit-rtl.c (init_emit_once): Initialize the stack_limit_rtx
>          variable based on the values of the two new global variables.

Make sure that when committed these all start with a tab character.

> +void foo ()
> +{
> +  int N = 2;
> +  int slots[N];
> +
> +}
> +

Watch extra blank lines at the end of files.

> @@ -442,3 +452,4 @@ handle_common_deferred_options (void)
>   	}
>       }
>   }
> +
>

Here too.

Ok with these fixed.


Bernd

Re: [PATCH v4] PR48344: Fix unrecognizable insn error with -fstack-limit-register=r2

[Andreas Schwab]

On Feb 11 2016, Kelvin Nilsen <kdnilsen@linux.vnet.ibm.com> wrote:

> 	* opts-global.c (handle_common_deferred_options): Introduce and
>         initialize two global variables to remember command-line options
>         specifying a stack-limiting register.
> 	* opts.h: Add extern declarations of the two new global variables. 
> 	* emit-rtl.c (init_emit_once): Initialize the stack_limit_rtx
>         variable based on the values of the two new global variables.

That breaks gcc.target/m68k/stack-limit-1.c:

/daten/aranym/gcc/test/gcc/testsuite/gcc.target/m68k/stack-limit-1.c: In function 'dummy':
/daten/aranym/gcc/test/gcc/testsuite/gcc.target/m68k/stack-limit-1.c:6:1: error: unrecognizable insn:
(insn 10 9 11 (trap_if (ltu (cc0)
            (const_int 0 [0]))
        (const_int 1 [0x1])) /daten/aranym/gcc/test/gcc/testsuite/gcc.target/m68k/stack-limit-1.c:6 -1
     (nil))
/daten/aranym/gcc/test/gcc/testsuite/gcc.target/m68k/stack-limit-1.c:6:1: internal compiler error: in extract_insn, at recog.c:2287

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH v4] PR48344: Fix unrecognizable insn error with -fstack-limit-register=r2

[Bernd Schmidt]

On 09/30/2016 10:02 PM, Andreas Schwab wrote:
> On Feb 11 2016, Kelvin Nilsen <kdnilsen@linux.vnet.ibm.com> wrote:
>
>> 	* opts-global.c (handle_common_deferred_options): Introduce and
>>         initialize two global variables to remember command-line options
>>         specifying a stack-limiting register.
>> 	* opts.h: Add extern declarations of the two new global variables.
>> 	* emit-rtl.c (init_emit_once): Initialize the stack_limit_rtx
>>         variable based on the values of the two new global variables.
>
> That breaks gcc.target/m68k/stack-limit-1.c:
>
> /daten/aranym/gcc/test/gcc/testsuite/gcc.target/m68k/stack-limit-1.c: In function 'dummy':
> /daten/aranym/gcc/test/gcc/testsuite/gcc.target/m68k/stack-limit-1.c:6:1: error: unrecognizable insn:
> (insn 10 9 11 (trap_if (ltu (cc0)
>             (const_int 0 [0]))
>         (const_int 1 [0x1])) /daten/aranym/gcc/test/gcc/testsuite/gcc.target/m68k/stack-limit-1.c:6 -1
>      (nil))
> /daten/aranym/gcc/test/gcc/testsuite/gcc.target/m68k/stack-limit-1.c:6:1: internal compiler error: in extract_insn, at recog.c:2287
>
Please file a PR if this isn't fixed yet.


Bernd

Re: [PATCH v4] PR48344: Fix unrecognizable insn error with -fstack-limit-register=r2

[Andreas Schwab]

I've committed this to fix the ICE.

Andreas.

	* config/m68k/m68k.c (m68k_option_override): Check
	opt_fstack_limit_symbol_arg and opt_fstack_limit_register_no
	instead of stack_limit_rtx.

	* gcc.target/m68k/stack-limit-1.c: Expect warning on line 0.

diff --git a/gcc/config/m68k/m68k.c b/gcc/config/m68k/m68k.c
index e6bcfa0caf..a883e42514 100644
--- a/gcc/config/m68k/m68k.c
+++ b/gcc/config/m68k/m68k.c
@@ -638,10 +638,12 @@ m68k_option_override (void)
     }
 #endif
 
-  if (stack_limit_rtx != NULL_RTX && !TARGET_68020)
+  if ((opt_fstack_limit_symbol_arg != NULL || opt_fstack_limit_register_no >= 0)
+      && !TARGET_68020)
     {
       warning (0, "-fstack-limit- options are not supported on this cpu");
-      stack_limit_rtx = NULL_RTX;
+      opt_fstack_limit_symbol_arg = NULL;
+      opt_fstack_limit_register_no = -1;
     }
 
   SUBTARGET_OVERRIDE_OPTIONS;
diff --git a/gcc/testsuite/gcc.target/m68k/stack-limit-1.c b/gcc/testsuite/gcc.target/m68k/stack-limit-1.c
index b1e9b99b26..5086edd77f 100644
--- a/gcc/testsuite/gcc.target/m68k/stack-limit-1.c
+++ b/gcc/testsuite/gcc.target/m68k/stack-limit-1.c
@@ -1,6 +1,6 @@
 /* -fstack-limit- should be ignored without an ICE if not supported.  */
 /* { dg-do compile } */
 /* { dg-options "-fstack-limit-symbol=_stack_limit -m68000" } */
-/* { dg-warning "not supported" "" { target *-*-* } 1 } */
+/* { dg-warning "not supported" "" { target *-*-* } 0 } */
 
 void dummy (void) { }
-- 
2.10.1


-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."