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

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?


—
j h woodyatt <jhw at conjury.org>
http://jhw.dreamwidth.org/

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.

I do not propose to change the names or semantics of any existing functions.

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);

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 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.


—
j h woodyatt <jhw at conjury.org>
http://jhw.dreamwidth.org/

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.