On Mon, Oct 29, 2018 at 11:46 AM Keane, Erich <erich.ke...@intel.com> wrote:
> > > > > *From:* David Blaikie [mailto:dblai...@gmail.com] > *Sent:* Monday, October 29, 2018 11:41 AM > *To:* Keane, Erich <erich.ke...@intel.com> > *Cc:* Eric Christopher <echri...@gmail.com>; cfe-commits@lists.llvm.org > > > *Subject:* Re: r344957 - Give Multiversion-inline functions linkonce > linkage > > > > > > On Mon, Oct 29, 2018 at 11:30 AM Keane, Erich <erich.ke...@intel.com> > wrote: > > GCC actually doesn’t support function multiversioning in C mode, so this > fix is part of our extension to support C with multiversioning. > > > Ah, what's the motivation for that? > > *[Keane, Erich] Basically, I identified no good reason to not support it. > GCC’s motivation is lost to time as far as I can tell. This > multiversioning is super useful in my employer’s math/intrinsic libraries > (MKL uses this feature extensively), so enabling it in C seemed like a > positive improvement. We’ve made some other behavioral changes to make > ‘target’ work more sanely to suppress some of the worst bugs (becoming MV > after usage) as well as make it work better with Clang’s > emit-functionality.* > > > > > I perhaps wonder if this is part of the reason GCC only supports it in > C++ mode... > > > Yeah... could be. > > I mean I suppose using linkonce isn't making multiversioning any worse in > C than it already is in C++ inline functions (which is where it's pretty > easy to misuse, as I understand it - two files, one compiled with a CPU > feature, one without, linkonce functions get linked together & the linker > picks one - then code built without the feature calls code that uses the > feature - but, I mean, that can happen even witohut multiversioning, if you > compile some source files with a feature and some without) - I guess the > main risk would be if it confuses users by making multiversioned C inline > functions end up behaving too differently from non-multiversioned ones... > > *[Keane, Erich] I suspect that multiversioning is such a sharp edged > feature, that a change like this makes it a touch more sane. Since the > individual versions are independently mangled, you don’t have to worry > about merging different versions. The only real issue comes from merging > the resolver functions if the visible list of declarations is different, > but we treat that as an ODR violation.* > > > Rightio - thanks for all the context (: > > - Dave > > > > > *From:* David Blaikie [mailto:dblai...@gmail.com] > *Sent:* Monday, October 29, 2018 11:25 AM > *To:* Keane, Erich <erich.ke...@intel.com>; Eric Christopher < > echri...@gmail.com> > *Cc:* cfe-commits@lists.llvm.org > *Subject:* Re: r344957 - Give Multiversion-inline functions linkonce > linkage > > > > Does this match GCC's approach here? > > (I ask this sort of as throwaway/conversation starter - because the > linkage/behavior around multiversion functions and their inlining is full > of sharp corners/risks of code moving out of the areas appropriately > restricted based on the cpu features) > > On Mon, Oct 22, 2018 at 2:22 PM Erich Keane via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > > Author: erichkeane > Date: Mon Oct 22 14:20:45 2018 > New Revision: 344957 > > URL: http://llvm.org/viewvc/llvm-project?rev=344957&view=rev > Log: > Give Multiversion-inline functions linkonce linkage > > Since multiversion variant functions can be inline, in C they become > available-externally linkage. This ends up causing the variants to not > be emitted, and not available to the linker. > > The solution is to make sure that multiversion functions are always > emitted by marking them linkonce. > > Change-Id: I897aa37c7cbba0c1eb2c57ee881d5000a2113b75 > > Modified: > cfe/trunk/lib/CodeGen/CodeGenModule.cpp > cfe/trunk/test/CodeGen/attr-target-mv.c > > Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=344957&r1=344956&r2=344957&view=diff > > ============================================================================== > --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original) > +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Mon Oct 22 14:20:45 2018 > @@ -3669,6 +3669,10 @@ llvm::GlobalValue::LinkageTypes CodeGenM > return llvm::GlobalVariable::WeakAnyLinkage; > } > > + if (const auto *FD = D->getAsFunction()) > + if (FD->isMultiVersion() && Linkage == GVA_AvailableExternally) > + return llvm::GlobalVariable::LinkOnceAnyLinkage; > + > // We are guaranteed to have a strong definition somewhere else, > // so we can use available_externally linkage. > if (Linkage == GVA_AvailableExternally) > > Modified: cfe/trunk/test/CodeGen/attr-target-mv.c > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/attr-target-mv.c?rev=344957&r1=344956&r2=344957&view=diff > > ============================================================================== > --- cfe/trunk/test/CodeGen/attr-target-mv.c (original) > +++ cfe/trunk/test/CodeGen/attr-target-mv.c Mon Oct 22 14:20:45 2018 > @@ -88,19 +88,19 @@ void bar4() { > > // CHECK: declare i32 @foo.arch_sandybridge() > > -// CHECK: define available_externally i32 @foo_inline.sse4.2() > +// CHECK: define linkonce i32 @foo_inline.sse4.2() > // CHECK: ret i32 0 > > // CHECK: declare i32 @foo_inline.arch_sandybridge() > // > -// CHECK: define available_externally i32 @foo_inline.arch_ivybridge() > +// CHECK: define linkonce i32 @foo_inline.arch_ivybridge() > // CHECK: ret i32 1 > -// CHECK: define available_externally i32 @foo_inline() > +// CHECK: define linkonce i32 @foo_inline() > // CHECK: ret i32 2 > > -// CHECK: define available_externally void @foo_decls() > -// CHECK: define available_externally void @foo_decls.sse4.2() > +// CHECK: define linkonce void @foo_decls() > +// CHECK: define linkonce void @foo_decls.sse4.2() > > -// CHECK: define available_externally void @foo_multi.avx_sse4.2() > -// CHECK: define available_externally void @foo_multi.fma4_sse4.2() > -// CHECK: define available_externally void > @foo_multi.arch_ivybridge_fma4_sse4.2() > +// CHECK: define linkonce void @foo_multi.avx_sse4.2() > +// CHECK: define linkonce void @foo_multi.fma4_sse4.2() > +// CHECK: define linkonce void @foo_multi.arch_ivybridge_fma4_sse4.2() > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits