[PATCH] D101639: Add build_fence to OCaml bindings

[Francesco Bertolaccini via Phabricator via llvm-commits]

frabert added inline comments.


================
Comment at: llvm/bindings/ocaml/llvm/llvm.mli:2600-2604
+(** [build_fence ord st name b] creates a
+    [%fence %]
+    instruction at the position specified by the instruction builder [b].
+    See the method [llvm::LLVMBuilder::CreateFence]. *)
+val build_fence : AtomicOrdering.t -> bool -> string -> llbuilder -> llvalue
----------------
jberdine wrote:
> frabert wrote:
> > jberdine wrote:
> > > It would be good to also document the [ord], [st], and [name] arguments. See e.g. the docstring for [build_atomicrmw] for how it describes the first two.
> > I have addressed the issue concerning the first two arguments, they now have documentation comparable to that of [build_atomicrmw].
> > 
> > I am not sure how to approach [name], though: I couldn't find from a first glance any [build_*] function which actually documents its use, leaving it implicitly assumed that it will be the name of the result.
> Would it make sense to do the same with `name` as in e.g. `build_freeze` just above?
I tried merging the two approaches. I have not found a way to succintly express the singlethread flag in the IR syntax, so I left that particular parameter out of the generated instruction, only mentioning in the doc comment.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101639/new/

https://reviews.llvm.org/D101639