[PATCH, PR81430] Use finalize_options in lto1

[PATCH, PR81430] Use finalize_options in lto1

[Tom de Vries]

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

Hi,

this patch fixes PR81430, an ICE in the libgomp testsuite for both 
openmp and openacc test-cases for x86_64 with nvptx accelerator.

The scenario how we hit the ICE is as follows:
- a testcase is compiled with -O2
- ix86_option_optimization_table enables
   OPT_freorder_blocks_and_partition at -O2
- cc1 writes out the flag as part of DECL_FUNCTION_SPECIFIC_OPTIMIZATION
- lto1 reads in the flag as part of DECL_FUNCTION_SPECIFIC_OPTIMIZATION
- lto1 uses the flag, and runs pass_partition_blocks
- pass_partition_blocks ICEs, because it generates code that is not
   supported by the nvptx target.

Note that for standalone compilation for single-thread ptx execution, we 
don't attempt to run pass_partition_blocks. This is because for nvptx, 
TARGET_HAVE_NAMED_SECTIONS is set to false, and this bit in 
finish_options switches off pass_partition_blocks:
...
    /* If the target requested unwind info, then turn off the
       partitioning optimization with a different message.  Likewise, if
       the target does not support named sections.  */

   if (opts->x_flag_reorder_blocks_and_partition
       && (!targetm_common.have_named_sections
           || (opts->x_flag_unwind_tables
               && targetm_common.unwind_tables_default
               && (ui_except == UI_SJLJ || ui_except >= UI_TARGET))))
     {
       if (opts_set->x_flag_reorder_blocks_and_partition)
         inform (loc,
                 "-freorder-blocks-and-partition does not work "
                 "on this architecture");
       opts->x_flag_reorder_blocks_and_partition = 0;
       opts->x_flag_reorder_blocks = 1;
     }
...

The patch fixes this by calling finish_options in lto1 after 
cl_optimization_restore.

Points for review:
1. I'm uncertain though about the placement of the call. Perhaps it 
should be in cl_optimization_restore, before 
targetm.override_options_after_change?

2. I think that this is offloading specific, so perhaps this should be 
guarded with lto_stream_offload_p or #ifdef ACCEL_COMPILER or some such.

Bootstrapped and reg-tested on x86_64.

OK for trunk?

Thanks,
- Tom

[-- Attachment #2: 0001-Use-finalize_options-in-lto1.patch --]
[-- Type: text/x-patch, Size: 1009 bytes --]

Use finalize_options in lto1

2017-07-20  Tom de Vries  <tom@codesourcery.com>

	PR lto/81430
	* function.c (invoke_set_current_function_hook): Call finish_options in
	lto1 after cl_optimization_restore.

---
 gcc/function.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/gcc/function.c b/gcc/function.c
index f625489..ccb312b 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -77,6 +77,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "rtl-chkp.h"
 #include "tree-dfa.h"
 #include "tree-ssa.h"
+#include "opts.h"
 
 /* So we can assign to cfun in this file.  */
 #undef cfun
@@ -4793,6 +4794,11 @@ invoke_set_current_function_hook (tree fndecl)
 	{
 	  optimization_current_node = opts;
 	  cl_optimization_restore (&global_options, TREE_OPTIMIZATION (opts));
+	  if (in_lto_p)
+	    finish_options (&global_options, &global_options_set,
+			    (fndecl
+			     ? DECL_SOURCE_LOCATION (fndecl)
+			     : UNKNOWN_LOCATION));
 	}
 
       targetm.set_current_function (fndecl);

Re: [PATCH, PR81430] Use finalize_options in lto1

[Richard Biener]

On Thu, 20 Jul 2017, Tom de Vries wrote:

> Hi,
> 
> this patch fixes PR81430, an ICE in the libgomp testsuite for both openmp and
> openacc test-cases for x86_64 with nvptx accelerator.
> 
> The scenario how we hit the ICE is as follows:
> - a testcase is compiled with -O2
> - ix86_option_optimization_table enables
>   OPT_freorder_blocks_and_partition at -O2
> - cc1 writes out the flag as part of DECL_FUNCTION_SPECIFIC_OPTIMIZATION
> - lto1 reads in the flag as part of DECL_FUNCTION_SPECIFIC_OPTIMIZATION
> - lto1 uses the flag, and runs pass_partition_blocks
> - pass_partition_blocks ICEs, because it generates code that is not
>   supported by the nvptx target.
> 
> Note that for standalone compilation for single-thread ptx execution, we don't
> attempt to run pass_partition_blocks. This is because for nvptx,
> TARGET_HAVE_NAMED_SECTIONS is set to false, and this bit in finish_options
> switches off pass_partition_blocks:
> ...
>    /* If the target requested unwind info, then turn off the
>       partitioning optimization with a different message.  Likewise, if
>       the target does not support named sections.  */
> 
>   if (opts->x_flag_reorder_blocks_and_partition
>       && (!targetm_common.have_named_sections
>           || (opts->x_flag_unwind_tables
>               && targetm_common.unwind_tables_default
>               && (ui_except == UI_SJLJ || ui_except >= UI_TARGET))))
>     {
>       if (opts_set->x_flag_reorder_blocks_and_partition)
>         inform (loc,
>                 "-freorder-blocks-and-partition does not work "
>                 "on this architecture");
>       opts->x_flag_reorder_blocks_and_partition = 0;
>       opts->x_flag_reorder_blocks = 1;
>     }
> ...
> 
> The patch fixes this by calling finish_options in lto1 after
> cl_optimization_restore.
> 
> Points for review:
> 1. I'm uncertain though about the placement of the call. Perhaps it should be
> in cl_optimization_restore, before targetm.override_options_after_change?
> 
> 2. I think that this is offloading specific, so perhaps this should be guarded
> with lto_stream_offload_p or #ifdef ACCEL_COMPILER or some such.

Hmm, I agree with #2.  I think it conceptually is a LTO stream adjustment
and thus we should do this at the time we stream in the 
optimization/target nodes (like we remap modes for example).  Not
sure if it's possible to do this at that point, but it looks like
finish_options takes two option structs and thus we should be able to
call it.

Do you get the inform note?  I suppose we don't really want that, no?

Richard.

> Bootstrapped and reg-tested on x86_64.
> 
> OK for trunk?
> 
> Thanks,
> - Tom
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

Re: [PATCH, PR81430] Use finalize_options in lto1

[Tom de Vries]

On 07/20/2017 12:10 PM, Richard Biener wrote:
> On Thu, 20 Jul 2017, Tom de Vries wrote:
> 
>> Hi,
>>
>> this patch fixes PR81430, an ICE in the libgomp testsuite for both openmp and
>> openacc test-cases for x86_64 with nvptx accelerator.
>>
>> The scenario how we hit the ICE is as follows:
>> - a testcase is compiled with -O2
>> - ix86_option_optimization_table enables
>>    OPT_freorder_blocks_and_partition at -O2
>> - cc1 writes out the flag as part of DECL_FUNCTION_SPECIFIC_OPTIMIZATION
>> - lto1 reads in the flag as part of DECL_FUNCTION_SPECIFIC_OPTIMIZATION
>> - lto1 uses the flag, and runs pass_partition_blocks
>> - pass_partition_blocks ICEs, because it generates code that is not
>>    supported by the nvptx target.
>>
>> Note that for standalone compilation for single-thread ptx execution, we don't
>> attempt to run pass_partition_blocks. This is because for nvptx,
>> TARGET_HAVE_NAMED_SECTIONS is set to false, and this bit in finish_options
>> switches off pass_partition_blocks:
>> ...
>>     /* If the target requested unwind info, then turn off the
>>        partitioning optimization with a different message.  Likewise, if
>>        the target does not support named sections.  */
>>
>>    if (opts->x_flag_reorder_blocks_and_partition
>>        && (!targetm_common.have_named_sections
>>            || (opts->x_flag_unwind_tables
>>                && targetm_common.unwind_tables_default
>>                && (ui_except == UI_SJLJ || ui_except >= UI_TARGET))))
>>      {
>>        if (opts_set->x_flag_reorder_blocks_and_partition)
>>          inform (loc,
>>                  "-freorder-blocks-and-partition does not work "
>>                  "on this architecture");
>>        opts->x_flag_reorder_blocks_and_partition = 0;
>>        opts->x_flag_reorder_blocks = 1;
>>      }
>> ...
>>
>> The patch fixes this by calling finish_options in lto1 after
>> cl_optimization_restore.
>>
>> Points for review:
>> 1. I'm uncertain though about the placement of the call. Perhaps it should be
>> in cl_optimization_restore, before targetm.override_options_after_change?
>>
>> 2. I think that this is offloading specific, so perhaps this should be guarded
>> with lto_stream_offload_p or #ifdef ACCEL_COMPILER or some such.
> 
> Hmm, I agree with #2.  I think it conceptually is a LTO stream adjustment
> and thus we should do this at the time we stream in the
> optimization/target nodes (like we remap modes for example).  Not
> sure if it's possible to do this at that point, but it looks like
> finish_options takes two option structs and thus we should be able to
> call it.
> 

With what parameters? Patch below tries with same option struct, but ...

> Do you get the inform note?  I suppose we don't really want that, no?
> 

... I think that way we'll get the inform note (while the previous 
solution did not).

