rnk added a comment. Seems reasonable. Should the resolver still be called `?foo@.resolver`, or should it get a new name, since it is quite functionally different now?
================ Comment at: lib/CodeGen/CodeGenFunction.cpp:2381 + +template<typename BuilderTy> +static void CreateMultiVersionResolverReturn(llvm::Function *Resolver, ---------------- Rather than templating over IRBuilder and CGBuilderTy, I'd standardize on CGBuilderTy. ================ Comment at: lib/CodeGen/CodeGenFunction.cpp:2391-2393 + llvm::SmallVector<llvm::Value*, 10> Args; + llvm::for_each(Resolver->args(), + [&](llvm::Argument &Arg) { Args.push_back(&Arg); }); ---------------- Surely this would be simpler as a range for or `Args{Resolver->arg_begin() ...};` ================ Comment at: lib/CodeGen/CodeGenFunction.cpp:2395 + + llvm::CallInst *Result = Builder.CreateCall(FuncToReturn, Args); + ---------------- This approach is... not going to work in the general case. Consider, for example, varargs. Then consider 32-bit x86 inalloca, which is quite widespread. Any non-trivially copyable object is not going to be passable through such a thunk. You might consider looking at CodeGenFunction::EmitMustTailThunk instead. ================ Comment at: lib/CodeGen/CodeGenFunction.cpp:2432 llvm::BasicBlock *RetBlock = createBasicBlock("resolver_return", Resolver); llvm::IRBuilder<> RetBuilder(RetBlock); + CreateMultiVersionResolverReturn(Resolver, RetBuilder, RO.Function, ---------------- You can make this a CGBuilderTy. Just pass it a type cache. ================ Comment at: test/CodeGenCXX/attr-target-mv-overloads.cpp:74 +// WINDOWS: define dso_local i32 @"?foo_overload@@YAHH@Z.resolver"(i32) comdat +// WINDOWS: call i32 @"?foo_overload@@YAHH@Z.arch_sandybridge" +// WINDOWS: call i32 @"?foo_overload@@YAHH@Z.arch_ivybridge" ---------------- Does dumpbin demangle these names correctly? Just curious. Repository: rC Clang https://reviews.llvm.org/D53586 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits