diff mbox

[PR69707] Handle -fdiagnostics-color in lto

Message ID 56B902F0.1030201@mentor.com
State New
Headers show

Commit Message

Tom de Vries Feb. 8, 2016, 9:04 p.m. UTC 
On 08/02/16 14:54, Jakub Jelinek wrote:
> On Mon, Feb 08, 2016 at 02:38:17PM +0100, Tom de Vries wrote:
>> hmm, indeed removing the 'Driver' flag from the fdiagnostics-color= entry in
>> common.opt breaks the functioning of fdiagnostics-color= in the gcc driver.
>>
>> This patch leaves the 'Driver' flag alone, and instead explicitly allows
>> fdiagnostics-color= in lto_write_options.
>>
>> Is this approach ok?
>
> Doesn't that mean storing -fdiagnostics-color= into the LTO option section and
> then using whatever was recorded from the first TU that recorded anything?

Yes.

> That doesn't look right for such diagnostics options either.
> I mean, if I
> $ gcc -fdiagnostics-color=always -c -flto a.c
> on another terminal
> $ gcc -fdiagnostics-color=never -c -flto b.c
> on yet another terminal
> $ gcc -flto -o a a.o b.o
> then I'd expect the default setting for -fdiagnostics-color= for all
> diagnostics while linking/LTO optimizing it, and for say
> $ gcc -fdiagnostics-color=always -flto -o a a.o b.o
> to have it always colorized, similarly for never.
> IMHO we should just honor what has been specified on the linker command
> line if anything.

Agreed.

> So, the question is, is -fdiagnostics-color=never passed
> in the testsuite just to the compilation and not to linking (that would be
> IMHO a testsuite bug),

In libgomp.exp we have:
...
     # Disable color diagnostics
     lappend ALWAYS_CFLAGS "additional_flags=-fdiagnostics-color=never"
...

The test setup looks ok to me.

> or is it passed on the linking command line, but not
> passed through to lto1?

Yes.

I've now realized this is specific to mkoffload.

If we're doing lto, in lto-wrapper.c:run_gcc, we have:
...
   append_compiler_options (&argv_obstack, fdecoded_options,
                            fdecoded_options_count);
   append_linker_options (&argv_obstack, decoded_options,
                          decoded_options_count);
...
And the last line will propagate the fdiagnostics-color setting.

But for calling mkoffload, we just have this:
...
       /* Append options from offload_lto sections.  */
       append_compiler_options (&argv_obstack, compiler_opts,
                                compiler_opt_count);
...

Followed by this, which just filters the -foffload options:
...
       /* Append options specified by -foffload last.  In case of
          conflicting options we expect offload compiler to choose
          the latest.  */
       append_offload_options (&argv_obstack, target, compiler_opts,
                               compiler_opt_count);
       append_offload_options (&argv_obstack, target, linker_opts,
                               linker_opt_count);
...

Attached patch adds the diagnostics flags to mkoffload.

Bootstrapped and reg-tested on x86_64.

OK for trunk, stage1?

Thanks,
- Tom

Comments

Jakub Jelinek Feb. 8, 2016, 10:02 p.m. UTC | #1 
On Mon, Feb 08, 2016 at 10:04:48PM +0100, Tom de Vries wrote:
> Attached patch adds the diagnostics flags to mkoffload.
> 
> Bootstrapped and reg-tested on x86_64.
> 
> OK for trunk, stage1?

Ok for stage4.

> Handle -fdiagnostics-color in lto
> 
> 2016-02-08  Tom de Vries  <tom@codesourcery.com>
> 
> 	PR lto/69707
> 	* lto-wrapper.c (append_diag_options): New function.
> 	(compile_offload_image): Call append_diag_options.
> 
> 	* testsuite/libgomp.oacc-c-c++-common/parallel-dims-2.c: New test.

	Jakub
diff mbox

Patch

Handle -fdiagnostics-color in lto

2016-02-08  Tom de Vries  <tom@codesourcery.com>

	PR lto/69707
	* lto-wrapper.c (append_diag_options): New function.
	(compile_offload_image): Call append_diag_options.

	* testsuite/libgomp.oacc-c-c++-common/parallel-dims-2.c: New test.

---
 gcc/lto-wrapper.c                                  | 31 ++++++++++++++++++++++
 .../libgomp.oacc-c-c++-common/parallel-dims-2.c    | 19 +++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
index ced6f2f..8cda1fa 100644
--- a/gcc/lto-wrapper.c
+++ b/gcc/lto-wrapper.c
@@ -542,6 +542,36 @@  append_compiler_options (obstack *argv_obstack, struct cl_decoded_option *opts,
     }
 }
 
+/* Append diag options in OPTS with length COUNT to ARGV_OBSTACK.  */
+
+static void
+append_diag_options (obstack *argv_obstack, struct cl_decoded_option *opts,
+		     unsigned int count)
+{
+  /* Append compiler driver arguments as far as they were merged.  */
+  for (unsigned int j = 1; j < count; ++j)
+    {
+      struct cl_decoded_option *option = &opts[j];
+
+      switch (option->opt_index)
+	{
+	case OPT_fdiagnostics_color_:
+	case OPT_fdiagnostics_show_caret:
+	case OPT_fdiagnostics_show_option:
+	case OPT_fdiagnostics_show_location_:
+	case OPT_fshow_column:
+	  break;
+	default:
+	  continue;
+	}
+
+      /* Pass the option on.  */
+      for (unsigned int i = 0; i < option->canonical_option_num_elements; ++i)
+	obstack_ptr_grow (argv_obstack, option->canonical_option[i]);
+    }
+}
+
+
 /* Append linker options OPTS to ARGV_OBSTACK.  */
 
 static void
@@ -724,6 +754,7 @@  compile_offload_image (const char *target, const char *compiler_path,
       /* Append options from offload_lto sections.  */
       append_compiler_options (&argv_obstack, compiler_opts,
 			       compiler_opt_count);
+      append_diag_options (&argv_obstack, linker_opts, linker_opt_count);
 
       /* Append options specified by -foffload last.  In case of conflicting
 	 options we expect offload compiler to choose the latest.  */
diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/parallel-dims-2.c b/libgomp/testsuite/libgomp.oacc-c-c++-common/parallel-dims-2.c
new file mode 100644
index 0000000..eea8c7e
--- /dev/null
+++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/parallel-dims-2.c
@@ -0,0 +1,19 @@ 
+/* { dg-do run { target { openacc_nvidia_accel_selected && lto } } } */
+/* { dg-additional-options "-flto -fno-use-linker-plugin" } */
+
+/* Worker and vector size checks.  Picked an outrageously large
+   value.  */
+
+int main ()
+{
+#pragma acc parallel num_workers (2<<20) /* { dg-error "using num_workers" } */
+  {
+  }
+
+#pragma acc parallel vector_length (2<<20) /* { dg-error "using vector_length" } */
+  {
+  }
+
+  return 0;
+}
+