[PATCH] Fix PR56888

Jan Hubicka hubicka@ucw.cz
Tue Feb 23 10:25:00 GMT 2016


> On Mon, 22 Feb 2016, Jakub Jelinek wrote:
> 
> > On Mon, Feb 22, 2016 at 01:44:09PM +0100, Richard Biener wrote:
> > > --- 1079,1086 ----
> > >   	  || !dominated_by_p (CDI_DOMINATORS,
> > >   			      loop->latch, gimple_bb (stmt)))
> > >   	return;
> > > +       if (cgraph_node::get (cfun->decl)->aliases (BUILT_IN_MEMSET))
> > > + 	return;
> > 
> > Perhaps also punt here for:
> > BUILT_IN_MEMSET_CHK
> > BUILT_IN_TM_MEMSET
> > BUILT_IN_BZERO
> > ?
> > 
> > >         partition->kind = PKIND_MEMSET;
> > >         partition->main_dr = single_store;
> > >         partition->niter = nb_iter;
> > > *************** classify_partition (loop_p loop, struct
> > > *** 1135,1140 ****
> > > --- 1138,1146 ----
> > >   	}
> > >         free_dependence_relation (ddr);
> > >         loops.release ();
> > > +       if (cgraph_node::get (cfun->decl)->aliases (BUILT_IN_MEMCPY)
> > > + 	  || cgraph_node::get (cfun->decl)->aliases (BUILT_IN_MEMMOVE))
> > > + 	return;
> > 
> > And here for
> > BUILT_IN_MEMCPY_CHK
> > BUILT_IN_TM_MEMCPY
> > BUILT_IN_TM_MEMCPY_RNWT
> > BUILT_IN_TM_MEMCPY_RTWN
> > BUILT_IN_MEMPCPY
> > BUILT_IN_MEMPCPY_CHK
> > BUILT_IN_MEMMOVE_CHK
> > BUILT_IN_TM_MEMMOVE
> > BUILT_IN_BCOPY
> > ?
> 
> I'm going to wait for Honzas feedback.  Testing all of the above
> looks expensive - if those are implemented in terms of memcpy/memset
> then hopefully in libc itself which hopefully always does local
> calls.

Well, other problem will be if memcpy itself is implemented by calling a
function that does the actual copying job (for example, for large block sizes
or so). So for ultimate QOI fix you really want to check if the function you
are optimizing is reachable from any of those bultins. This is of course
doable by means of simple propagation pass, but I am not sure if it makes
sense to get that far.

The checks however should not be that bad - they are done only when we
recognize the memcpy pattern and want to optimize it and that is not too common
case, right?
> 
> It's not going to be an exhaustive check anyway, just a QOI one
> covering those cases we've seen in the wild.  Esp. as indirection
> will break the detection (so does using asm(".alias XXX") which glibc
> does).

To lookup definition of the bulitin, you don't really need assembler name
lookup.  Just get cgraph node for the builtin decl and look
next_sharing_asm_name and prev_shaing_asm_name lists (this works only when asm
name hash is initialized, so to be sure just call
symtab->symtab_initialize_asm_name_hash ();

For next stage1 I have queued patch that makes duplicated decls to be
transparent alises, so you will need to only lookup the ultimate alias target.
I did not commited it to mainline becuase it requires lto-symtab like linking
to be done each time new duplciate is inserted into the symbol table and it
tends to find new user errors on misuse of asm("...") constructs.
Also one needs to fix the chkp issues....

Honza
> 
> Richard.



More information about the Gcc-patches mailing list