I've also tried with a tmp2 memset to 0, but that ran into problems when 
doing a maybe_set_param_value.

Thanks,
- Tom

diff --git a/gcc/tree-streamer-in.c b/gcc/tree-streamer-in.c
index 7f7ea7f..e0e792b 100644
--- a/gcc/tree-streamer-in.c
+++ b/gcc/tree-streamer-in.c
@@ -33,6 +33,7 @@ along with GCC; see the file COPYING3.  If not see
  #include "ipa-chkp.h"
  #include "gomp-constants.h"
  #include "asan.h"
+#include "opts.h"


  /* Read a STRING_CST from the string table in DATA_IN using input
@@ -769,6 +770,20 @@ lto_input_ts_function_decl_tree_pointers (struct 
lto_input_block *ib,
    DECL_FUNCTION_SPECIFIC_TARGET (expr) = stream_read_tree (ib, data_in);
  #endif
    DECL_FUNCTION_SPECIFIC_OPTIMIZATION (expr) = stream_read_tree (ib, 
data_in);
+#ifdef ACCEL_COMPILER
+  {
+    tree opts = DECL_FUNCTION_SPECIFIC_OPTIMIZATION (expr);
+    if (opts)
+      {
+       struct gcc_options tmp;
+       cl_optimization_restore (&tmp, TREE_OPTIMIZATION (opts));
+       finish_options (&tmp, &tmp, DECL_SOURCE_LOCATION (expr));
+       opts = build_optimization_node (&tmp);
+
+       DECL_FUNCTION_SPECIFIC_OPTIMIZATION (expr) = opts;
+      }
+  }
+#endif

    /* If the file contains a function with an EH personality set,
       then it was compiled with -fexceptions.  In that case, initialize

Re: [PATCH, PR81430] Use finalize_options in lto1

[Richard Biener]

On Thu, 20 Jul 2017, Tom de Vries wrote:

> On 07/20/2017 12:10 PM, Richard Biener wrote:
> > On Thu, 20 Jul 2017, Tom de Vries wrote:
> > 
> > > Hi,
> > > 
> > > this patch fixes PR81430, an ICE in the libgomp testsuite for both openmp
> > > and
> > > openacc test-cases for x86_64 with nvptx accelerator.
> > > 
> > > The scenario how we hit the ICE is as follows:
> > > - a testcase is compiled with -O2
> > > - ix86_option_optimization_table enables
> > >    OPT_freorder_blocks_and_partition at -O2
> > > - cc1 writes out the flag as part of DECL_FUNCTION_SPECIFIC_OPTIMIZATION
> > > - lto1 reads in the flag as part of DECL_FUNCTION_SPECIFIC_OPTIMIZATION
> > > - lto1 uses the flag, and runs pass_partition_blocks
> > > - pass_partition_blocks ICEs, because it generates code that is not
> > >    supported by the nvptx target.
> > > 
> > > Note that for standalone compilation for single-thread ptx execution, we
> > > don't
> > > attempt to run pass_partition_blocks. This is because for nvptx,
> > > TARGET_HAVE_NAMED_SECTIONS is set to false, and this bit in finish_options
> > > switches off pass_partition_blocks:
> > > ...
> > >     /* If the target requested unwind info, then turn off the
> > >        partitioning optimization with a different message.  Likewise, if
> > >        the target does not support named sections.  */
> > > 
> > >    if (opts->x_flag_reorder_blocks_and_partition
> > >        && (!targetm_common.have_named_sections
> > >            || (opts->x_flag_unwind_tables
> > >                && targetm_common.unwind_tables_default
> > >                && (ui_except == UI_SJLJ || ui_except >= UI_TARGET))))
> > >      {
> > >        if (opts_set->x_flag_reorder_blocks_and_partition)
> > >          inform (loc,
> > >                  "-freorder-blocks-and-partition does not work "
> > >                  "on this architecture");
> > >        opts->x_flag_reorder_blocks_and_partition = 0;
> > >        opts->x_flag_reorder_blocks = 1;
> > >      }
> > > ...
> > > 
> > > The patch fixes this by calling finish_options in lto1 after
> > > cl_optimization_restore.
> > > 
> > > Points for review:
> > > 1. I'm uncertain though about the placement of the call. Perhaps it should
> > > be
> > > in cl_optimization_restore, before targetm.override_options_after_change?
> > > 
> > > 2. I think that this is offloading specific, so perhaps this should be
> > > guarded
> > > with lto_stream_offload_p or #ifdef ACCEL_COMPILER or some such.
> > 
> > Hmm, I agree with #2.  I think it conceptually is a LTO stream adjustment
> > and thus we should do this at the time we stream in the
> > optimization/target nodes (like we remap modes for example).  Not
> > sure if it's possible to do this at that point, but it looks like
> > finish_options takes two option structs and thus we should be able to
> > call it.
> > 
> 
> With what parameters? Patch below tries with same option struct, but ...
> 
> > Do you get the inform note?  I suppose we don't really want that, no?
> > 
> 
> ... I think that way we'll get the inform note (while the previous solution
> did not).
> 
> I've also tried with a tmp2 memset to 0, but that ran into problems when doing
> a maybe_set_param_value.

Use global_options_set?  That should do what the other patch did.

> Thanks,
> - Tom
> 
> diff --git a/gcc/tree-streamer-in.c b/gcc/tree-streamer-in.c
> index 7f7ea7f..e0e792b 100644
> --- a/gcc/tree-streamer-in.c
> +++ b/gcc/tree-streamer-in.c
> @@ -33,6 +33,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "ipa-chkp.h"
>  #include "gomp-constants.h"
>  #include "asan.h"
> +#include "opts.h"
> 
> 
>  /* Read a STRING_CST from the string table in DATA_IN using input
> @@ -769,6 +770,20 @@ lto_input_ts_function_decl_tree_pointers (struct
> lto_input_block *ib,
>    DECL_FUNCTION_SPECIFIC_TARGET (expr) = stream_read_tree (ib, data_in);
>  #endif
>    DECL_FUNCTION_SPECIFIC_OPTIMIZATION (expr) = stream_read_tree (ib,
> data_in);
> +#ifdef ACCEL_COMPILER
> +  {
> +    tree opts = DECL_FUNCTION_SPECIFIC_OPTIMIZATION (expr);
> +    if (opts)
> +      {
> +       struct gcc_options tmp;
> +       cl_optimization_restore (&tmp, TREE_OPTIMIZATION (opts));
> +       finish_options (&tmp, &tmp, DECL_SOURCE_LOCATION (expr));
> +       opts = build_optimization_node (&tmp);
> +
> +       DECL_FUNCTION_SPECIFIC_OPTIMIZATION (expr) = opts;
> +      }
> +  }
> +#endif
> 
>    /* If the file contains a function with an EH personality set,
>       then it was compiled with -fexceptions.  In that case, initialize
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

Re: [PATCH, PR81430] Use finalize_options in lto1

[Tom de Vries]

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

On 07/21/2017 11:41 AM, Richard Biener wrote:
> On Thu, 20 Jul 2017, Tom de Vries wrote:
> 
>> On 07/20/2017 12:10 PM, Richard Biener wrote:
>>> On Thu, 20 Jul 2017, Tom de Vries wrote:
>>>
>>>> Hi,
>>>>
>>>> this patch fixes PR81430, an ICE in the libgomp testsuite for both openmp
>>>> and
>>>> openacc test-cases for x86_64 with nvptx accelerator.
>>>>
>>>> The scenario how we hit the ICE is as follows:
>>>> - a testcase is compiled with -O2
>>>> - ix86_option_optimization_table enables
>>>>     OPT_freorder_blocks_and_partition at -O2
>>>> - cc1 writes out the flag as part of DECL_FUNCTION_SPECIFIC_OPTIMIZATION
>>>> - lto1 reads in the flag as part of DECL_FUNCTION_SPECIFIC_OPTIMIZATION
>>>> - lto1 uses the flag, and runs pass_partition_blocks
>>>> - pass_partition_blocks ICEs, because it generates code that is not
>>>>     supported by the nvptx target.
>>>>
>>>> Note that for standalone compilation for single-thread ptx execution, we
>>>> don't
>>>> attempt to run pass_partition_blocks. This is because for nvptx,
>>>> TARGET_HAVE_NAMED_SECTIONS is set to false, and this bit in finish_options
>>>> switches off pass_partition_blocks:
>>>> ...
>>>>      /* If the target requested unwind info, then turn off the
>>>>         partitioning optimization with a different message.  Likewise, if
>>>>         the target does not support named sections.  */
>>>>
>>>>     if (opts->x_flag_reorder_blocks_and_partition
>>>>         && (!targetm_common.have_named_sections
>>>>             || (opts->x_flag_unwind_tables
>>>>                 && targetm_common.unwind_tables_default
>>>>                 && (ui_except == UI_SJLJ || ui_except >= UI_TARGET))))
>>>>       {
>>>>         if (opts_set->x_flag_reorder_blocks_and_partition)
>>>>           inform (loc,
>>>>                   "-freorder-blocks-and-partition does not work "
>>>>                   "on this architecture");
>>>>         opts->x_flag_reorder_blocks_and_partition = 0;
>>>>         opts->x_flag_reorder_blocks = 1;
>>>>       }
>>>> ...
>>>>
>>>> The patch fixes this by calling finish_options in lto1 after
>>>> cl_optimization_restore.
>>>>
>>>> Points for review:
>>>> 1. I'm uncertain though about the placement of the call. Perhaps it should
>>>> be
>>>> in cl_optimization_restore, before targetm.override_options_after_change?
>>>>
>>>> 2. I think that this is offloading specific, so perhaps this should be
>>>> guarded
>>>> with lto_stream_offload_p or #ifdef ACCEL_COMPILER or some such.
>>>
>>> Hmm, I agree with #2.  I think it conceptually is a LTO stream adjustment
>>> and thus we should do this at the time we stream in the
>>> optimization/target nodes (like we remap modes for example).  Not
>>> sure if it's possible to do this at that point, but it looks like
>>> finish_options takes two option structs and thus we should be able to
>>> call it.
>>>
>>
>> With what parameters? Patch below tries with same option struct, but ...
>>
>>> Do you get the inform note?  I suppose we don't really want that, no?
>>>
>>
>> ... I think that way we'll get the inform note (while the previous solution
>> did not).
>>
>> I've also tried with a tmp2 memset to 0, but that ran into problems when doing
>> a maybe_set_param_value.
> 
> Use global_options_set?  That should do what the other patch did.
> 

