[LLVMdev] ParamAttr Patch - Alignment fix

[Gordon Henriksen]

On Apr 26, 2008, at 17:41, Anders Johnsen wrote:

> Hi Gordon,
>
> Thanks a lot for the feedback. I can see I've been way to  
> concentrated on how
> llvm is build, then on this particular patch. I've done the changes  
> you have
> suggested and it's now a lot nicer and cleaner!
>
> Please do say, if there is anything else.


Nice. Just a few small formatting nitpicks…

• please stick to 80 columns
• don't place braces on the same line

> Index: include/llvm-c/Core.h
> ===================================================================
> --- include/llvm-c/Core.h	(revision 50213)
> +++ include/llvm-c/Core.h	(working copy)
> @@ -69,6 +69,7 @@
>  typedef struct LLVMOpaqueBasicBlock *LLVMBasicBlockRef;
>  typedef struct LLVMOpaqueBuilder *LLVMBuilderRef;
>
> +

>  /* Used to provide a module to JIT or interpreter.
>   * See the llvm::ModuleProvider class.
>   */

? :)

> @@ -441,6 +459,9 @@
>  /* Operations on call sites */
>  void LLVMSetInstructionCallConv(LLVMValueRef Instr, unsigned CC);
>  unsigned LLVMGetInstructionCallConv(LLVMValueRef Instr);
> +void LLVMInstrAddParamAttr(LLVMValueRef Instr, unsigned index,  
> LLVMParamAttr);
> +void LLVMInstrRemoveParamAttr(LLVMValueRef Instr, unsigned index,  
> LLVMParamAttr);
> +void LLVMSetInstrParamAlignment(LLVMValueRef Instr, unsigned index,  
> unsigned align);

Could you swap LLVMSetInstrParamAlignment around to be consistent with  
the others in sentence formation (LLVM<Verb>Instr<Noun>)?

> Index: lib/VMCore/Core.cpp
> ===================================================================
> --- lib/VMCore/Core.cpp	(revision 50213)
> +++ lib/VMCore/Core.cpp	(working copy)
> +void LLVMSetParamAlignment(LLVMValueRef Arg, unsigned align)
> +{
> +  unwrap<Argument>(Arg)->addAttr(
> +          ParamAttr::constructAlignmentFromInt(align));
> +}
> +
...
> +void LLVMSetInstrParamAlignment(LLVMValueRef Instr, unsigned index,  
> unsigned align) {
> +  CallSite Call = CallSite(unwrap<Instruction>(Instr));
> +  Call.setParamAttrs(
> +    Call.getParamAttrs().addAttr(index,
> +        ParamAttr::constructAlignmentFromInt(align)));
> +}
> +

If I call this twice with different values, don't I get the bitwise OR  
of the two constructAlignmentFromInt values? Does PAListPtr provide a  
better API for this?

Thanks Anders!

— Gordon