Re: [PATCH] Fix LTO option streaming

[Richard Biener <rguenther at suse dot de>]

On Thu, 23 Apr 2015, Richard Biener wrote:

> On Thu, 23 Apr 2015, Jan Hubicka wrote:
> 
> > > 
> > > It looks like when transitioning to using target and optimization
> > > option nodes for compile-time to link-time option streaming you
> > > didn't adjust lto-opts.c nor lto-wrapper.c.  The following fixes
> > 
> > Yep, I assumed that lto-wrapper's merging is now redundant for optimization
> > options, while it is still needed for global functions (-fPIC) that was
> > not covered by my changes and since this was all bit late, it seemed I can
> > just keep it for next stage1.
> > 
> > > the target option side (for SWITCHABLE_TARGET).  Do not record
> > > any target options in the lto_opts section.
> > > 
> > > Honza - I suppose we don't have any testcase that this works, I'll
> > > try to come up with sth.  This also looks like a correctness issue
> > > to me.
> > 
> > Thanks, that would be a good idea. So with SWITCHABLE_TARGET
> > we have problem that after switching the options are misinterpretted?

Ok, so with

t.c
---
extern int have_fma;
extern double x, y, z;
void do_fma (void);
int main()
{
  if (have_fma)
    do_fma ();
  else
    x = x + y * z;
}

t2.c
---
int have_fma;
volatile double x, y, z;
void do_fma (void)
{
  x = x + y * z;
}

and doing

gcc -c -Ofast -flto t.c
gcc -c -Ofast -mfma -flto t2.c
gcc t.o t2.o

I get -mfma on the lto1 command-line for the link.  But I also get for 
main()

 <target_option_node 0x7ffff6ac40d8
    ix86_isa_flags (0x1800220000012)
    ix86_fpmath (0x2)
    target_flags (0xa0004c3)
    arch = 18 (k8)
    tune = 0 (generic)
    branch_cost = 3
    -m64 -msse2 -msse -mmmx -mfxsr -m128bit-long-double -m80387 
-mfp-ret-in-387 -mtls-direct-seg-refs -mvzeroupper 
-mavx256-split-unaligned-load -mavx256-split-unaligned-store -mfpmath=sse
>

while for do_fma()

 <target_option_node 0x7ffff6ac2e58
    ix86_isa_flags (0x117809224000112)
    ix86_fpmath (0x2)
    target_flags (0xa0004c3)
    arch = 18 (k8)
    tune = 0 (generic)
    branch_cost = 3
    -m64 -mfma -msse4.2 -msse4.1 -mssse3 -msse3 -msse2 -msse -mmmx -mfxsr 
-mpopcnt -mxsave -m128bit-long-double -m80387 -mfp-ret-in-387 
-mtls-direct-seg-refs -mvzeroupper -mavx256-split-unaligned-load 
-mavx256-split-unaligned-store -mfpmath=sse
>

and for some reason main () ends up _not_ having -mfma enabled.

I'm not sure what disables it or how we arrive at the above set of
options in the first place...  does the TARGET_OPTION_NODE store
the complete state of all target options?  I had the impression
it only stores "modification"?

So it seems to work (and simply ignore the global -mfma flag).  Not
sure on whether by design or by luck ;)

Richard.


> > I think Jakub made patch to simply skip target-option-node streaming here,
> > how this is supposed to work.
> 
> Not all targets support target attributes and I think those that
> support them are those that are SWITCHABLE_TARGET - but maybe I am
> mistaken?
> 
> > > 
> > > We can do similar changes for optimize options, now, for all targets, no?
> > 
> > I am not quite sure about this; not all options are per-function nature.
> > I need to audit all the target options for LTO safety, I did some work
> > on this for optimization options, but the target options are still very wild.
> > For example I am not quite convinced mixing -malign-double -mno-align-double units
> > will work as expected.
> 
> Well, but once types are laid out specifying sth else at link time
> won't help.  But yes, if we don't stream the option we have no chance
> to complain in lto-wrapper.  It's just that we seem to not enter
> the merging machinery for single-file testcases so I chose to "fix"
> lto option streaming rather than lto-wrapper ... I'll have a closer
> look to lto-wrapper.  But to complain about target option mismatches
> we'd have to call a target hook from lto-wrapper (or finally re-structure
> all this and merge options in lto1 itself).
> 
> Richard.
> 
> > Honza
> > > 
> > > Thanks,
> > > Richard.
> > > 
> > > 2015-04-23  Richard Biener  <rguenther@suse.de>
> > > 
> > > 	* lto-opts.c (lto_write_options): Do not record target options
> > > 	if we are targeting a SWITCHABLE_TARGET target.  We use target
> > > 	options on functions to transfer target flags from compile to
> > > 	link time.
> > > 
> > > Index: gcc/lto-opts.c
> > > ===================================================================
> > > *** gcc/lto-opts.c	(revision 222360)
> > > --- gcc/lto-opts.c	(working copy)
> > > *************** lto_write_options (void)
> > > *** 219,224 ****
> > > --- 219,230 ----
> > >   	  && lto_stream_offload_p)
> > >          continue;
> > >   
> > > +       /* Do not store target-specific options if we target a
> > > +          SWITCHABLE_TARGET target.  */
> > > +       if ((cl_options[option->opt_index].flags & CL_TARGET)
> > > + 	  && SWITCHABLE_TARGET)
> > > + 	continue;
> > > + 
> > >         /* Drop options created from the gcc driver that will be rejected
> > >   	 when passed on to the driver again.  */
> > >         if (cl_options[option->opt_index].cl_reject_driver)
> > 
> > 
> 
> 

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