Re: [PATCH] Fix PR60319, mixing -f[no-]strict-overflow units with LTO

[Richard Biener <rguenther at suse dot de>]

On Mon, 24 Feb 2014, Jan Hubicka wrote:

> > 
> > This addresses miscompilations caused by improperly mixing
> > -f[no-]strict-overflow units (and thus optimize <= 1 and optimize >= 2
> > units!).  It does so by merging -f[no-]strict-overflow and the
> > related -f[no-]wrapv and -f[no-]trapv conservatively, that is,
> > if any unit uses (implicitely) -fno-strict-overflow, -fwrapv
> > or -fno-trapv then force that upon the compilation in and after
> > WPA stage.
> > 
> > I know honza wants to eventually handle this via optimize attributes
> > but currently at least -fstrict-overflow is not suitable for that
> > (it's not marked 'Optimization').  Nor do we even check for
> > -f[no-]wrapv mismatch in can_inline_edge_p.
> 
> Yep, we need to fix all that for 4.10
>
> > LTO bootstrap & test ongoing on x86_64-unknown-linux-gnu, ok for trunk?
> > 
> > Any other "obvious" candidates for conservative merging?
> > 
> > Note that this makes it quite harmful (well - maybe?) if your
> > LTO compile contains a single non-optimized TU ... but you
> > at least know if you inspect the lto_opts section of one of the
> > ltrans files.
> 
> Indeed, i think we should at least mention that in news.html and perhsps
> have short section on LTO and flags in the doc/invoke.texi?

I'll try to come up with sth for invoke.texi / lto.texi.

> Users generally get wrong the fact that they need -flto and optimization
> flags at command line and we do crazy things behind their back.
> Like last week I noticed that we build a static binary of Firefox's shell
> with -fpic just because they manage to merge in an object file copiled -fPIC.
> (while LLVM give non-PIC binary)
> -fno-pic helps there, what will bebehaviour with -fstrict-overflow?

Options given at link-time will override those constructed from the
input TUs as far as the option processing machinery handles it
(more specific options always trump general ones, so -O2 at link-time
doesn't override -fno-strict-overflow from compile-time).  This is
implemented by simply appending all link-time options to those
built from compile-time.

> > Eventually this asks for a diagnostic by lto-wrapper (but that
> > gets delayed until very late ...).
> 
> I am definitely trying to push myself to make command line options an 
> priority for 4.10 stage1. Lets try to wrok on it, I think it is one of 
> main sowstoppers for LTO at the moment. (along with debug info and fun 
> with ASM statements definitng symbols I suppose)

I think the basic scheme implemented right now is sound - we have to
pass on some options from compile to link-time for correctness reasons.

What is missing is

 1) diagnosing harmful mismatches
 2) diagnose options that are dropped (we don't know which at the moment
    as we don't stream all options from compile-time)
 3) pass on the general optimization level (don't default to -O0)
 4) eventually use optimize/target attributes to mitigate 1) and 2)

I would go 4) at the very last resort only - it's not a good solution
IMHO (maybe a good solution for target options, but certainly not for
general opts).

Oh, and of course we need to improve our IL so that its meaning
doesn't depend on flags like -fwrapv.

I have committed the patch now.

Thanks,
Richard.

