[LLVMdev] the optional function return attribute and the llvm-c bindings

[Duncan Sands]

Hi James,

> On Dec 21, 2010, at 11:33, james woodyatt wrote:
>> On Dec 21, 2010, at 00:43, Duncan Sands wrote:
>>>
>>> IIRC the function return value is considered to be the parameter with index 0.
>>> The function itself is considered to be the parameter with index ~0U.
>>
>> Yes, that's what the documentation seems to say is the proper mode for indexing the return parameter, but when I set an attribute on the parameter with index zero, it gets applied to the first function argument and not the return parameter.
>>
>> I believe the reason for this to be that LLVMGetParams() [and its cognates] all use Function::arg_iterator, which begins with the first function *argument* parameter and not the return parameter.  The index argument to LLVMGetParams() is actually incremented by one when it's mapped to the index used for the Function::AttributeList member.  There are separate functions for the function attributes, which are at index ~0UL, but no similar functions for handling the function return parameter attribute at index zero.
>>
>> Is this intentional?  What is the correct way to get and set a function return parameter attribute with C-language binding to the LLVM core?
>
> I'm sorry to be a pest about this question, but it's a thorn in my paw.
>
> Assuming I'm right and there currently isn't a way to get at the return parameter attribute from the C-language bindings, would a patch to add the facility be welcomed?  If so, then it would be nice to have some guidance on naming issues.

a patch would be welcome.  I suggest you copy the naming style used in the
existing bindings, and base names on the C++ API names when possible.

> I propose to add the following declarations to<llvm-c/Core.h>  along with corresponding definitions in $LLVM/lib/VMCore/Core.cpp:
>
>    extern void LLVMAddReturnAttr(LLVMValueRef Fn, LLVMAttribute PA);
>    extern void LLVMRemoveReturnAttr(LLVMValueRef Fn, LLVMAttribute PA);

Looks good to me.

> I also propose to add the corresponding OCaml functions to the "Operations on functions" section of the Llvm module in the OCaml bindings too, of course:
>
>    (** [add_return_attr f a] adds the return parameter
>        attribute [a] to the return type of the function [f].
>    *)
>    val add_return_attr: llvalue ->  Attribute.t ->  unit
>
>    (** [remove_return_attr f a] removes the return parameter
>        attribute [a] from the return type of the function [f].
>    *)
>    val remove_return_attr: llvalue ->  Attribute.t ->  unit

I don't know anything about OCaml, but except for that it looks OK!

> I also propose to update the ocamldoc text for the following OCaml functions to reflect that the list traversed is the function *argument* list and not the parameter list:
>
>    Llvm.param
>    Llvm.params
>    Llvm.param_begin
>    Llvm.param_end
>    Llvm.param_succ
>    Llvm.param_pred
>    Llvm.iter_params
>    Llvm.rev_iter_params
>    Llvm.fold_left_params
>    Llvm.fold_right_params
>
> Finally, I think the ocamldoc texts for Llvm.add_function_attr and Llvm.remove_function_attr are wrong.  They actually add and remove function attributes, not return parameter attributes, which are no longer the same thing.  I propose to fix those too.
>
> If these contributions would be welcomed, then I'd like to write and submit two patches: one with the above changes to the VMCore, and another with the above changes to the OCaml bindings.  I could get started soon and probably have them ready by the end of the week.

Looking forward to seeing the patch.

Ciao, Duncan.