[PATCH] D18628: Cloning: Clean up the interface to the CloneFunction function.

Duncan P. N. Exon Smith via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 15 13:08:50 PDT 2016


> On 2016-Mar-30, at 15:36, Duncan P. N. Exon Smith via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> +alon.mishne at intel.com, who committed r203662.

-alon.mishne at intel.com, since the last email bounced.

>> On 2016-Mar-30, at 15:14, Peter Collingbourne <peter at pcc.me.uk> wrote:
>> 
>> pcc created this revision.
>> pcc added reviewers: dexonsmith, aprantl.
>> pcc added subscribers: probinson, eugenis, tejohnson, loladiro, llvm-commits.
>> Herald added a reviewer: tstellarAMD.
>> Herald added a subscriber: arsenm.
>> 
>> Remove the ModuleLevelChanges argument, and the ability to create new
>> subprograms for cloned functions. The latter was added without review in
>> r203662, but it has no in-tree clients (all non-test callers pass false
>> for ModuleLevelChanges [1], so it isn't reachable outside of tests). It
>> also isn't clear that adding a duplicate subprogram to the compile unit is
>> always the right thing to do when cloning a function within a module. If
>> this functionality comes back it should be accompanied with a more concrete
>> use case.
>> 
>> Furthermore, all in-tree clients add the returned function to the module.
>> Since that's pretty much the only sensible thing you can do with the function,
>> just do that in CloneFunction.
> 
> This seems like a nice cleanup to me.  LGTM if others agree there's
> no impending use case!

I just saw you mention this in a dev post.  No one has spoken up to
defend the API (and you don't typically need pre-commit review to
delete dead code anyway).  I say go for it.

>> [1] http://llvm-cs.pcc.me.uk/lib/Transforms/Utils/CloneFunction.cpp/rCloneFunction
>> 
>> http://reviews.llvm.org/D18628
>> 
>> Files:
>> include/llvm/Transforms/Utils/Cloning.h
>> lib/Target/AMDGPU/AMDGPUAlwaysInlinePass.cpp
>> lib/Transforms/IPO/PartialInlining.cpp
>> lib/Transforms/Utils/CloneFunction.cpp
>> unittests/Transforms/Utils/Cloning.cpp
>> 
>> <D18628.52140.patch>
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits



More information about the llvm-commits mailing list