<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div></div><div><div><font class="Apple-style-span" color="#000000">At </font><span class="Apple-style-span" style="font-family: Times; font-size: 16px; font-style: italic; "><font class="Apple-style-span" color="#000000" face="'Trebuchet MS'" size="3"><span class="Apple-style-span" style="background-color: transparent; font-style: normal; ">Sun Mar 23 22:25:54 CDT 2008, Erick</span></font><span class="Apple-style-span" style="font-family: 'Trebuchet MS'; font-size: 12px; font-style: normal; "> Trzelaar wrote:</span></span></div></div><br><blockquote type="cite"><div>This builds off of the code gordonh recently added for manipulating <span class="Apple-style-span" style="-webkit-text-stroke-width: -1; ">iterators. This adds support for instruction iterators, as well as rewriting the builder code to use these new functions. This lets us eliminate the c bindings for moving around the builder. It also adds some convenience functions like "builder_at_start".</span></div></blockquote><br><div><div>Hi Erick,</div><div><br></div><div>Looks great overall. A bit of nitpicking inline. Only thing missing is a test.</div><div><br></div></div><div>Thanks,</div><div>Gordon</div><div><br></div><div></div><blockquote type="cite"><div>diff --git bindings/ocaml/llvm/llvm.ml bindings/ocaml/llvm/llvm.ml</div><div>index 56bfa7b..c168ead 100644</div><div>--- bindings/ocaml/llvm/llvm.ml</div><div>+++ bindings/ocaml/llvm/llvm.ml</div><div>@@ -531,6 +531,55 @@ let fold_right_blocks f fn init =</div><div> </div><div> (*--... Operations on instructions .........................................--*)</div><div> external instr_parent : llvalue -> llbasicblock = "LLVMGetInstructionParent"</div><div>+external instr_begin : llbasicblock -> (llbasicblock, llvalue) llpos</div><div>+ = "llvm_instr_begin"</div><div>+external instr_succ : llvalue -> (llbasicblock, llvalue) llpos</div><div>+ = "llvm_instr_succ"</div><div>+external instr_end : llbasicblock -> (llbasicblock, llvalue) llrev_pos</div><div>+ = "llvm_instr_end"</div><div>+external instr_pred : llvalue -> (llbasicblock, llvalue) llrev_pos</div><div>+ = "llvm_instr_pred"</div><div>+</div><div>+let rec iter_instrs_range f i e =</div><div>+ if i = e then () else</div><div>+ match i with</div><div>+ | At_end _ -> raise (Invalid_argument "Invalid instruction range.")</div><div>+ | Before i -></div><div>+ f i;</div><div>+ iter_instrs_range f (instr_succ i) e</div><div>+</div><div>+let iter_instrs f bb =</div><div>+ iter_instrs_range f (instr_begin bb) (At_end bb)</div><div>+</div><div>+let rec fold_left_instrs_range f init i e =</div><div>+ if i = e then init else</div><div>+ match i with</div><div>+ | At_end _ -> raise (Invalid_argument "Invalid instruction range.")</div><div>+ | Before i -> fold_left_instrs_range f (f init i) (instr_succ i) e</div><div>+</div><div>+let fold_left_instrs f init bb =</div><div>+ fold_left_instrs_range f init (instr_begin bb) (At_end bb)</div><div>+</div><div>+let rec rev_iter_instrs_range f i e =</div><div>+ if i = e then () else</div><div>+ match i with</div><div>+ | At_start _ -> raise (Invalid_argument "Invalid instruction range.")</div><div>+ | After i -></div><div>+ f i;</div><div>+ rev_iter_instrs_range f (instr_pred i) e</div><div>+</div><div>+let rev_iter_instrs f bb =</div><div>+ rev_iter_instrs_range f (instr_end bb) (At_start bb)</div><div>+</div><div>+let rec fold_right_instr_range f i e init =</div><div>+ if i = e then init else</div><div>+ match i with</div><div>+ | At_start _ -> raise (Invalid_argument "Invalid instruction range.")</div><div>+ | After i -> fold_right_instr_range f (instr_pred i) e (f i init)</div><div>+</div><div>+let fold_right_instrs f bb init =</div><div>+ fold_right_instr_range f (instr_end bb) (At_start bb) init</div><div>+</div></blockquote><div><br></div><div>Looks good. ;)</div><br><blockquote type="cite"><div> </div><div> (*--... Operations on call sites ...........................................--*)</div><div> external instruction_call_conv: llvalue -> int</div><div>@@ -545,14 +594,27 @@ external incoming : llvalue -> (llvalue * llbasicblock) list = "llvm_incoming"</div><div> </div><div> </div><div> (*===-- Instruction builders ----------------------------------------------===*)</div><div>-external builder: unit-> llbuilder = "llvm_builder"</div><div>-external builder_before : llvalue -> llbuilder = "llvm_builder_before"</div><div>-external builder_at_end : llbasicblock -> llbuilder = "llvm_builder_at_end"</div><div>-external position_before : llvalue -> llbuilder -> unit = "llvm_position_before"</div><div>-external position_at_end : llbasicblock -> llbuilder -> unit</div><div>- = "llvm_position_at_end"</div><div>+external builder : unit -> llbuilder = "llvm_builder"</div><div>+external position_builder : (llbasicblock, llvalue) llpos -> llbuilder -> unit</div><div>+ = "llvm_position_builder"</div><div> external insertion_block : llbuilder -> llbasicblock = "llvm_insertion_block"</div><div> </div><div>+let builder_at ip =</div><div>+ let b = builder () in</div><div>+ position_builder ip b;</div><div>+ b</div><div>+</div><div>+let builder_before i = builder_at (Before i)</div><div>+let builder_after i = builder_at (instr_succ i)</div><div>+let builder_at_start bb = builder_at (instr_begin bb)</div><div>+let builder_at_end bb = builder_at (At_end bb)</div><div>+</div><div>+let position_before i = position_builder (Before i)</div><div>+let position_after i = position_builder (instr_succ i)</div><div>+let position_at_start bb = position_builder (instr_begin bb)</div><div>+let position_at_end bb = position_builder (At_end bb)</div></blockquote><div><br></div><div>I think these 8 are largely redundant since they're so trivial, with the exception of the *_after and *_at_end variants that are modestly more important for maintaining compatibility. Though I'm happy to add them if you really prefer.</div><br><blockquote type="cite"><div>+</div><div>+</div><div> (*--... Terminators ........................................................--*)</div><div> external build_ret_void : llbuilder -> llvalue = "llvm_build_ret_void"</div><div> external build_ret : llvalue -> llbuilder -> llvalue = "llvm_build_ret"</div><div>diff --git bindings/ocaml/llvm/llvm.mli bindings/ocaml/llvm/llvm.mli</div><div>index 8be5c65..e5f7835 100644</div><div>--- bindings/ocaml/llvm/llvm.mli</div><div>+++ bindings/ocaml/llvm/llvm.mli</div><div>@@ -1077,6 +1077,43 @@ external block_of_value : llvalue -> llbasicblock = "LLVMValueAsBasicBlock"</div><div> See the method [llvm::Instruction::getParent]. *)</div><div> external instr_parent : llvalue -> llbasicblock = "LLVMGetInstructionParent"</div><div> </div><div>+(** [instr_begin bb] returns the first position i the basic block [bb]'s</div></blockquote><div><br></div><div>'first position i'?</div><br><blockquote type="cite"><div>+ instruction list. [instr_begin] and [instr_succ] can be used to iterate</div><div>+ over the global list in order.</div><div>+ See the method [llvm::Instruction::begin]. *)</div></blockquote><div><br></div><div>llvm::BasicBlock::begin</div><br><blockquote type="cite"><div>+external instr_begin : llbasicblock -> (llbasicblock, llvalue) llpos</div><div>+ = "llvm_instr_begin"</div><div>+</div><div>+(** [instr_succ i] returns the instr variable list position succeeding</div></blockquote><div><br></div><div>instruction list</div><br><blockquote type="cite"><div>+ [Before i].</div><div>+ See the method [llvm::BasicBlock::iterator::operator++]. *)</div><div>+external instr_succ : llvalue -> (llbasicblock, llvalue) llpos</div><div>+ = "llvm_instr_succ"</div><div>+</div><div>+(** [iter_instrs f bb] applies function [f] to each of the instructions of basic</div><div>+ block [bb] in order. Tail recursive. *)</div><div>+val iter_instrs: (llvalue -> unit) -> llbasicblock -> unit</div><div>+</div><div>+(** [fold_left_instrs f init bb] is [f (... (f init g1) ...) gN] where</div><div>+ [g1,...,gN] are the instructions of basic block [bb]. Tail recursive. *)</div><div>+val fold_left_instrs: ('a -> llvalue -> 'a) -> 'a -> llbasicblock -> 'a</div><div>+</div><div>+(** [instr_end bb] returns the last position i the basic block [bb]'s</div></blockquote><div><br></div><div>'last position i'?</div><br><blockquote type="cite"><div>+ instruction list. [instr_end] and [instr_succ] can be used to iterate over</div><div>+ the global list in order.</div></blockquote><div><br></div><div>again</div><br><blockquote type="cite"><div>+ See the method [llvm::BasicBlock::end]. *)</div><div>+external instr_end : llbasicblock -> (llbasicblock, llvalue) llrev_pos</div><div>+ = "llvm_instr_end"</div><div>+</div><div>+(** [instr_pred i] returns the instr variable list position preceding [After i].</div></blockquote><div><br></div><div>instruction list</div><br><blockquote type="cite"><div>+ See the method [llvm::BasicBlock::iterator::operator--]. *)</div><div>+external instr_pred : llvalue -> (llbasicblock, llvalue) llrev_pos</div><div>+ = "llvm_instr_pred"</div><div>+</div><div>+(** [fold_right_instrs f bb init] is [f (... (f init fN) ...) f1] where</div><div>+ [f1,...,fN] are the instructions of basicblock [bb]. Tail recursive. *)</div></blockquote><div><br></div><div>basic block</div><br><blockquote type="cite"><div>+val fold_right_instrs: (llvalue -> 'a -> 'a) -> llbasicblock -> 'a -> 'a</div><div>+</div><div> </div><div> (** {7 Operations on call sites} *)</div><div> </div><div>@@ -1114,25 +1151,50 @@ external incoming : llvalue -> (llvalue * llbasicblock) list = "llvm_incoming"</div><div> (** [builder ()] creates an instruction builder with no position. It is invalid</div><div> to use this builder until its position is set with {!position_before} or</div><div> {!position_at_end}. See the constructor for [llvm::LLVMBuilder]. *)</div><div>-external builder: unit-> llbuilder</div><div>- = "llvm_builder"</div><div>+external builder : unit -> llbuilder = "llvm_builder"</div><div>+</div><div>+(** [builder_at ip] creates an instruction builder positioned at [ip].</div><div>+ See the constructor for [llvm::LLVMBuilder]. *)</div><div>+val builder_at : (llbasicblock, llvalue) llpos -> llbuilder</div><div> </div><div> (** [builder_before ins] creates an instruction builder positioned before the</div><div> instruction [isn]. See the constructor for [llvm::LLVMBuilder]. *)</div><div>-external builder_before : llvalue -> llbuilder = "llvm_builder_before"</div><div>+val builder_before : llvalue -> llbuilder</div><div>+</div><div>+(** [builder_after ins] creates an instruction builder positioned after the</div><div>+ instruction [isn]. See the constructor for [llvm::LLVMBuilder]. *)</div><div>+val builder_after: llvalue -> llbuilder</div><div>+</div><div>+(** [builder_at_start bb] creates an instruction builder positioned at the start</div><div>+ of the basic block [bb]. See the constructor for [llvm::LLVMBuilder]. *)</div><div>+val builder_at_start: llbasicblock -> llbuilder</div><div> </div><div> (** [builder_at_end bb] creates an instruction builder positioned at the end of</div><div> the basic block [bb]. See the constructor for [llvm::LLVMBuilder]. *)</div><div>-external builder_at_end : llbasicblock -> llbuilder = "llvm_builder_at_end"</div><div>+val builder_at_end : llbasicblock -> llbuilder</div><div>+</div><div>+(** [position_builder ip bb] moves the instruction builder [bb] to the position</div><div>+ [ip].</div><div>+ See the constructor for [llvm::LLVMBuilder]. *)</div><div>+external position_builder : (llbasicblock, llvalue) llpos -> llbuilder -> unit</div><div>+ = "llvm_position_builder"</div><div> </div><div> (** [position_before ins b] moves the instruction builder [b] to before the</div><div> instruction [isn]. See the method [llvm::LLVMBuilder::SetInsertPoint]. *)</div><div>-external position_before : llvalue -> llbuilder -> unit = "llvm_position_before"</div><div>+val position_before : llvalue -> llbuilder -> unit</div><div>+</div><div>+(** [position_after ins b] moves the instruction builder [b] to after the</div><div>+ instruction [isn]. See the method [llvm::LLVMBuilder::SetInsertPoint]. *)</div><div>+val position_after : llvalue -> llbuilder -> unit</div><div>+</div><div>+(** [position_at_start bb b] moves the instruction builder [b] to the start of</div><div>+ the of basic block [bb]. See the method</div><div>+ [llvm::LLVMBuilder::SetInsertPoint]. *)</div><div>+val position_at_start : llbasicblock -> llbuilder -> unit</div><div> </div><div> (** [position_at_end bb b] moves the instruction builder [b] to the end of the</div><div> basic block [bb]. See the method [llvm::LLVMBuilder::SetInsertPoint]. *)</div><div>-external position_at_end : llbasicblock -> llbuilder -> unit</div><div>- = "llvm_position_at_end"</div><div>+val position_at_end : llbasicblock -> llbuilder -> unit</div><div> </div><div> (** [insertion_block b] returns the basic block that the builder [b] is</div><div> positioned to insert into. Raises [Not_Found] if the instruction builder is</div><div>diff --git bindings/ocaml/llvm/llvm_ocaml.c bindings/ocaml/llvm/llvm_ocaml.c</div><div>index 1b76488..a4a940e 100644</div><div>--- bindings/ocaml/llvm/llvm_ocaml.c</div><div>+++ bindings/ocaml/llvm/llvm_ocaml.c</div><div>@@ -714,6 +714,12 @@ CAMLprim value llvm_value_is_block(LLVMValueRef Val) {</div><div> return Val_bool(LLVMValueIsBasicBlock(Val));</div><div> }</div><div> </div><div>+/*--... Operations on instructions .........................................--*/</div><div>+</div><div>+DEFINE_ITERATORS(instr, Instruction, LLVMBasicBlockRef, LLVMValueRef,</div><div>+ LLVMGetInstructionParent)</div><div>+</div><div>+</div></blockquote><div><br></div><div>Isn't there already a section for this? I think llvm_instr_parent is bound already.</div><br><blockquote type="cite"><div> /*--... Operations on call sites ...........................................--*/</div><div> </div><div> /* llvalue -> int */</div><div>@@ -789,29 +795,15 @@ CAMLprim value llvm_builder(value Unit) {</div><div> return alloc_builder(LLVMCreateBuilder());</div><div> }</div><div> </div><div>-/* llvalue -> llbuilder */</div><div>-CAMLprim value llvm_builder_before(LLVMValueRef Inst) {</div><div>- LLVMBuilderRef B = LLVMCreateBuilder();</div><div>- LLVMPositionBuilderBefore(B, Inst);</div><div>- return alloc_builder(B);</div><div>-}</div><div>-</div><div>-/* llbasicblock -> llbuilder */</div><div>-CAMLprim value llvm_builder_at_end(LLVMBasicBlockRef BB) {</div><div>- LLVMBuilderRef B = LLVMCreateBuilder();</div><div>- LLVMPositionBuilderAtEnd(B, BB);</div><div>- return alloc_builder(B);</div><div>-}</div><div>-</div><div>-/* llvalue -> llbuilder -> unit */</div><div>-CAMLprim value llvm_position_before(LLVMValueRef Inst, value B) {</div><div>- LLVMPositionBuilderBefore(Builder_val(B), Inst);</div><div>- return Val_unit;</div><div>-}</div><div>-</div><div>-/* llbasicblock -> llbuilder -> unit */</div><div>-CAMLprim value llvm_position_at_end(LLVMBasicBlockRef BB, value B) {</div><div>- LLVMPositionBuilderAtEnd(Builder_val(B), BB);</div><div>+/* (llbasicblock, llvalue) llpos -> llbuilder -> unit */</div><div>+CAMLprim value llvm_position_builder(value Pos, value B) {</div><div>+ if (Tag_val(Pos) == 0) {</div><div>+ LLVMBasicBlockRef BB = (LLVMBasicBlockRef) Op_val(Field(Pos, 0));</div><div>+ LLVMPositionBuilderAtEnd(Builder_val(B), BB);</div><div>+ } else {</div><div>+ LLVMValueRef I = (LLVMValueRef) Op_val(Field(Pos, 0));</div><div>+ LLVMPositionBuilderBefore(Builder_val(B), I);</div><div>+ }</div><div> return Val_unit;</div><div> }</div><div> </div></blockquote><div><br></div><div>Mm. Delicious -'s!</div><div><br></div></body></html>