Add build_fence to OCaml bindings

[frabert]

Summary

Adds support for calling LLVMBuildFence from the OCaml bindings

[frabert]

Fix formatting issues

[frabert]

Squash changes

[jberdine]

Looks good apart from a typo, and a bit more documentation.

llvm/bindings/ocaml/llvm/llvm.ml

1351

llvm/bindings/ocaml/llvm/llvm.mli

2600–2604

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.

[frabert]

Address review

[frabert]

llvm/bindings/ocaml/llvm/llvm.mli

2600–2604

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.

[jberdine]

llvm/bindings/ocaml/llvm/llvm.mli

2600–2604

Would it make sense to do the same with name as in e.g. build_freeze just above?

[frabert]

llvm/bindings/ocaml/llvm/llvm.mli

2600–2604

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.

[jberdine]

Thanks, the docs look good.

I'm sorry to give feedback piecemeal, but it would also be good to add a basic test to llvm/test/Bindings/OCaml/core.ml.

[frabert]

Add tests

[jberdine]

Thanks! LGTM

[frabert]

In D101639#2736074, @jberdine wrote:

I'm sorry to give feedback piecemeal, but it would also be good to add a basic test to llvm/test/Bindings/OCaml/core.ml.

No problem, only issue is I'm not really set up to run the tests right now, so for the time being I can only do stuff by copying. The automated checks are K.O. too, it seems, unfortunately.

[jberdine]

I will try the tests locally

[jberdine]

There is an issue. The fence instruction always returns type void. An SSA name is not permitted for values of type void. The test complains:

> ninja bindings/ocaml/all check-llvm-bindings-ocaml
[17/18] Running lit suite /Users/jjb/src/llvm-project/llvm/test/Bindings/OCaml
FAIL: LLVM :: Bindings/OCaml/core.ml (1 of 16)
******************** TEST 'LLVM :: Bindings/OCaml/core.ml' FAILED ********************
Script:
--
: 'RUN: at line 1';   rm -rf /Users/jjb/src/llvm-project/_build/test/Bindings/OCaml/Output/core.ml.tmp && mkdir -p /Users/jjb/src/llvm-project/_build/test/Bindings/OCaml/Output/core.ml.tmp && cp /Users/jjb/src/llvm-project/llvm/test/Bindings/OCaml/core.ml /Users/jjb/src/llvm-project/_build/test/Bindings/OCaml/Output/core.ml.tmp/core.ml && cp /Users/jjb/src/llvm-project/llvm/test/Bindings/OCaml/Utils/Testsuite.ml /Users/jjb/src/llvm-project/_build/test/Bindings/OCaml/Output/core.ml.tmp/Testsuite.ml
: 'RUN: at line 2';   /Users/jjb/.opam/sledge/bin/ocamlfind ocamlc -cclib -L/Users/jjb/src/llvm-project/_build/test/../lib  -g -w +A -package llvm.analysis -package llvm.bitwriter -I /Users/jjb/src/llvm-project/_build/test/Bindings/OCaml/Output/core.ml.tmp/ -linkpkg /Users/jjb/src/llvm-project/_build/test/Bindings/OCaml/Output/core.ml.tmp/Testsuite.ml /Users/jjb/src/llvm-project/_build/test/Bindings/OCaml/Output/core.ml.tmp/core.ml -o /Users/jjb/src/llvm-project/_build/test/Bindings/OCaml/Output/core.ml.tmp/executable
: 'RUN: at line 3';   /Users/jjb/src/llvm-project/_build/test/Bindings/OCaml/Output/core.ml.tmp/executable /Users/jjb/src/llvm-project/_build/test/Bindings/OCaml/Output/core.ml.tmp/bitcode.bc
: 'RUN: at line 4';   /Users/jjb/.opam/sledge/bin/ocamlfind ocamlopt -cclib -L/Users/jjb/src/llvm-project/_build/test/../lib -cclib -Wl,-rpath,/Users/jjb/src/llvm-project/_build/test/../lib  -g -w +A -package llvm.analysis -package llvm.bitwriter -I /Users/jjb/src/llvm-project/_build/test/Bindings/OCaml/Output/core.ml.tmp/ -linkpkg /Users/jjb/src/llvm-project/_build/test/Bindings/OCaml/Output/core.ml.tmp/Testsuite.ml /Users/jjb/src/llvm-project/_build/test/Bindings/OCaml/Output/core.ml.tmp/core.ml -o /Users/jjb/src/llvm-project/_build/test/Bindings/OCaml/Output/core.ml.tmp/executable
: 'RUN: at line 5';   /Users/jjb/src/llvm-project/_build/test/Bindings/OCaml/Output/core.ml.tmp/executable /Users/jjb/src/llvm-project/_build/test/Bindings/OCaml/Output/core.ml.tmp/bitcode.bc
: 'RUN: at line 6';   /Users/jjb/src/llvm-project/_build/bin/llvm-dis < /Users/jjb/src/llvm-project/_build/test/Bindings/OCaml/Output/core.ml.tmp/bitcode.bc > /Users/jjb/src/llvm-project/_build/test/Bindings/OCaml/Output/core.ml.tmp/dis.ll
: 'RUN: at line 7';   /Users/jjb/src/llvm-project/_build/bin/FileCheck /Users/jjb/src/llvm-project/llvm/test/Bindings/OCaml/core.ml < /Users/jjb/src/llvm-project/_build/test/Bindings/OCaml/Output/core.ml.tmp/dis.ll
: 'RUN: at line 9';   /Users/jjb/src/llvm-project/_build/bin/FileCheck -check-prefix=CHECK-NOWHERE /Users/jjb/src/llvm-project/llvm/test/Bindings/OCaml/core.ml < /Users/jjb/src/llvm-project/_build/test/Bindings/OCaml/Output/core.ml.tmp/dis.ll
--
Exit Code: 134

Command Output (stderr):
--
Assertion failed: (!getType()->isVoidTy() && "Cannot assign a name to void values!"), function setNameImpl, file /Users/jjb/src/llvm-project/llvm/lib/IR/Value.cpp, line 331.
/Users/jjb/src/llvm-project/_build/test/Bindings/OCaml/Output/core.ml.script: line 8: 36476 Abort trap: 6           /Users/jjb/src/llvm-project/_build/test/Bindings/OCaml/Output/core.ml.tmp/executable /Users/jjb/src/llvm-project/_build/test/Bindings/OCaml/Output/core.ml.tmp/bitcode.bc

--

********************
********************
Failed Tests (1):
  LLVM :: Bindings/OCaml/core.ml


Testing Time: 11.17s
  Passed: 15
  Failed:  1

llvm/test/Bindings/OCaml/core.ml

1177–1182

[frabert]

Makes sense, but I don't really understand what purpose the string parameter serves, then. Is it supposed to always be the empty string?

[jberdine]

I did a bit of hunting. The name string must be empty or else LLVMBuildFence will assert. It seems to me that

FenceInst *CreateFence(AtomicOrdering Ordering,
                       SyncScope::ID SSID = SyncScope::System,
                       const Twine &Name = "") {
  return Insert(new FenceInst(Context, Ordering, SSID), Name);
}

in IRBuilder.h should be

FenceInst *CreateFence(AtomicOrdering Ordering,
                       SyncScope::ID SSID = SyncScope::System) {
  return Insert(new FenceInst(Context, Ordering, SSID));
}

and LLVMBuildFence should not take Name as an argument.

A solution that is local to the OCaml bindings would be to just always pass the empty string, and the underlying issue in IRBuilder can be addressed separately.

[frabert]

Remove [name] parameter

[frabert]

Fix docs

[jberdine]

LGTM now, thanks! FTR local testing yields:

❯ ninja bindings/ocaml/all check-llvm-bindings-ocaml
[2598/2599] Running lit suite /Users/jjb/src/llvm-project/llvm/test/Bindings/OCaml

Testing Time: 11.40s
  Passed: 16

[frabert]

Ping

llvm/bindings/ocaml/llvm/llvm.ml

 external build_insertelement : llvalue -> llvalue -> llvalue -> string ->
                                llbuilder -> llvalue = "llvm_build_insertelement"
 external build_shufflevector : llvalue -> llvalue -> llvalue -> string ->
                                llbuilder -> llvalue = "llvm_build_shufflevector"
 external build_extractvalue : llvalue -> int -> string -> llbuilder -> llvalue
                             = "llvm_build_extractvalue"
 external build_insertvalue : llvalue -> llvalue -> int -> string -> llbuilder ->
                              llvalue = "llvm_build_insertvalue"
+external build_fence : AtomicOrdering.t -> bool -> llbuilder -> llvalue
+                     = "llvm_build_fence"
 
 external build_is_null : llvalue -> string -> llbuilder -> llvalue
                        = "llvm_build_is_null"
 external build_is_not_null : llvalue -> string -> llbuilder -> llvalue
                            = "llvm_build_is_not_null"
 external build_ptrdiff : llvalue -> llvalue -> string -> llbuilder -> llvalue
                        = "llvm_build_ptrdiff"
 external build_freeze : llvalue -> string -> llbuilder -> llvalue

llvm/bindings/ocaml/llvm/llvm.mli

 val build_ptrdiff : llvalue -> llvalue -> string -> llbuilder -> llvalue
 
 (** [build_freeze x name b] creates a
     [%name = freeze %x]
     instruction at the position specified by the instruction builder [b].
     See the method [llvm::LLVMBuilder::CreateFreeze]. *)
 val build_freeze : llvalue -> string -> llbuilder -> llvalue
 
