Re: [PATCH, PR81430] Use finalize_options in lto1

[Richard Biener <rguenther@suse.de>]

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)