[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