I managed to get it working now.  The variable tmp was only partly 
initialized, which caused the problems when calling 
maybe_set_param_value. I'm now using init_options_struct.

There's no note when using -O2 or "-O2 -freorder-blocks-and-partition".

But when I do "-O2 -foffload=-freorder-blocks-and-partition" I get:
...
lto1: note: '-freorder-blocks-and-partition' does not work on this 
architecture
lto1: note: '-freorder-blocks-and-partition' does not support unwind 
info on this architecture
...

And for "-O0 -foffload=-freorder-blocks-and-partition" I just get:
...
lto1: note: '-freorder-blocks-and-partition' does not work on this 
architecture
...

Thanks,
- Tom

[-- Attachment #2: 0002-Call-finish_options-in-lto1.patch --]
[-- Type: text/x-patch, Size: 1335 bytes --]

Call finish_options in lto1

---
 gcc/tree-streamer-in.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/gcc/tree-streamer-in.c b/gcc/tree-streamer-in.c
index d7b6d22..eb41e75 100644
--- a/gcc/tree-streamer-in.c
+++ b/gcc/tree-streamer-in.c
@@ -33,6 +33,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "ipa-chkp.h"
 #include "gomp-constants.h"
 #include "asan.h"
+#include "opts.h"
 
 
 /* Read a STRING_CST from the string table in DATA_IN using input
@@ -769,6 +770,21 @@ lto_input_ts_function_decl_tree_pointers (struct lto_input_block *ib,
   DECL_FUNCTION_SPECIFIC_TARGET (expr) = stream_read_tree (ib, data_in);
 #endif
   DECL_FUNCTION_SPECIFIC_OPTIMIZATION (expr) = stream_read_tree (ib, data_in);
+#ifdef ACCEL_COMPILER
+  {
+    tree opts = DECL_FUNCTION_SPECIFIC_OPTIMIZATION (expr);
+    if (opts)
+      {
+	struct gcc_options tmp;
+	init_options_struct (&tmp, NULL);
+	cl_optimization_restore (&tmp, TREE_OPTIMIZATION (opts));
+	finish_options (&tmp, &global_options_set, UNKNOWN_LOCATION);
+	opts = build_optimization_node (&tmp);
+	finalize_options_struct (&tmp);
+	DECL_FUNCTION_SPECIFIC_OPTIMIZATION (expr) = opts;
+      }
+  }
+#endif
 
   /* If the file contains a function with an EH personality set,
      then it was compiled with -fexceptions.  In that case, initialize

Re: [PATCH, PR81430] Use finalize_options in lto1

[Richard Biener]

On Fri, 21 Jul 2017, Tom de Vries wrote:

> On 07/21/2017 11:41 AM, Richard Biener wrote:
> > On Thu, 20 Jul 2017, Tom de Vries wrote:
> > 
> > > On 07/20/2017 12:10 PM, Richard Biener wrote:
> > > > On Thu, 20 Jul 2017, Tom de Vries wrote:
> > > > 
> > > > > Hi,
> > > > > 
> > > > > this patch fixes PR81430, an ICE in the libgomp testsuite for both
> > > > > openmp
> > > > > and
> > > > > openacc test-cases for x86_64 with nvptx accelerator.
> > > > > 
> > > > > The scenario how we hit the ICE is as follows:
> > > > > - a testcase is compiled with -O2
> > > > > - ix86_option_optimization_table enables
> > > > >     OPT_freorder_blocks_and_partition at -O2
> > > > > - cc1 writes out the flag as part of
> > > > > DECL_FUNCTION_SPECIFIC_OPTIMIZATION
> > > > > - lto1 reads in the flag as part of
> > > > > DECL_FUNCTION_SPECIFIC_OPTIMIZATION
> > > > > - lto1 uses the flag, and runs pass_partition_blocks
> > > > > - pass_partition_blocks ICEs, because it generates code that is not
> > > > >     supported by the nvptx target.
> > > > > 
> > > > > Note that for standalone compilation for single-thread ptx execution,
> > > > > we
> > > > > don't
> > > > > attempt to run pass_partition_blocks. This is because for nvptx,
> > > > > TARGET_HAVE_NAMED_SECTIONS is set to false, and this bit in
> > > > > finish_options
> > > > > switches off pass_partition_blocks:
> > > > > ...
> > > > >      /* If the target requested unwind info, then turn off the
> > > > >         partitioning optimization with a different message.  Likewise,
> > > > > if
> > > > >         the target does not support named sections.  */
> > > > > 
> > > > >     if (opts->x_flag_reorder_blocks_and_partition
> > > > >         && (!targetm_common.have_named_sections
> > > > >             || (opts->x_flag_unwind_tables
> > > > >                 && targetm_common.unwind_tables_default
> > > > >                 && (ui_except == UI_SJLJ || ui_except >= UI_TARGET))))
> > > > >       {
> > > > >         if (opts_set->x_flag_reorder_blocks_and_partition)
> > > > >           inform (loc,
> > > > >                   "-freorder-blocks-and-partition does not work "
> > > > >                   "on this architecture");
> > > > >         opts->x_flag_reorder_blocks_and_partition = 0;
> > > > >         opts->x_flag_reorder_blocks = 1;
> > > > >       }
> > > > > ...
> > > > > 
> > > > > The patch fixes this by calling finish_options in lto1 after
> > > > > cl_optimization_restore.
> > > > > 
> > > > > Points for review:
> > > > > 1. I'm uncertain though about the placement of the call. Perhaps it
> > > > > should
> > > > > be
> > > > > in cl_optimization_restore, before
> > > > > targetm.override_options_after_change?
> > > > > 
> > > > > 2. I think that this is offloading specific, so perhaps this should be
> > > > > guarded
> > > > > with lto_stream_offload_p or #ifdef ACCEL_COMPILER or some such.
> > > > 
> > > > Hmm, I agree with #2.  I think it conceptually is a LTO stream
> > > > adjustment
> > > > and thus we should do this at the time we stream in the
> > > > optimization/target nodes (like we remap modes for example).  Not
> > > > sure if it's possible to do this at that point, but it looks like
> > > > finish_options takes two option structs and thus we should be able to
> > > > call it.
> > > > 
> > > 
> > > With what parameters? Patch below tries with same option struct, but ...
> > > 
> > > > Do you get the inform note?  I suppose we don't really want that, no?
> > > > 
> > > 
> > > ... I think that way we'll get the inform note (while the previous
> > > solution
> > > did not).
> > > 
> > > I've also tried with a tmp2 memset to 0, but that ran into problems when
> > > doing
> > > a maybe_set_param_value.
> > 
> > Use global_options_set?  That should do what the other patch did.
> > 
> 
> I managed to get it working now.  The variable tmp was only partly
> initialized, which caused the problems when calling maybe_set_param_value. I'm
> now using init_options_struct.
> 
> There's no note when using -O2 or "-O2 -freorder-blocks-and-partition".
> 
> But when I do "-O2 -foffload=-freorder-blocks-and-partition" I get:
> ...
> lto1: note: '-freorder-blocks-and-partition' does not work on this
> architecture
> lto1: note: '-freorder-blocks-and-partition' does not support unwind info on
> this architecture
> ...
> 
> And for "-O0 -foffload=-freorder-blocks-and-partition" I just get:
> ...
> lto1: note: '-freorder-blocks-and-partition' does not work on this
> architecture
> ...