> Honza
> > 
> > Thanks,
> > Richard.
> > 
> > 2014-02-24  Richard Biener  <rguenther@suse.de>
> > 
> > 	PR lto/60319
> > 	* lto-opts.c (lto_write_options): Output non-explicit conservative
> > 	-fwrapv, -fno-trapv and -fno-strict-overflow.
> > 	* lto-wrapper.c (merge_and_complain): Handle merging those options.
> > 	(run_gcc): And pass them through.
> > 
> > Index: gcc/lto-opts.c
> > ===================================================================
> > *** gcc/lto-opts.c	(revision 208077)
> > --- gcc/lto-opts.c	(working copy)
> > *************** lto_write_options (void)
> > *** 117,122 ****
> > --- 117,134 ----
> >         default:
> >   	gcc_unreachable ();
> >         }
> > +   /* We need to merge -f[no-]strict-overflow, -f[no-]wrapv and -f[no-]trapv
> > +      conservatively, so stream out their defaults.  */
> > +   if (!global_options_set.x_flag_wrapv
> > +       && global_options.x_flag_wrapv)
> > +     append_to_collect_gcc_options (&temporary_obstack, &first_p, "-fwrapv");
> > +   if (!global_options_set.x_flag_trapv
> > +       && !global_options.x_flag_trapv)
> > +     append_to_collect_gcc_options (&temporary_obstack, &first_p, "-fno-trapv");
> > +   if (!global_options_set.x_flag_strict_overflow
> > +       && !global_options.x_flag_strict_overflow)
> > +     append_to_collect_gcc_options (&temporary_obstack, &first_p,
> > + 			       "-fno-strict-overflow");
> >   
> >     /* Output explicitly passed options.  */
> >     for (i = 1; i < save_decoded_options_count; ++i)
> > Index: gcc/lto-wrapper.c
> > ===================================================================
> > *** gcc/lto-wrapper.c	(revision 208077)
> > --- gcc/lto-wrapper.c	(working copy)
> > *************** merge_and_complain (struct cl_decoded_op
> > *** 422,427 ****
> > --- 422,429 ----
> >   	    append_option (decoded_options, decoded_options_count, foption);
> >   	  break;
> >   
> > + 	case OPT_ftrapv:
> > + 	case OPT_fstrict_overflow:
> >   	case OPT_ffp_contract_:
> >   	  /* For selected options we can merge conservatively.  */
> >   	  for (j = 0; j < *decoded_options_count; ++j)
> > *************** merge_and_complain (struct cl_decoded_op
> > *** 429,439 ****
> >   	      break;
> >   	  if (j == *decoded_options_count)
> >   	    append_option (decoded_options, decoded_options_count, foption);
> > ! 	  /* FP_CONTRACT_OFF < FP_CONTRACT_ON < FP_CONTRACT_FAST.  */
> >   	  else if (foption->value < (*decoded_options)[j].value)
> >   	    (*decoded_options)[j] = *foption;
> >   	  break;
> >   
> >   	case OPT_freg_struct_return:
> >   	case OPT_fpcc_struct_return:
> >   	  for (j = 0; j < *decoded_options_count; ++j)
> > --- 431,455 ----
> >   	      break;
> >   	  if (j == *decoded_options_count)
> >   	    append_option (decoded_options, decoded_options_count, foption);
> > ! 	  /* FP_CONTRACT_OFF < FP_CONTRACT_ON < FP_CONTRACT_FAST,
> > ! 	     -fno-trapv < -ftrapv,
> > ! 	     -fno-strict-overflow < -fstrict-overflow  */
> >   	  else if (foption->value < (*decoded_options)[j].value)
> >   	    (*decoded_options)[j] = *foption;
> >   	  break;
> >   
> > + 	case OPT_fwrapv:
> > + 	  /* For selected options we can merge conservatively.  */
> > + 	  for (j = 0; j < *decoded_options_count; ++j)
> > + 	    if ((*decoded_options)[j].opt_index == foption->opt_index)
> > + 	      break;
> > + 	  if (j == *decoded_options_count)
> > + 	    append_option (decoded_options, decoded_options_count, foption);
> > + 	  /* -fwrapv > -fno-wrapv.  */
> > + 	  else if (foption->value > (*decoded_options)[j].value)
> > + 	    (*decoded_options)[j] = *foption;
> > + 	  break;
> > + 
> >   	case OPT_freg_struct_return:
> >   	case OPT_fpcc_struct_return:
> >   	  for (j = 0; j < *decoded_options_count; ++j)
> > *************** run_gcc (unsigned argc, char *argv[])
> > *** 591,596 ****
> > --- 607,615 ----
> >   	case OPT_freg_struct_return:
> >   	case OPT_fpcc_struct_return:
> >   	case OPT_ffp_contract_:
> > + 	case OPT_fwrapv:
> > + 	case OPT_ftrapv:
> > + 	case OPT_fstrict_overflow:
> >   	  break;
> >   
> >   	default:
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer