On Sunday 27 April 2008 00:48:00 Gordon Henriksen wrote:> 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?
You are not allowed to set alignment twice - tried it, and got myself an
assert. (Is this expected behavior?)
>
> Thanks Anders!
>
> — Gordon
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
I'll wrap it all up sometime tomorrow.
Thanks,
Anders Johnsen