Ok.

Thanks,
Richard.

> Thanks,
> - Tom
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

[committed, nvptx] Add nvptx_override_options_after_change

[Tom de Vries]

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

Hi,

this patch adds nvptx_override_options_after_change, containing a 
workaround for PR81430.

Tested on x86_64 with nvptx accelerator.

Committed.

Thanks,
- Tom

[-- Attachment #2: 0001-Add-nvptx_override_options_after_change.patch --]
[-- Type: text/x-patch, Size: 1499 bytes --]

Add nvptx_override_options_after_change

2017-07-21  Tom de Vries  <tom@codesourcery.com>

	PR lto/81430
	* config/nvptx/nvptx.c (nvptx_override_options_after_change): New
	function.
	(TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE): Define to
	nvptx_override_options_after_change.

---
 gcc/config/nvptx/nvptx.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/gcc/config/nvptx/nvptx.c b/gcc/config/nvptx/nvptx.c
index d0aa054..a718054 100644
--- a/gcc/config/nvptx/nvptx.c
+++ b/gcc/config/nvptx/nvptx.c
@@ -207,6 +207,17 @@ nvptx_option_override (void)
     target_flags |= MASK_SOFT_STACK | MASK_UNIFORM_SIMT;
 }
 
+/* Implement TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE.  */
+
+static void
+nvptx_override_options_after_change (void)
+{
+  /* This is a workaround for PR81430 - nvptx acceleration compilation broken
+     because of running pass_partition_blocks.  This should be dealt with in the
+     common code, not in the target.  */
+  flag_reorder_blocks_and_partition = 0;
+}
+
 /* Return a ptx type for MODE.  If PROMOTE, then use .u32 for QImode to
    deal with ptx ideosyncracies.  */
 
