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