[llvm] r242558 - MergeFuncs: Transfer the function parameter attributes to the call site

[Arnold Schwaighofer]

> On Jul 19, 2015, at 12:54 PM, Nick Lewycky <nicholas at mxc.ca> wrote:
> 
> Arnold wrote:
>> 
>> 
>> Sent from my iPhone
>> 
>>> On Jul 19, 2015, at 12:03 PM, Nick Lewycky<nicholas at mxc.ca>  wrote:
>>> 
>>> Pete Cooper wrote:
>>>> 
>>>>> On Jul 19, 2015, at 10:45 AM, Arnold<aschwaighofer at apple.com
>>>>> <mailto:aschwaighofer at apple.com>>  wrote:
>>>>> 
>>>>> Given that we can call function pointers it seems to make sense to me
>>>>> to require to have (especially ABI changing) parameter attributes on
>>>>> the call site.
>>>> Absolutely.
>>> 
>>> Yes, there is a surprise in LLVM IR: if you take a direct call and turn it into an indirect call on the same callee, you may change how that call is made in ABI-affecting ways. This is just a fact of life for whoever wants to do such a transform. However, this isn't the case regarding this commit to mergefunc.
>>> 
>> 
>> So I misread the LangRef an transferring the parameter attributes is not required?
> 
> I think so. You may have a valid reading, but I don't know how you got there.


I was interpreting the following sentence as requiring a call to have the parameter attributes:

http://llvm.org/docs/LangRef.html#call-instruction

7. ‘function args‘: argument list whose types match the function signature argument types and parameter attributes

> 

> Looking at LangRef again, there is something new to me:
>  * "All ABI-impacting function attributes, such as sret, byval, inreg, returned, and inalloca, must match."
> but referring only to 'musttail' calls. So in that case, you would have to copy the param attrs! (I'm not sure why this rule is here, but your patch isn't the right place for me to argue changes to the langref.)
> 
> If you want to re-submit with a testcase for musttail, I think that would be a good bug fix.