[PATCH] Remove CloneFunction VMap

Pete Cooper peter_cooper at apple.com
Tue Apr 28 13:21:19 PDT 2015


Hey Tobias

Sorry for the late reply.
> On Apr 24, 2015, at 10:49 AM, Tobias Edler von Koch <tobias at codeaurora.org> wrote:
> 
> Hi Pete,
> 
> I have a downstream pass using this, but it's not a big deal because
> it's possible to replicate the old behavior by creating a new
> function yourself and then using CloneFunctionInto, which lets you remap
> the arguments.
So it took a look at the callers of CloneFunctionInto and none of them are using this functionality either.  I’d love to remove all of the attribute set hacking at the top of CloneFunctionInto, but that’ll break your out-of-tree code.  Specifically, its this code, and the loop under it:

  // Copy all attributes other than those stored in the AttributeSet.  We need
  // to remap the parameter indices of the AttributeSet.
  AttributeSet NewAttrs = NewFunc->getAttributes();
  NewFunc->copyAttributesFrom(OldFunc);
  NewFunc->setAttributes(NewAttrs);

If this code is particularly useful to you then i’m happy to leave it as is.  Otherwise, if there’s some way I can delete the unused code in CloneFunctionInto then that would be great.

What do you think?
> 
> Perhaps it might make sense to add something like "Use
> CloneFunctionInto if remapping of arguments is required" to the
> comment to make it easier for people to figure this out?
Also thanks for this piece of feedback.  I’ve added this comment to the patch.

Cheers,
Pete
> 
> Tobias
> 
> On Fri, 24 Apr 2015 10:13:42 -0700 Pete Cooper
> <peter_cooper at apple.com> wrote:
> 
>> Hi Rafael
>> 
>> The method llvm::CloneFunction takes a VMap in which "any references specified in the VMap are changed to refer to their mapped value instead of the original one".
>> 
>> However, all callers of this function (AMDGPUAlwaysInlinePass, clang’s CGVTables, and PartialInlining) never make use of this functionality.  That is, they always pass an empty map.
>> 
>> Attached is a patch which removes the ability for this method to rewrite arguments, and asserts that no-one tries to.  The function comment has been updated to state this.
>> 
>> This also exposed some code cleanup due to the function types of the old and new functions being the same.
>> 
>> Cheers,
>> Pete
> 
> 
> 
> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
> Linux Foundation Collaborative Project.
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150428/8669b699/attachment.html>


More information about the llvm-commits mailing list