[PR target/78213] Do not ICE on non-empty -fself-test

[PR target/78213] Do not ICE on non-empty -fself-test

[Aldy Hernandez]

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

The problem in this PR is that -fself-test is being run on a non empty 
source file.  This causes init_emit() to run, which sets:

	REG_POINTER (virtual_incoming_args_rtx) = 1;

Setting REG_POINTER on the virtual incoming args, causes /f to be 
printed on some RTL dumps, causing the -fself-test machinery to fail at 
matching the expected value.

It looks that by design -fself-test is meant to be run before any 
initialization like the aforementioned runs.

We could error/fail when running -fself-test on a non-empty file, but I 
think we can just enable -fsyntax-only and get the same result.  After 
all, this is an undocumented internal option.

BTW, this is being triggered on aarch64, but will likely show up in 
different ports in different ways.

OK for trunk?

[-- Attachment #2: curr --]
[-- Type: text/plain, Size: 1529 bytes --]

commit f5d7bfbdae2f74d3b98a81e1f3ee7b9b24c8f3be
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Fri Nov 11 06:32:08 2016 -0800

    	PR target/78213
    	* opts.c (common_handle_option): Set -fsyntax-only if running self
    	tests.

diff --git a/gcc/opts.c b/gcc/opts.c
index d2d6100..35357bd 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -1770,6 +1770,16 @@ common_handle_option (struct gcc_options *opts,
       opts->x_exit_after_options = true;
       break;
 
+    case OPT_fself_test:
+      /* -fself-test depends on the state of the compiler prior to
+         compiling anything.  Ideally it should be run on an empty
+         source file.  However, in case we get run with actual source,
+         assume -fsyntax-only which will inhibit any compiler
+         initialization which may confuse the self tests.  */
+      if (lang_mask != CL_DRIVER)
+	flag_syntax_only = 1;
+      break;
+
     case OPT_fsanitize_:
       opts->x_flag_sanitize
 	= parse_sanitizer_options (arg, loc, code,
diff --git a/gcc/testsuite/gcc.dg/pr78213.c b/gcc/testsuite/gcc.dg/pr78213.c
new file mode 100644
index 0000000..b262fa1
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr78213.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-fself-test" } */
+
+/* Verify that -fself-test does not fail on a non empty source.  */
+
+int i;                                                                          void bar();                                                                     void foo()
+{
+  while (i--)
+    bar();
+}

Re: [PR target/78213] Do not ICE on non-empty -fself-test

[Jakub Jelinek]

On Fri, Nov 11, 2016 at 09:10:48AM -0800, Aldy Hernandez wrote:
> The problem in this PR is that -fself-test is being run on a non empty
> source file.  This causes init_emit() to run, which sets:
> 
> 	REG_POINTER (virtual_incoming_args_rtx) = 1;
> 
> Setting REG_POINTER on the virtual incoming args, causes /f to be printed on
> some RTL dumps, causing the -fself-test machinery to fail at matching the
> expected value.
> 
> It looks that by design -fself-test is meant to be run before any
> initialization like the aforementioned runs.

Won't -fself-test -fno-syntax-only still crash?  I.e. wouldn't it be better
to deal with this e.g. in finish_options?  Or perhaps error out if
-fself-tests is not used with -fsyntax-only and change the Makefile to
invoke it with both options?

	Jakub

Re: [PR target/78213] Do not ICE on non-empty -fself-test

[Bernd Schmidt]

On 11/11/2016 06:10 PM, Aldy Hernandez wrote:
> The problem in this PR is that -fself-test is being run on a non empty
> source file.  This causes init_emit() to run, which sets:
>
>     REG_POINTER (virtual_incoming_args_rtx) = 1;
>
> Setting REG_POINTER on the virtual incoming args, causes /f to be
> printed on some RTL dumps, causing the -fself-test machinery to fail at
> matching the expected value.

How about always running init_emit and testing for the correct output?


Bernd

Re: [PR target/78213] Do not ICE on non-empty -fself-test

[Jakub Jelinek]

On Mon, Nov 14, 2016 at 02:18:02PM +0100, Bernd Schmidt wrote:
> On 11/11/2016 06:10 PM, Aldy Hernandez wrote:
> >The problem in this PR is that -fself-test is being run on a non empty
> >source file.  This causes init_emit() to run, which sets:
> >
> >    REG_POINTER (virtual_incoming_args_rtx) = 1;
> >
> >Setting REG_POINTER on the virtual incoming args, causes /f to be
> >printed on some RTL dumps, causing the -fself-test machinery to fail at
> >matching the expected value.
> 
> How about always running init_emit and testing for the correct output?

You mean only if -fself-test, right?

	Jakub

Re: [PR target/78213] Do not ICE on non-empty -fself-test

[Bernd Schmidt]

On 11/14/2016 02:20 PM, Jakub Jelinek wrote:
> On Mon, Nov 14, 2016 at 02:18:02PM +0100, Bernd Schmidt wrote:
>> On 11/11/2016 06:10 PM, Aldy Hernandez wrote:
>>> The problem in this PR is that -fself-test is being run on a non empty
>>> source file.  This causes init_emit() to run, which sets:
>>>
>>>    REG_POINTER (virtual_incoming_args_rtx) = 1;
>>>
>>> Setting REG_POINTER on the virtual incoming args, causes /f to be
>>> printed on some RTL dumps, causing the -fself-test machinery to fail at
>>> matching the expected value.
>>
>> How about always running init_emit and testing for the correct output?
>
> You mean only if -fself-test, right?

I guess.


Bernd

Re: [PR target/78213] Do not ICE on non-empty -fself-test

[Aldy Hernandez]

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

On 11/14/2016 08:18 AM, Bernd Schmidt wrote:
> On 11/11/2016 06:10 PM, Aldy Hernandez wrote:
>> The problem in this PR is that -fself-test is being run on a non empty
>> source file.  This causes init_emit() to run, which sets:
>>
>>     REG_POINTER (virtual_incoming_args_rtx) = 1;
>>
>> Setting REG_POINTER on the virtual incoming args, causes /f to be
>> printed on some RTL dumps, causing the -fself-test machinery to fail at
>> matching the expected value.
>
> How about always running init_emit and testing for the correct output?

I would prefer Jakub's suggestion of running in finish_options().

I assume there are other places throughout the self-tests that depend on 
NOT continuing the compilation process, and I'd hate to plug each one.

Would the attached patch be acceptable to both of you?

Aldy

[-- Attachment #2: curr --]
[-- Type: text/plain, Size: 1597 bytes --]

commit 52b906160a109b84a26f5aa15a653f4aee612942
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Fri Nov 11 06:32:08 2016 -0800

    	PR target/78213
    	* opts.c (finish_options): Set -fsyntax-only if running self
    	tests.

diff --git a/gcc/opts.c b/gcc/opts.c
index d2d6100..cb20154 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -744,6 +744,14 @@ finish_options (struct gcc_options *opts, struct gcc_options *opts_set,
       opts->x_flag_toplevel_reorder = 0;
     }
 
+  /* -fself-test depends on the state of the compiler prior to
+     compiling anything.  Ideally it should be run on an empty source
+     file.  However, in case we get run with actual source, assume
+     -fsyntax-only which will inhibit any compiler initialization
+     which may confuse the self tests.  */
+  if (opts->x_flag_self_test)
+    opts->x_flag_syntax_only = 1;
+
   if (opts->x_flag_tm && opts->x_flag_non_call_exceptions)
     sorry ("transactional memory is not supported with non-call exceptions");
 
diff --git a/gcc/testsuite/gcc.dg/pr78213.c b/gcc/testsuite/gcc.dg/pr78213.c
new file mode 100644
index 0000000..e43c83c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr78213.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-fself-test" } */
+
+/* Verify that -fself-test does not fail on a non empty source.  */
+
+int i;                                                                          void bar();                                                                     void foo()
+{
+  while (i--)
+    bar();
+}
+/* { dg-message "fself\-test: " "-fself-test" { target *-*-* } 0 } */

Re: [PR target/78213] Do not ICE on non-empty -fself-test

[Bernd Schmidt]

On 11/16/2016 11:45 AM, Aldy Hernandez wrote:

> I would prefer Jakub's suggestion of running in finish_options().

I suspect we'll want both. Selftests should really run in an environment 
that's as close as possible to what would normally be going on in the 
compiler.

> I assume there are other places throughout the self-tests that depend on
> NOT continuing the compilation process, and I'd hate to plug each one.
>
> Would the attached patch be acceptable to both of you?

Good enough for now.


Bernd