Produce direct calls instead of alias to linkonce_odr functions

Reid Kleckner rnk at google.com
Tue Nov 5 13:23:05 PST 2013


Looks good, I'm fairly confident this will DTRT.

As a followup someone should remove the hack at the top of
GetAddrOfCXXDestructor.

+  typedef llvm::StringMap<llvm::TrackingVH<llvm::Constant> >
ReplacemensType;
+  ReplacemensType Replacements;

typo

Do we prefer Type or Ty for typedefs?

+void CodeGenModule::applyReplacements() {
+  for (ReplacemensType::iterator I = Replacements.begin(),
+                                 E = Replacements.end();
+    StringRef MangledName = I->first();

I don't know enough C++ to know why the ()'s are needed here.



On Tue, Nov 5, 2013 at 1:04 PM, Rafael EspĂ­ndola <rafael.espindola at gmail.com
> wrote:

> Now with the patch actually attached.
>
>
> On 5 November 2013 00:33, Rafael EspĂ­ndola <rafael.espindola at gmail.com>
> wrote:
> >>> What happens is that at the end of GetOrCreateLLVMFunction we check if
> >>> we have a decl defined in class and if so add it to
> >>> DeferredDeclsToEmit. The net result is that we end up running
> >>> replaceAllUsesWith twice when something in DeferredDeclsToEmit causes
> >>> a new use of the destructor/constructor.
> >>>
> >>> Do you think this is OK? If not I can add a list of "DeclsToReplace"
> >>> to Codegen and process that after EmitDeferred.
> >>
> >>
> >> Ah, that makes sense.  Sounds fine, if a little bit silly.
> >
> > You were right. The example I tried worked because there was a call to
> > GetOrCreateLLVMFunction before every try to emit the decl, so we would
> > recreate the declaration before trying to emit it in EmitDeferred.
> >
> > With
> >
> > struct Option {
> >   virtual ~Option() {}
> > };
> > template <class DataType> class opt : public Option {};
> > template class opt<int>;
> > bool handleOccurrence() {
> >   Option x;
> >   return true;
> > }
> >
> > the template instantiation will not cause a declaration to be created
> > and we assert when trying to replace the value the second time.
> >
> > The attached patch fixes this by doing all replacements after the
> > deferred decls have been emitted. With this approach it should then be
> > easy to also handle
> >
> > class foo {
> >   ~foo();
> > }
> > class bar : public foo {
> > }
> >
> > i.e., when we know a function is equal to one we don't have a definition
> for.
> >
> > Cheers,
> > Rafael
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131105/253dfe47/attachment.html>


More information about the cfe-commits mailing list