diff mbox

[PR69707] Handle -fdiagnostics-color in lto

Message ID 56B89A49.3040700@mentor.com
State New
Headers show

Commit Message

Tom de Vries Feb. 8, 2016, 1:38 p.m. UTC 
On 08/02/16 11:42, Jakub Jelinek wrote:
> On Mon, Feb 08, 2016 at 11:34:39AM +0100, Tom de Vries wrote:
>> Hi,
>>
>> when running libgomp.oacc-c-c++-common/parallel-dims.c with -flto
>> -fno-use-linker-plugin, we run into a failing 'test for excess errors'.
>>
>> The problem is that while -fdiagnostics-color=never is passed to gcc, it's
>> not propagated to lto1, and the error message is annotated with color
>> information, which confuses the test for excess errors.
>>
>> This patch fixes the problem by making sure that -fdiagnostics-color is
>> propagated to lto1, in the same way that -fdiagnostics-show-caret is
>> propagated to lto1.
>>
>> Bootstrapped and reg-tested on x86_64.
>>
>> OK for trunk, stage1?
>
> Doesn't that mean diagnostics from the driver itself will no longer honor
> that option when deciding if the diagnostics should be colorized or not?
>

Hi,

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?

Thanks,
- Tom

Comments

Jakub Jelinek Feb. 8, 2016, 1:54 p.m. UTC | #1 
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?
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.  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), or is it passed on the linking command line, but not
passed through to lto1?

> Handle -fdiagnostics-color in lto
> 
> 2016-02-08  Tom de Vries  <tom@codesourcery.com>
> 
> 	PR lto/69707
> 	* lto-opts.c (lto_write_options): Allow fdiagnostics-color.
> 	* lto-wrapper.c (merge_and_complain, append_compiler_options): Handle
> 	OPT_fdiagnostics_color_.
> 
> 	* 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-opts.c (lto_write_options): Allow fdiagnostics-color.
	* lto-wrapper.c (merge_and_complain, append_compiler_options): Handle
	OPT_fdiagnostics_color_.

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

---
 gcc/lto-opts.c                                      | 21 ++++++++++++++++++---
 gcc/lto-wrapper.c                                   |  2 ++
 .../libgomp.oacc-c-c++-common/parallel-dims-2.c     | 19 +++++++++++++++++++
 3 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/gcc/lto-opts.c b/gcc/lto-opts.c
index 5f4b427..b15b9e8 100644
--- a/gcc/lto-opts.c
+++ b/gcc/lto-opts.c
@@ -200,9 +200,24 @@  lto_write_options (void)
 	 which includes things like -o and -v or -fhelp for example.
 	 We do not need those.  The only exception is -foffload option, if we
 	 write it in offload_lto section.  Also drop all diagnostic options.  */
-      if ((cl_options[option->opt_index].flags & (CL_DRIVER|CL_WARNING))
-	  && (!lto_stream_offload_p || option->opt_index != OPT_foffload_))
-	continue;
+      if (cl_options[option->opt_index].flags & (CL_DRIVER|CL_WARNING))
+	{
+	  bool keep = false;
+	  switch (option->opt_index)
+	    {
+	    case OPT_foffload_:
+	      keep = lto_stream_offload_p;
+	      break;
+	    case OPT_fdiagnostics_color_:
+	      keep = true;
+	      break;
+	    default:
+	      break;
+	    }
+
+	  if (!keep)
+	    continue;
+	}
 
       for (j = 0; j < option->canonical_option_num_elements; ++j)
 	append_to_collect_gcc_options (&temporary_obstack, &first_p,
diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
index ced6f2f..484dbc1 100644
--- a/gcc/lto-wrapper.c
+++ b/gcc/lto-wrapper.c
@@ -232,6 +232,7 @@  merge_and_complain (struct cl_decoded_option **decoded_options,
 	    break;
 
 	  /* Fallthru.  */
+	case OPT_fdiagnostics_color_:
 	case OPT_fdiagnostics_show_caret:
 	case OPT_fdiagnostics_show_option:
 	case OPT_fdiagnostics_show_location_:
@@ -497,6 +498,7 @@  append_compiler_options (obstack *argv_obstack, struct cl_decoded_option *opts,
 	 on any CL_TARGET flag and a few selected others.  */
       switch (option->opt_index)
 	{
+	case OPT_fdiagnostics_color_:
 	case OPT_fdiagnostics_show_caret:
 	case OPT_fdiagnostics_show_option:
 	case OPT_fdiagnostics_show_location_:
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;
+}
+