@@ -5505,6 +5516,9 @@ nvptx_data_alignment (const_tree type, unsigned int basic_align)
 #undef TARGET_OPTION_OVERRIDE
 #define TARGET_OPTION_OVERRIDE nvptx_option_override
 
+#undef TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE
+#define TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE nvptx_override_options_after_change
+
 #undef TARGET_ATTRIBUTE_TABLE
 #define TARGET_ATTRIBUTE_TABLE nvptx_attribute_table
 

Re: [committed, nvptx] Add nvptx_override_options_after_change

[Thomas Schwinge]

Hi Tom!

On Fri, 21 Jul 2017 11:49:11 +0200, Tom de Vries <Tom_deVries@mentor.com> wrote:
> this patch adds nvptx_override_options_after_change, containing a 
> workaround for PR81430.

> --- a/gcc/config/nvptx/nvptx.c
> +++ b/gcc/config/nvptx/nvptx.c
> @@ -207,6 +207,17 @@ nvptx_option_override (void)
>      target_flags |= MASK_SOFT_STACK | MASK_UNIFORM_SIMT;
>  }
>  
> +/* Implement TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE.  */
> +
> +static void
> +nvptx_override_options_after_change (void)
> +{
> +  /* This is a workaround for PR81430 - nvptx acceleration compilation broken
> +     because of running pass_partition_blocks.  This should be dealt with in the
> +     common code, not in the target.  */
> +  flag_reorder_blocks_and_partition = 0;
> +}

