> >Of course it also depends what you inline into function. You can have > > > >bar() target(-mavx) {fancy avx code} > >foobar() { ...... if (avx) bar();} > >foo() ctarget(-mavx,-mno-avx) {....foobar();....} > > > >Now if you compile with -mavx and because ctarget takes effect only after > >inlining, > >at inlining time the target attributes will match and we can edn up inline > >bar->foobar->foo. > >After that we multiversion foo and drop AVX flag we will likely get ICE at > >expansion > >time. > But isn't that avoided by fixing up the call graph so that all calls > to the affected function are going through the dispatcher? Or is > that happening too late?
There is dispatcher only for foo that is the root of the callgarph tree. When inlining we compare target attributes for match (in can_inline_edge_p). We do not compare ctarget attributes. Expanding ctarget to target early would avoid need for ctarget handling. > > > > >> > >> > >>Since we're going through a dispatcher I don't think we're allowed > >>to change the ABI across the clones. > > > >If the dispatcher was something like > > > >switch(value) > >{ > > case 1: foo_ver1(param); break; > > case 2: foo_ver2(param); break; > >} > >then it would be possible to change ABI if we can prove that param=0 in > >ipa-cp. > >If the dispatcher uses indirect calls, then we probably want to consider > >foo_ver* as having address taken and thus not local. > Yea, I guess that could work. For some reason I kept thinking about > indirect calls, but that shouldn't be the case here. > > That argues further that setting TREE_PUBLIC on the target specific > implementations is wrong. Yep, if the function doesn't need to be public then the whole block of code tampering with visibility and other flags can go. We probably also don't need to care about giving clone some fixed assembler name as clonning will invent .target_clone.XYZ name itself. Honza > > > Jeff