On Thu, Feb 29, 2024 at 6:34 AM Jan Hubicka <hubi...@ucw.cz> wrote: > > > On Thu, Feb 29, 2024 at 5:39 AM Jan Hubicka <hubi...@ucw.cz> wrote: > > > > > > > We can't instrument an IFUNC resolver nor its callees as it may require > > > > TLS which hasn't been set up yet when the dynamic linker is resolving > > > > IFUNC symbols. Add an IFUNC resolver caller marker to symtab_node to > > > > avoid recursive checking. > > > > > > > > gcc/ChangeLog: > > > > > > > > PR tree-optimization/114115 > > > > * cgraph.h (enum ifunc_caller): New. > > > > (symtab_node): Add has_ifunc_caller. > > > Unless we have users outside of tree-profile, I think it is better to > > > avoid adding extra data to cgraph_node. One can use node->get_uid() > > > indexed hash > > > set to save the two bits needed for propagation. > > > > * tree-profile.cc (check_ifunc_resolver): New. > > > > (is_caller_ifunc_resolver): Likewise. > > > > (tree_profiling): Don't instrument an IFUNC resolver nor its > > > > callees. > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > PR tree-optimization/114115 > > > > * gcc.dg/pr114115.c: New test. > > > > > > The problem with this approach is that tracking callees of ifunc > > > resolvers will stop on the translation unit boundary and also with > > > indirect call. Also while ifunc resolver itself is called only once, > > > its callees may also be used from performance critical code. > > > > IFUNC selector shouldn't have any external dependencies which > > can cause issues at run-time. > > I am worried about scenario where ifunc selector calls function foo > defined locally and foo is also used from other places possibly in hot > loops. > > > > > So it is not really reliable fix (though I guess it will work a lot of > > > common code). I wonder what would be alternatives. In GCC generated > > > profling code we use TLS only for undirect call profiling (so there is > > > no need to turn off rest of profiling). I wonder if there is any chance > > > to not make it seffault when it is done before TLS is set up? > > > > IFUNC selector should make minimum external calls, none is preferred. > > Edge porfiling only inserts (atomic) 64bit increments of counters. > If target supports these operations inline, no external calls will be > done. > > Indirect call profiling inserts the problematic TLS variable (to track > caller-callee pairs). Value profiling also inserts various additional > external calls to counters. > > I am perfectly fine with disabling instrumentation for ifunc selectors > and functions only reachable from them, but I am worried about calles > used also from non-ifunc path.
Programmers need to understand not to do it. > For example selector implemented in C++ may do some string handling to > match CPU name and propagation will disable profiling for std::string On x86, they should use CPUID, not string functions. > member functions (which may not be effective if comdat section is > prevailed from other translation unit). String functions may lead to external function calls which is dangerous. > > Any external calls may lead to issues at run-time. It is a very bad idea > > to profile IFUNC selector via external function call. > > Looking at https://sourceware.org/glibc/wiki/GNU_IFUNC > there are other limitations on ifunc except for profiling, such as > -fstack-protector-all. So perhaps your propagation can be used to > disable those features as well. So, it may not be tree-profile specific. Where should these 2 bits be added? > "Unfortunately there are actually a lot of restrictions placed on IFUNC > usage which aren't entirely clear and the documentation needs to be > updated." makes me wonder what other transformations are potentially > dangerous. > > Honza -- H.J.
