Thanks for the comments! New patch coming up.
================ Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:355 @@ +354,3 @@ + /// \brief Generate a thunk for calling a virtual member function MD. + llvm::Function *EmitVirtualMemPtrThunk(const CXXMethodDecl *MD, + StringRef ThunkName); ---------------- Timur Iskhodzhanov wrote: > Does this need to be public? Nope. Fixed. ================ Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1932 @@ +1931,3 @@ + I != E; ++I) + FunctionArgs.push_back(*I); + ---------------- Timur Iskhodzhanov wrote: > Can this be shared with the general thunk emission code? > I'm worried by the duplication of non-trivial code. > > I think pretty much everything except this/return adjustment, call target and > the particular linkage type can be shared. I've tried to break it out into two functions: one for setting up the function and one for making the call and return. ================ Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1972 @@ +1971,3 @@ + CGF.FinishFunction(); + CGM.setFunctionLinkage(MD, ThunkFn); // XXX: Do we need this? + ---------------- Reid Kleckner wrote: > Timur Iskhodzhanov wrote: > > Reid Kleckner wrote: > > > Timur Iskhodzhanov wrote: > > > > Isn't this copying the function's linkage over to the thunk? > > > > Don't you get "symbol already defined" if you link two TUs generating > > > > the same thunk together? > > > These should be merged like inline functions. They should be > > > linkonce_odr for externally visible types and internal for internal > > > types. We probably have logic for that somewhere. > > I don't see this in the lit test expectations: > > > > define x86_thiscallcc void @"\01??_9C@@$B7AE" > Yes, I agree with you, the logic needs to change. :) I'm just adding that > if the type is internal than we can make the thunk internal too, which is > slightly better for the optimizers. We can probably reuse some existing code > for this. Yeah, I think the answer to "Do we need this?" is "no" :) That means they get linkonce_odr linkage, which should work right? http://llvm-reviews.chandlerc.com/D2104 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