Should this be reverted now that r250852 "Apply finish_options on
DECL_FUNCTION_SPECIFIC_OPTIMIZATION for ACCEL_COMPILER" has been
committed?  (I'm confirming this works fine, thanks.)


Grüße
 Thomas

Re: [committed, nvptx] Add nvptx_override_options_after_change

[Tom de Vries]

On 08/08/2017 10:45 AM, Thomas Schwinge wrote:
> Hi Tom!
> 
> On Fri, 21 Jul 2017 11:49:11 +0200, Tom de Vries <Tom_deVries@mentor.com> wrote:
>> this patch adds nvptx_override_options_after_change, containing a
>> workaround for PR81430.
> 
>> --- a/gcc/config/nvptx/nvptx.c
>> +++ b/gcc/config/nvptx/nvptx.c
>> @@ -207,6 +207,17 @@ nvptx_option_override (void)
>>       target_flags |= MASK_SOFT_STACK | MASK_UNIFORM_SIMT;
>>   }
>>   
>> +/* Implement TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE.  */
>> +
>> +static void
>> +nvptx_override_options_after_change (void)
>> +{
>> +  /* This is a workaround for PR81430 - nvptx acceleration compilation broken
>> +     because of running pass_partition_blocks.  This should be dealt with in the
>> +     common code, not in the target.  */
>> +  flag_reorder_blocks_and_partition = 0;
>> +}
> 
> Should this be reverted now that r250852 "Apply finish_options on
> DECL_FUNCTION_SPECIFIC_OPTIMIZATION for ACCEL_COMPILER" has been
> committed?  (I'm confirming this works fine, thanks.)

Hi,

Indeed, it can be reverted.

Thanks,
- Tom

Re: [committed, nvptx] Add nvptx_override_options_after_change

[Thomas Schwinge]

Hi!

On Tue, 8 Aug 2017 10:58:12 +0200, Tom de Vries <Tom_deVries@mentor.com> wrote:
> On 08/08/2017 10:45 AM, Thomas Schwinge wrote:
> > On Fri, 21 Jul 2017 11:49:11 +0200, Tom de Vries <Tom_deVries@mentor.com> wrote:
> >> this patch adds nvptx_override_options_after_change, containing a
> >> workaround for PR81430.

> > Should this be reverted now that r250852 "Apply finish_options on
> > DECL_FUNCTION_SPECIFIC_OPTIMIZATION for ACCEL_COMPILER" has been
> > committed?  (I'm confirming this works fine, thanks.)
> 
> Indeed, it can be reverted.

Committed to trunk in r251053:

commit 5b005e86c8c45aa84c090064b2f789c802384fc5
Author: tschwinge <tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Fri Aug 11 15:37:14 2017 +0000

    [PR lto/81430] Revert "Add nvptx_override_options_after_change"
    
    This reverts r250421; properly fixed by r250852.
    
            PR lto/81430
            * config/nvptx/nvptx.c (nvptx_override_options_after_change):
            Remove function.
            (TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE): Remove definition.
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@251053 138bc75d-0d04-0410-961f-82ee72b054a4
---
 gcc/ChangeLog            |  7 +++++++
 gcc/config/nvptx/nvptx.c | 14 --------------
 2 files changed, 7 insertions(+), 14 deletions(-)

diff --git gcc/ChangeLog gcc/ChangeLog
index 2975655..295b4c9 100644
--- gcc/ChangeLog
+++ gcc/ChangeLog
@@ -1,3 +1,10 @@
+2017-08-11  Thomas Schwinge  <thomas@codesourcery.com>
+
+	PR lto/81430
+	* config/nvptx/nvptx.c (nvptx_override_options_after_change):
+	Remove function.
+	(TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE): Remove definition.
+
 2017-08-11  Tamar Christina  <tamar.christina@arm.com>
 	* config/aarch64/aarch64.md (mov<mode>): Change.
 	(*movhf_aarch64, *movsf_aarch64, *movdf_aarch64):
diff --git gcc/config/nvptx/nvptx.c gcc/config/nvptx/nvptx.c
index ffd50d7..8babac7 100644
--- gcc/config/nvptx/nvptx.c
+++ gcc/config/nvptx/nvptx.c
@@ -212,17 +212,6 @@ nvptx_option_override (void)
     target_flags |= MASK_SOFT_STACK | MASK_UNIFORM_SIMT;
 }
 
-/* Implement TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE.  */
-
-static void
-nvptx_override_options_after_change (void)
-{
-  /* This is a workaround for PR81430 - nvptx acceleration compilation broken
-     because of running pass_partition_blocks.  This should be dealt with in the
-     common code, not in the target.  */
-  flag_reorder_blocks_and_partition = 0;
-}
-
 /* Return a ptx type for MODE.  If PROMOTE, then use .u32 for QImode to
    deal with ptx ideosyncracies.  */
 
@@ -5527,9 +5516,6 @@ nvptx_data_alignment (const_tree type, unsigned int basic_align)
 #undef TARGET_OPTION_OVERRIDE
 #define TARGET_OPTION_OVERRIDE nvptx_option_override
 
-#undef TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE
-#define TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE nvptx_override_options_after_change
-
 #undef TARGET_ATTRIBUTE_TABLE
 #define TARGET_ATTRIBUTE_TABLE nvptx_attribute_table
 


Grüße
 Thomas