+(** [build_fence o st b] creates a 
+    [fence o]
+    instruction with ordering [o] and singlethread flag set to [st] at the
+    position specified by the instruction builder [b].
+    See the method [llvm::LLVMBuilder::CreateFence]. *)
+val build_fence : AtomicOrdering.t -> bool -> llbuilder -> llvalue
 
 (** {6 Memory buffers} *)
 
 module MemoryBuffer : sig
   (** [of_file p] is the memory buffer containing the contents of the file at
       path [p]. If the file could not be read, then [IoError msg] is
       raised. *)
   val of_file : string -> llmemorybuffer

llvm/bindings/ocaml/llvm/llvm_ocaml.c

 \*===----------------------------------------------------------------------===*/
 
 #include <assert.h>
 #include <stdlib.h>
 #include <string.h>
 #include "llvm-c/Core.h"
 #include "llvm-c/Support.h"
 #include "llvm/Config/llvm-config.h"
 #include "caml/alloc.h"
 #include "caml/custom.h"
 #include "caml/memory.h"
 #include "caml/fail.h"
 #include "caml/callback.h"
 
 value llvm_string_of_message(char* Message) {
   value String = caml_copy_string(Message);
   LLVMDisposeMessage(Message);

 }
 
 /* llvalue -> string -> llbuilder -> llvalue */
 CAMLprim LLVMValueRef llvm_build_freeze(LLVMValueRef X,
                                         value Name, value B) {
   return LLVMBuildFreeze(Builder_val(B), X, String_val(Name));
 }
 
+/* AtomicOrdering.t -> bool -> llbuilder -> llvalue */
+CAMLprim LLVMValueRef llvm_build_fence(value Ord, value ST, value B) {
+  return LLVMBuildFence(Builder_val(B), Int_val(Ord), Bool_val(ST), "");
+}
+
 /*===-- Memory buffers ----------------------------------------------------===*/
 
 /* string -> llmemorybuffer
    raises IoError msg on error */
 CAMLprim value llvm_memorybuffer_of_file(value Path) {
   CAMLparam1(Path);
   char *Message;
   LLVMMemoryBufferRef MemBuf;

llvm/test/Bindings/OCaml/core.ml

group "miscellaneous"; begin
     (* CHECK: %build_call = tail call cc63 zeroext i32 @{{.*}}(i32 signext %P2, i32 %P1)
      * CHECK: %build_select = select i1 %build_icmp, i32 %P1, i32 %P2
      * CHECK: %build_va_arg = va_arg i8** null, i32
      * CHECK: %build_extractelement = extractelement <4 x i32> %Vec1, i32 %P2
      * CHECK: %build_insertelement = insertelement <4 x i32> %Vec1, i32 %P1, i32 %P2
      * CHECK: %build_shufflevector = shufflevector <4 x i32> %Vec1, <4 x i32> %Vec2, <4 x i32> <i32 1, i32 1, i32 0, i32 0>
      * CHECK: %build_insertvalue0 = insertvalue{{.*}}%bl, i32 1, 0
      * CHECK: %build_extractvalue = extractvalue{{.*}}%build_insertvalue1, 1
+     * CHECK: fence seq_cst
+     * CHECK: fence acquire
+     * CHECK: fence syncscope("singlethread") release
      *)
     let ci = build_call fn [| p2; p1 |] "build_call" atentry in
     insist (CallConv.c = instruction_call_conv ci);
     set_instruction_call_conv 63 ci;
     insist (63 = instruction_call_conv ci);
     insist (not (is_tail_call ci));
     set_tail_call true ci;
     insist (is_tail_call ci);
group "miscellaneous"; begin
     ignore (build_shufflevector vec1 vec2 t3 "build_shufflevector" atentry);
 
     let p = build_alloca sty "ba" atentry in
     let agg = build_load p "bl" atentry in
     let agg0 = build_insertvalue agg (const_int i32_type 1) 0
                  "build_insertvalue0" atentry in
     let agg1 = build_insertvalue agg0 (const_int i8_type 2) 1
                  "build_insertvalue1" atentry in
-    ignore (build_extractvalue agg1 1 "build_extractvalue" atentry)
+    ignore (build_extractvalue agg1 1 "build_extractvalue" atentry);
+
+    ignore (build_fence AtomicOrdering.SequentiallyConsistent false atentry);
+    ignore (build_fence AtomicOrdering.Acquire false atentry);
+    ignore (build_fence AtomicOrdering.Release true atentry)
   end;
 
   group "metadata"; begin
     (* CHECK: %metadata = add i32 %P1, %P2, !test !1
      * !1 is metadata emitted at EOF.
      *)
     let i = build_add p1 p2 "metadata" atentry in
     insist ((has_metadata i) = false);